From 57353a33f11d0a2baf42ea2aaca60297732bffa6 Mon Sep 17 00:00:00 2001 From: "marpan@webrtc.org" Date: Fri, 16 Dec 2011 17:21:09 +0000 Subject: [PATCH] FEC Receiver: Fix to how old packets (e.g., re-tranmitted packets in hybird NACK-FEC mode) are treated. This change avoids having old packets end up on the current packet list for FEC decoding, and so they are immediately sent out to jitter buffer. The current list of packets for FEC decoding are sent out only when new packet arrives (with time-stamp greater than current). Review URL: http://webrtc-codereview.appspot.com/322009 git-svn-id: http://webrtc.googlecode.com/svn/trunk@1222 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../rtp_rtcp/source/overuse_detector.cc | 32 ++----- .../rtp_rtcp/source/overuse_detector.h | 1 - src/modules/rtp_rtcp/source/receiver_fec.cc | 68 ++++++++++----- src/modules/rtp_rtcp/source/receiver_fec.h | 11 ++- .../rtp_rtcp/source/rtp_receiver_video.cc | 86 +++++++++++++------ src/modules/rtp_rtcp/source/rtp_utility.cc | 26 ++++++ src/modules/rtp_rtcp/source/rtp_utility.h | 11 ++- 7 files changed, 160 insertions(+), 75 deletions(-) diff --git a/src/modules/rtp_rtcp/source/overuse_detector.cc b/src/modules/rtp_rtcp/source/overuse_detector.cc index c34533e574..0657436824 100644 --- a/src/modules/rtp_rtcp/source/overuse_detector.cc +++ b/src/modules/rtp_rtcp/source/overuse_detector.cc @@ -15,6 +15,7 @@ #include "trace.h" #include "overuse_detector.h" #include "remote_rate_control.h" +#include "rtp_utility.h" #include #include //abs @@ -182,7 +183,10 @@ bool OverUseDetector::Update(const WebRtcRTPHeader& rtpHeader, { _currentFrame._timestamp = rtpHeader.header.timestamp; } - else if (OldTimestamp(rtpHeader.header.timestamp, static_cast(_currentFrame._timestamp), wrapped)) + else if (ModuleRTPUtility::OldTimestamp( + rtpHeader.header.timestamp, + static_cast(_currentFrame._timestamp), + &wrapped)) { // Don't update with old data return completeFrame; @@ -196,7 +200,10 @@ bool OverUseDetector::Update(const WebRtcRTPHeader& rtpHeader, WebRtc_Word64 tDelta = 0; double tsDelta = 0; // Check for wrap - OldTimestamp(static_cast(_prevFrame._timestamp), static_cast(_currentFrame._timestamp), wrapped); + ModuleRTPUtility::OldTimestamp( + static_cast(_prevFrame._timestamp), + static_cast(_currentFrame._timestamp), + &wrapped); CompensatedTimeDelta(_currentFrame, _prevFrame, tDelta, tsDelta, wrapped); UpdateKalman(tDelta, tsDelta, _currentFrame._size, _prevFrame._size); } @@ -476,25 +483,4 @@ BandwidthUsage OverUseDetector::Detect(double tsDelta) return _hypothesis; } -bool OverUseDetector::OldTimestamp(WebRtc_UWord32 newTimestamp, WebRtc_UWord32 existingTimestamp, bool& wrapped) -{ - wrapped = (newTimestamp < 0x0000ffff && existingTimestamp > 0xffff0000) || - (newTimestamp > 0xffff0000 && existingTimestamp < 0x0000ffff); - if (existingTimestamp > newTimestamp && !wrapped) - { - return true; - } - else if (existingTimestamp <= newTimestamp && !wrapped) - { - return false; - } - else if (existingTimestamp < newTimestamp && wrapped) - { - return true; - } - else - { - return false; - } -} } // namespace webrtc diff --git a/src/modules/rtp_rtcp/source/overuse_detector.h b/src/modules/rtp_rtcp/source/overuse_detector.h index 685cc6ffa1..3275ce5c63 100644 --- a/src/modules/rtp_rtcp/source/overuse_detector.h +++ b/src/modules/rtp_rtcp/source/overuse_detector.h @@ -48,7 +48,6 @@ public: void SetRateControlRegion(RateControlRegion region); private: - static bool OldTimestamp(WebRtc_UWord32 newTimestamp, WebRtc_UWord32 existingTimestamp, bool& wrapped); void CompensatedTimeDelta(const FrameSample& currentFrame, const FrameSample& prevFrame, WebRtc_Word64& tDelta, double& tsDelta, bool wrapped); void UpdateKalman(WebRtc_Word64 tDelta, double tsDelta, WebRtc_UWord32 frameSize, WebRtc_UWord32 prevFrameSize); diff --git a/src/modules/rtp_rtcp/source/receiver_fec.cc b/src/modules/rtp_rtcp/source/receiver_fec.cc index 2278278242..63c2821110 100644 --- a/src/modules/rtp_rtcp/source/receiver_fec.cc +++ b/src/modules/rtp_rtcp/source/receiver_fec.cc @@ -12,7 +12,6 @@ #include "receiver_fec.h" #include "rtp_receiver_video.h" -#include "forward_error_correction.h" #include "rtp_utility.h" // RFC 5109 @@ -89,7 +88,8 @@ WebRtc_Word32 ReceiverFEC::AddReceivedFECPacket(const WebRtcRTPHeader* rtpHeader, const WebRtc_UWord8* incomingRtpPacket, const WebRtc_UWord16 payloadDataLength, - bool& FECpacket ) + bool& FECpacket, + bool oldPacket) { if (_payloadTypeFEC == -1) { @@ -112,6 +112,11 @@ ReceiverFEC::AddReceivedFECPacket(const WebRtcRTPHeader* rtpHeader, { receivedPacket->isFec = true; FECpacket = true; + // We don't need to parse old FEC packets. + // Old FEC packets are sent to jitter buffer as empty packets in the + // callback in rtp_receiver_video. + if (oldPacket) + return 0; } else { receivedPacket->isFec = false; @@ -229,11 +234,28 @@ ReceiverFEC::AddReceivedFECPacket(const WebRtcRTPHeader* rtpHeader, delete receivedPacket; return 0; } - _receivedPacketList.PushBack(receivedPacket); - if (secondReceivedPacket) + + // Send any old media packets to jitter buffer, don't push them onto + // received list for FEC decoding (we don't do FEC decoding on old packets). + if (oldPacket && receivedPacket->isFec == false) { - _receivedPacketList.PushBack(secondReceivedPacket); + if (ParseAndReceivePacket(receivedPacket->pkt) != 0) { + return -1; + } + + delete receivedPacket->pkt; + delete receivedPacket; } + + else + { + _receivedPacketList.PushBack(receivedPacket); + if (secondReceivedPacket) + { + _receivedPacketList.PushBack(secondReceivedPacket); + } + } + return 0; } @@ -306,21 +328,8 @@ ReceiverFEC::ProcessReceivedFEC(const bool forceFrameDecode) ForwardErrorCorrection::RecoveredPacket* recoveredPacket = static_cast(_recoveredPacketList.First()->GetItem()); - WebRtcRTPHeader rtpHeader; - memset(&rtpHeader, 0, sizeof(rtpHeader)); - - ModuleRTPUtility::RTPHeaderParser rtpHeaderParser(recoveredPacket->pkt->data, - recoveredPacket->pkt->length); - - if (!rtpHeaderParser.Parse(rtpHeader)) - { - return -1; - } - if (_owner->ReceiveRecoveredPacketCallback(&rtpHeader, - &recoveredPacket->pkt->data[rtpHeader.header.headerLength], - recoveredPacket->pkt->length - rtpHeader.header.headerLength) != 0) - { - return -1; + if (ParseAndReceivePacket(recoveredPacket->pkt) != 0) { + return -1; } delete recoveredPacket->pkt; @@ -333,4 +342,23 @@ ReceiverFEC::ProcessReceivedFEC(const bool forceFrameDecode) return 0; } + +int ReceiverFEC::ParseAndReceivePacket( + const ForwardErrorCorrection::Packet* packet) { + + WebRtcRTPHeader header; + memset(&header, 0, sizeof(header)); + ModuleRTPUtility::RTPHeaderParser parser(packet->data, + packet->length); + if (!parser.Parse(header)) { + return -1; + } + if (_owner->ReceiveRecoveredPacketCallback( + &header, + &packet->data[header.header.headerLength], + packet->length - header.header.headerLength) != 0) { + return -1; + } + return 0; +} } // namespace webrtc diff --git a/src/modules/rtp_rtcp/source/receiver_fec.h b/src/modules/rtp_rtcp/source/receiver_fec.h index e680cfee4c..e7b34f2e6e 100644 --- a/src/modules/rtp_rtcp/source/receiver_fec.h +++ b/src/modules/rtp_rtcp/source/receiver_fec.h @@ -12,12 +12,13 @@ #define WEBRTC_MODULES_RTP_RTCP_SOURCE_RECEIVER_FEC_H_ #include "rtp_rtcp_defines.h" +// This header is included to get the nested declaration of Packet structure. +#include "forward_error_correction.h" #include "typedefs.h" #include "list_wrapper.h" namespace webrtc { -class ForwardErrorCorrection; class RTPReceiverVideo; class ReceiverFEC @@ -27,9 +28,10 @@ public: virtual ~ReceiverFEC(); WebRtc_Word32 AddReceivedFECPacket(const WebRtcRTPHeader* rtpHeader, - const WebRtc_UWord8* incomingRtpPacket, - const WebRtc_UWord16 payloadDataLength, - bool& FECpacket); + const WebRtc_UWord8* incomingRtpPacket, + const WebRtc_UWord16 payloadDataLength, + bool& FECpacket, + bool oldPacket); void AddReceivedFECInfo(const WebRtcRTPHeader* rtpHeader, const WebRtc_UWord8* incomingRtpPacket, @@ -40,6 +42,7 @@ public: void SetPayloadTypeFEC(const WebRtc_Word8 payloadType); private: + int ParseAndReceivePacket(const ForwardErrorCorrection::Packet* packet); RTPReceiverVideo* _owner; ForwardErrorCorrection* _fec; ListWrapper _receivedPacketList; diff --git a/src/modules/rtp_rtcp/source/rtp_receiver_video.cc b/src/modules/rtp_rtcp/source/rtp_receiver_video.cc index 051f752bee..82c664de72 100644 --- a/src/modules/rtp_rtcp/source/rtp_receiver_video.cc +++ b/src/modules/rtp_rtcp/source/rtp_receiver_video.cc @@ -17,6 +17,7 @@ #include "critical_section_wrapper.h" #include "receiver_fec.h" #include "rtp_rtcp_impl.h" +#include "rtp_utility.h" namespace webrtc { WebRtc_UWord32 BitRateBPS(WebRtc_UWord16 x ) @@ -245,45 +246,78 @@ RTPReceiverVideo::ParseVideoCodecSpecific(WebRtcRTPHeader* rtpHeader, _criticalSectionReceiverVideo->Leave(); return -1; } - if (rtpHeader->header.timestamp != TimeStamp()) - { - // We have a new frame. Force a decode with the existing packets. - retVal = _receiveFEC->ProcessReceivedFEC(true); - _currentFecFrameDecoded = false; - } - + bool oldPacket = false; bool FECpacket = false; - if(retVal != -1) - { - if (!_currentFecFrameDecoded) - { - retVal = _receiveFEC->AddReceivedFECPacket(rtpHeader, incomingRtpPacket, payloadDataLength, FECpacket); + bool wrapped = false; // Not used; just for OldTimeStamp(). - if (retVal != -1 && (FECpacket || rtpHeader->header.markerBit)) - { - // Only attempt a decode after receiving the last media packet. - retVal = _receiveFEC->ProcessReceivedFEC(false); - } - }else + // Check for old packets. + if (ModuleRTPUtility::OldTimestamp(rtpHeader->header.timestamp, + TimeStamp(), + &wrapped)) + { + // We have an old packet. + // FEC receiver holds a list of packets with current timestamp. + // Setting "oldPacket = true" will send old packets directly + // to the jitter buffer. + oldPacket = true; + retVal = _receiveFEC->AddReceivedFECPacket(rtpHeader, + incomingRtpPacket, + payloadDataLength, + FECpacket, + oldPacket); + } + else + { + // Check for future packets. + if (rtpHeader->header.timestamp != TimeStamp()) { - _receiveFEC->AddReceivedFECInfo(rtpHeader,incomingRtpPacket, FECpacket); + // We have a packet from next frame. + // Force a decode with the existing packets. + retVal = _receiveFEC->ProcessReceivedFEC(true); + _currentFecFrameDecoded = false; + } + if(retVal != -1) + { + if (!_currentFecFrameDecoded) + { + retVal = _receiveFEC->AddReceivedFECPacket( + rtpHeader, + incomingRtpPacket, + payloadDataLength, + FECpacket, + oldPacket); + + if (retVal != -1 && (FECpacket || + rtpHeader->header.markerBit)) + { + // Only attempt a decode after receiving the + // last media packet or an FEC packet. + retVal = _receiveFEC->ProcessReceivedFEC(false); + } + }else + { + _receiveFEC->AddReceivedFECInfo(rtpHeader, + incomingRtpPacket, + FECpacket); + } } } _criticalSectionReceiverVideo->Leave(); - if(retVal == 0 && FECpacket ) + if(retVal == 0 && FECpacket) { - // callback with the received FEC packet, the normal packets are deliverd after parsing - // this contain the original RTP packet header but with empty payload and data length + // Callback with the received FEC packet. + // The normal packets are delivered after parsing. + // This contains the original RTP packet header but with + // empty payload and data length. rtpHeader->frameType = kFrameEmpty; - WebRtc_Word32 retVal = SetCodecType(videoType, rtpHeader); //we need this for the routing + // We need this for the routing. + WebRtc_Word32 retVal = SetCodecType(videoType, rtpHeader); if(retVal != 0) { return retVal; } - retVal =CallbackOfReceivedPayloadData(NULL, - 0, - rtpHeader); + retVal = CallbackOfReceivedPayloadData(NULL, 0, rtpHeader); } }else { diff --git a/src/modules/rtp_rtcp/source/rtp_utility.cc b/src/modules/rtp_rtcp/source/rtp_utility.cc index f77a99d61c..de4871ece3 100644 --- a/src/modules/rtp_rtcp/source/rtp_utility.cc +++ b/src/modules/rtp_rtcp/source/rtp_utility.cc @@ -289,6 +289,32 @@ WebRtc_UWord32 ConvertNTPTimeToMS(WebRtc_UWord32 NTPsec, return MStime; } +bool OldTimestamp(uint32_t newTimestamp, + uint32_t existingTimestamp, + bool* wrapped) +{ + bool tmpWrapped = + (newTimestamp < 0x0000ffff && existingTimestamp > 0xffff0000) || + (newTimestamp > 0xffff0000 && existingTimestamp < 0x0000ffff); + *wrapped = tmpWrapped; + if (existingTimestamp > newTimestamp && !tmpWrapped) + { + return true; + } + else if (existingTimestamp <= newTimestamp && !tmpWrapped) + { + return false; + } + else if (existingTimestamp < newTimestamp && tmpWrapped) + { + return true; + } + else + { + return false; + } +} + } // namespace ModuleRTPUtility /* diff --git a/src/modules/rtp_rtcp/source/rtp_utility.h b/src/modules/rtp_rtcp/source/rtp_utility.h index 3ce3537a7f..39ce9ae3f2 100644 --- a/src/modules/rtp_rtcp/source/rtp_utility.h +++ b/src/modules/rtp_rtcp/source/rtp_utility.h @@ -85,7 +85,16 @@ namespace ModuleRTPUtility WebRtc_UWord32 pow2(WebRtc_UWord8 exp); - bool StringCompare(const WebRtc_Word8* str1 , const WebRtc_Word8* str2, const WebRtc_UWord32 length); + // Returns true if |newTimestamp| is older than |existingTimestamp|. + // |wrapped| will be set to true if there has been a wraparound between the + // two timestamps. + bool OldTimestamp(uint32_t newTimestamp, + uint32_t existingTimestamp, + bool* wrapped); + + bool StringCompare(const WebRtc_Word8* str1, + const WebRtc_Word8* str2, + const WebRtc_UWord32 length); void AssignUWord32ToBuffer(WebRtc_UWord8* dataBuffer, WebRtc_UWord32 value); void AssignUWord24ToBuffer(WebRtc_UWord8* dataBuffer, WebRtc_UWord32 value);