generic-library/webrtc
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix for making sure that the packet in order checks are done prior to updating the last received packet state.

Browse Source 
Without this fix all packets are considered out-of-order by the rtp receiver, causing the last received state
in the rtp receiver to never get valid.

Also makes sure that only valid timestamps and receive times are used for audio/video sync.

BUG=2608
R=mflodman@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/3609004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5102 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
stefan@webrtc.org 2013-11-08 15:18:52 +00:00
parent bff9620116
commit 48df38114d
9 changed files with 54 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
webrtc/modules/rtp_rtcp/interface/rtp_receiver.h
View File

@ -82,10 +82,12 @@ class RtpReceiver {
// Turn negative acknowledgement (NACK) requests on/off.
virtual void SetNACKStatus(const NACKMethod method) = 0;
// Returns the last received timestamp.
virtual uint32_t Timestamp() const = 0;
// Returns the time in milliseconds when the last timestamp was received.
virtual int32_t LastReceivedTimeMs() const = 0;
// Gets the last received timestamp. Returns true if a packet has been
// received, false otherwise.
virtual bool Timestamp(uint32_t* timestamp) const = 0;
// Gets the time in milliseconds when the last timestamp was received.
// Returns true if a packet has been received, false otherwise.
virtual bool LastReceivedTimeMs(int64_t* receive_time_ms) const = 0;
// Returns the remote SSRC of the currently received RTP stream.
virtual uint32_t SSRC() const = 0;

34
webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc
View File

@ -80,7 +80,7 @@ RtpReceiverImpl::RtpReceiverImpl(int32_t id,
num_csrcs_(0),
current_remote_csrc_(),
last_received_timestamp_(0),
last_received_frame_time_ms_(0),
last_received_frame_time_ms_(-1),
last_received_sequence_number_(0),
nack_method_(kNackOff) {
assert(incoming_audio_messages_callback);
@ -228,18 +228,20 @@ bool RtpReceiverImpl::IncomingRtpPacket(
uint16_t payload_data_length = payload_length - rtp_header.paddingLength;
bool is_first_packet_in_frame = false;
bool is_first_packet = false;
{
CriticalSectionScoped lock(critical_section_rtp_receiver_.get());
is_first_packet_in_frame =
if (HaveReceivedFrame()) {
is_first_packet_in_frame =
last_received_sequence_number_ + 1 == rtp_header.sequenceNumber &&
Timestamp() != rtp_header.timestamp;
is_first_packet = is_first_packet_in_frame || last_receive_time_ == 0;
last_received_timestamp_ != rtp_header.timestamp;
} else {
is_first_packet_in_frame = true;
}
}
int32_t ret_val = rtp_media_receiver_->ParseRtpPacket(
&webrtc_rtp_header, payload_specific, is_red, payload, payload_length,
clock_->TimeInMilliseconds(), is_first_packet);
clock_->TimeInMilliseconds(), is_first_packet_in_frame);
if (ret_val < 0) {
return false;
@ -266,14 +268,24 @@ TelephoneEventHandler* RtpReceiverImpl::GetTelephoneEventHandler() {
return rtp_media_receiver_->GetTelephoneEventHandler();
}
uint32_t RtpReceiverImpl::Timestamp() const {
bool RtpReceiverImpl::Timestamp(uint32_t* timestamp) const {
CriticalSectionScoped lock(critical_section_rtp_receiver_.get());
return last_received_timestamp_;
if (!HaveReceivedFrame())
return false;
*timestamp = last_received_timestamp_;
return true;
}
int32_t RtpReceiverImpl::LastReceivedTimeMs() const {
bool RtpReceiverImpl::LastReceivedTimeMs(int64_t* receive_time_ms) const {
CriticalSectionScoped lock(critical_section_rtp_receiver_.get());
return last_received_frame_time_ms_;
if (!HaveReceivedFrame())
return false;
*receive_time_ms = last_received_frame_time_ms_;
return true;
}
bool RtpReceiverImpl::HaveReceivedFrame() const {
return last_received_frame_time_ms_ >= 0;
}
// Implementation note: must not hold critsect when called.
@ -298,7 +310,7 @@ void RtpReceiverImpl::CheckSSRCChanged(const RTPHeader& rtp_header) {
last_received_timestamp_ = 0;
last_received_sequence_number_ = 0;
last_received_frame_time_ms_ = 0;
last_received_frame_time_ms_ = -1;
// Do we have a SSRC? Then the stream is restarted.
if (ssrc_ != 0) {

6
webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.h
View File

@ -58,8 +58,8 @@ class RtpReceiverImpl : public RtpReceiver {
void SetNACKStatus(const NACKMethod method);
// Returns the last received timestamp.
virtual uint32_t Timestamp() const;
int32_t LastReceivedTimeMs() const;
bool Timestamp(uint32_t* timestamp) const;
bool LastReceivedTimeMs(int64_t* receive_time_ms) const;
uint32_t SSRC() const;
@ -77,6 +77,8 @@ class RtpReceiverImpl : public RtpReceiver {
TelephoneEventHandler* GetTelephoneEventHandler();
private:
bool HaveReceivedFrame() const;
RtpVideoCodecTypes VideoCodecType() const;
void CheckSSRCChanged(const RTPHeader& rtp_header);

4
webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc
View File

@ -232,7 +232,9 @@ TEST_F(RtpRtcpAudioTest, Basic) {
0, -1, test, 4));
EXPECT_EQ(test_ssrc, rtp_receiver2_->SSRC());
EXPECT_EQ(test_timestamp, rtp_receiver2_->Timestamp());
uint32_t timestamp;
EXPECT_TRUE(rtp_receiver2_->Timestamp(&timestamp));
EXPECT_EQ(test_timestamp, timestamp);
}
TEST_F(RtpRtcpAudioTest, RED) {

10
webrtc/video_engine/vie_receiver.cc
View File

@ -260,11 +260,12 @@ int ViEReceiver::InsertRTPPacket(const int8_t* rtp_packet,
payload_length, header);
header.payload_type_frequency = kVideoPayloadTypeFrequency;
bool in_order = IsPacketInOrder(header);
rtp_receive_statistics_->IncomingPacket(header, received_packet_length,
IsPacketRetransmitted(header));
IsPacketRetransmitted(header, in_order));
rtp_payload_registry_->SetIncomingPayloadType(header);
return ReceivePacket(received_packet, received_packet_length, header,
IsPacketInOrder(header)) ? 0 : -1;
in_order) ? 0 : -1;
}
bool ViEReceiver::ReceivePacket(const uint8_t* packet,
@ -457,7 +458,8 @@ bool ViEReceiver::IsPacketInOrder(const RTPHeader& header) const {
return statistician->IsPacketInOrder(header.sequenceNumber);
}
bool ViEReceiver::IsPacketRetransmitted(const RTPHeader& header) const {
bool ViEReceiver::IsPacketRetransmitted(const RTPHeader& header,
bool in_order) const {
// Retransmissions are handled separately if RTX is enabled.
if (rtp_payload_registry_->RtxEnabled())
return false;
@ -468,7 +470,7 @@ bool ViEReceiver::IsPacketRetransmitted(const RTPHeader& header) const {
// Check if this is a retransmission.
uint16_t min_rtt = 0;
rtp_rtcp_->RTT(rtp_receiver_->SSRC(), NULL, NULL, &min_rtt, NULL);
return !IsPacketInOrder(header) &&
return !in_order &&
statistician->IsRetransmitOfOldPacket(header, min_rtt);
}
} // namespace webrtc

2
webrtc/video_engine/vie_receiver.h
View File

@ -96,7 +96,7 @@ class ViEReceiver : public RtpData {
const RTPHeader& header);
int InsertRTCPPacket(const int8_t* rtcp_packet, int rtcp_packet_length);
bool IsPacketInOrder(const RTPHeader& header) const;
bool IsPacketRetransmitted(const RTPHeader& header) const;
bool IsPacketRetransmitted(const RTPHeader& header, bool in_order) const;
scoped_ptr<CriticalSectionWrapper> receive_cs_;
const int32_t channel_id_;

6
webrtc/video_engine/vie_sync_module.cc
View File

@ -26,8 +26,10 @@ enum { kSyncInterval = 1000};
int UpdateMeasurements(StreamSynchronization::Measurements* stream,
const RtpRtcp& rtp_rtcp, const RtpReceiver& receiver) {
stream->latest_timestamp = receiver.Timestamp();
stream->latest_receive_time_ms = receiver.LastReceivedTimeMs();
if (!receiver.Timestamp(&stream->latest_timestamp))
return -1;
if (!receiver.LastReceivedTimeMs(&stream->latest_receive_time_ms))
return -1;
synchronization::RtcpMeasurement measurement;
if (0 != rtp_rtcp.RemoteNTP(&measurement.ntp_secs,
&measurement.ntp_frac,

11
webrtc/voice_engine/channel.cc
View File

@ -2107,11 +2107,11 @@ int32_t Channel::ReceivedRTPPacket(const int8_t* data, int32_t length) {
rtp_payload_registry_->GetPayloadTypeFrequency(header.payloadType);
if (header.payload_type_frequency < 0)
return -1;
bool in_order = IsPacketInOrder(header);
rtp_receive_statistics_->IncomingPacket(header, length,
IsPacketRetransmitted(header));
IsPacketRetransmitted(header, in_order));
rtp_payload_registry_->SetIncomingPayloadType(header);
return ReceivePacket(received_packet, length, header,
IsPacketInOrder(header)) ? 0 : -1;
return ReceivePacket(received_packet, length, header, in_order) ? 0 : -1;
}
bool Channel::ReceivePacket(const uint8_t* packet,
@ -2171,7 +2171,8 @@ bool Channel::IsPacketInOrder(const RTPHeader& header) const {
return statistician->IsPacketInOrder(header.sequenceNumber);
}
bool Channel::IsPacketRetransmitted(const RTPHeader& header) const {
bool Channel::IsPacketRetransmitted(const RTPHeader& header,
bool in_order) const {
// Retransmissions are handled separately if RTX is enabled.
if (rtp_payload_registry_->RtxEnabled())
return false;
@ -2182,7 +2183,7 @@ bool Channel::IsPacketRetransmitted(const RTPHeader& header) const {
// Check if this is a retransmission.
uint16_t min_rtt = 0;
_rtpRtcpModule->RTT(rtp_receiver_->SSRC(), NULL, NULL, &min_rtt, NULL);
return !IsPacketInOrder(header) &&
return !in_order &&
statistician->IsRetransmitOfOldPacket(header, min_rtt);
}

2
webrtc/voice_engine/channel.h
View File

@ -431,7 +431,7 @@ private:
int packet_length,
const RTPHeader& header);
bool IsPacketInOrder(const RTPHeader& header) const;
bool IsPacketRetransmitted(const RTPHeader& header) const;
bool IsPacketRetransmitted(const RTPHeader& header, bool in_order) const;
int ResendPackets(const uint16_t* sequence_numbers, int length);
int InsertInbandDtmfTone();
int32_t MixOrReplaceAudioWithFile(int mixingFrequency);