diff --git a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h index 3a0105d15f..7fcef2099c 100644 --- a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h @@ -733,10 +733,14 @@ class RtpRtcp : public Module { /* * Turn negative acknowledgement requests on/off + * |max_reordering_threshold| should be set to how much a retransmitted + * packet can be expected to be reordered (in sequence numbers) compared to + * a packet which has not been retransmitted. * * return -1 on failure else 0 */ - virtual WebRtc_Word32 SetNACKStatus(const NACKMethod method) = 0; + virtual WebRtc_Word32 SetNACKStatus(const NACKMethod method, + int max_reordering_threshold) = 0; /* * TODO(holmer): Propagate this API to VideoEngine. @@ -774,7 +778,7 @@ class RtpRtcp : public Module { */ virtual WebRtc_Word32 SetStorePacketsStatus( const bool enable, - const WebRtc_UWord16 numberToStore = 200) = 0; + const WebRtc_UWord16 numberToStore) = 0; /************************************************************************** * diff --git a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 52a9c5fba5..e4f3c1c83a 100644 --- a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -233,8 +233,8 @@ class MockRtpRtcp : public RtpRtcp { void(WebRtc_UWord16 bandWidthKbit)); MOCK_CONST_METHOD0(NACK, NACKMethod()); - MOCK_METHOD1(SetNACKStatus, - WebRtc_Word32(const NACKMethod method)); + MOCK_METHOD2(SetNACKStatus, + WebRtc_Word32(const NACKMethod method, int oldestSequenceNumberToNack)); MOCK_CONST_METHOD0(SelectiveRetransmissions, int()); MOCK_METHOD1(SetSelectiveRetransmissions, diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc index d05dd2d5cd..fba1818dd8 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -800,7 +800,6 @@ RTCPReceiver::HandleNACK(RTCPUtility::RTCPParserV2& rtcpParser, rtcpParser.Iterate(); return; } - rtcpPacketInformation.ResetNACKPacketIdArray(); RTCPUtility::RTCPPacketTypes pktType = rtcpParser.Iterate(); @@ -1271,13 +1270,11 @@ void RTCPReceiver::TriggerCallbacksFromRTCPPacket( _rtpRtcp.OnRequestSendReport(); } if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpNack) { - if (rtcpPacketInformation.nackSequenceNumbersLength > 0) { + if (rtcpPacketInformation.nackSequenceNumbers.size() > 0) { WEBRTC_TRACE(kTraceStateInfo, kTraceRtpRtcp, _id, "SIG [RTCP] Incoming NACK length:%d", - rtcpPacketInformation.nackSequenceNumbersLength); - _rtpRtcp.OnReceivedNACK( - rtcpPacketInformation.nackSequenceNumbersLength, - rtcpPacketInformation.nackSequenceNumbers); + rtcpPacketInformation.nackSequenceNumbers.size()); + _rtpRtcp.OnReceivedNACK(rtcpPacketInformation.nackSequenceNumbers); } } { @@ -1451,4 +1448,5 @@ void RTCPReceiver::PacketTimeout() _cbRtcpFeedback->OnRTCPPacketTimeout(_id); } } + } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc index 09c8ba4ffd..4115f5d027 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc @@ -8,12 +8,12 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "rtcp_receiver_help.h" +#include "webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.h" #include // memset #include // assert -#include "modules/rtp_rtcp/source/rtp_utility.h" +#include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" namespace webrtc { using namespace RTCPHelp; @@ -21,8 +21,7 @@ using namespace RTCPHelp; RTCPPacketInformation::RTCPPacketInformation() : rtcpPacketTypeFlags(0), remoteSSRC(0), - nackSequenceNumbers(0), - nackSequenceNumbersLength(0), + nackSequenceNumbers(), applicationSubType(0), applicationName(0), applicationData(), @@ -44,7 +43,6 @@ RTCPPacketInformation::RTCPPacketInformation() RTCPPacketInformation::~RTCPPacketInformation() { - delete [] nackSequenceNumbers; delete [] applicationData; delete VoIPMetric; } @@ -84,23 +82,16 @@ void RTCPPacketInformation::AddApplicationData(const WebRtc_UWord8* data, void RTCPPacketInformation::ResetNACKPacketIdArray() { - if (NULL == nackSequenceNumbers) - { - nackSequenceNumbers = new WebRtc_UWord16[NACK_PACKETS_MAX_SIZE]; - } - nackSequenceNumbersLength = 0; + nackSequenceNumbers.clear(); } void RTCPPacketInformation::AddNACKPacket(const WebRtc_UWord16 packetID) { - assert(nackSequenceNumbers); - - WebRtc_UWord16& idx = nackSequenceNumbersLength; - if (idx < NACK_PACKETS_MAX_SIZE) - { - nackSequenceNumbers[idx++] = packetID; - } + if (nackSequenceNumbers.size() >= kSendSideNackListSizeSanity) { + return; + } + nackSequenceNumbers.push_back(packetID); } void diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.h index 8af7b66949..1d538f0b48 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.h @@ -11,12 +11,14 @@ #ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_RECEIVER_HELP_H_ #define WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_RECEIVER_HELP_H_ -#include +#include -#include "modules/rtp_rtcp/interface/rtp_rtcp_defines.h" // RTCPReportBlock -#include "modules/rtp_rtcp/source/rtcp_utility.h" -#include "modules/rtp_rtcp/source/tmmbr_help.h" -#include "typedefs.h" +#include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h" // RTCPReportBlock +#include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" +#include "webrtc/modules/rtp_rtcp/source/tmmbr_help.h" +#include "webrtc/system_wrappers/interface/constructor_magic.h" +#include "webrtc/system_wrappers/interface/scoped_ptr.h" +#include "webrtc/typedefs.h" namespace webrtc { namespace RTCPHelp @@ -44,8 +46,7 @@ public: WebRtc_UWord32 rtcpPacketTypeFlags; // RTCPPacketTypeFlags bit field WebRtc_UWord32 remoteSSRC; - WebRtc_UWord16* nackSequenceNumbers; - WebRtc_UWord16 nackSequenceNumbersLength; + std::list nackSequenceNumbers; WebRtc_UWord8 applicationSubType; WebRtc_UWord32 applicationName; @@ -69,6 +70,9 @@ public: uint32_t rtp_timestamp; RTCPVoIPMetric* VoIPMetric; + +private: + DISALLOW_COPY_AND_ASSIGN(RTCPPacketInformation); }; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 11d158accb..ad4b9090d6 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -205,7 +205,29 @@ class RtcpReceiverTest : public ::testing::Test { RTCPHelp::RTCPPacketInformation rtcpPacketInformation; int result = rtcp_receiver_->IncomingRTCPPacket(rtcpPacketInformation, &rtcpParser); - rtcp_packet_info_ = rtcpPacketInformation; + // The NACK list is on purpose not copied below as it isn't needed by the + // test. + rtcp_packet_info_.rtcpPacketTypeFlags = + rtcpPacketInformation.rtcpPacketTypeFlags; + rtcp_packet_info_.remoteSSRC = rtcpPacketInformation.remoteSSRC; + rtcp_packet_info_.applicationSubType = + rtcpPacketInformation.applicationSubType; + rtcp_packet_info_.applicationName = rtcpPacketInformation.applicationName; + rtcp_packet_info_.reportBlock = rtcpPacketInformation.reportBlock; + rtcp_packet_info_.fractionLost = rtcpPacketInformation.fractionLost; + rtcp_packet_info_.roundTripTime = rtcpPacketInformation.roundTripTime; + rtcp_packet_info_.lastReceivedExtendedHighSeqNum = + rtcpPacketInformation.lastReceivedExtendedHighSeqNum; + rtcp_packet_info_.jitter = rtcpPacketInformation.jitter; + rtcp_packet_info_.interArrivalJitter = + rtcpPacketInformation.interArrivalJitter; + rtcp_packet_info_.sliPictureId = rtcpPacketInformation.sliPictureId; + rtcp_packet_info_.rpsiPictureId = rtcpPacketInformation.rpsiPictureId; + rtcp_packet_info_.receiverEstimatedMaxBitrate = + rtcpPacketInformation.receiverEstimatedMaxBitrate; + rtcp_packet_info_.ntp_secs = rtcpPacketInformation.ntp_secs; + rtcp_packet_info_.ntp_frac = rtcpPacketInformation.ntp_frac; + rtcp_packet_info_.rtp_timestamp = rtcpPacketInformation.rtp_timestamp; return result; } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index e3cadbd5f7..89f42792a8 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -80,8 +80,28 @@ class TestTransport : public Transport, EXPECT_TRUE(rtcpParser.IsValid()); RTCPHelp::RTCPPacketInformation rtcpPacketInformation; EXPECT_EQ(0, rtcp_receiver_->IncomingRTCPPacket(rtcpPacketInformation, - &rtcpParser)); - rtcp_packet_info_ = rtcpPacketInformation; + &rtcpParser)); + rtcp_packet_info_.rtcpPacketTypeFlags = + rtcpPacketInformation.rtcpPacketTypeFlags; + rtcp_packet_info_.remoteSSRC = rtcpPacketInformation.remoteSSRC; + rtcp_packet_info_.applicationSubType = + rtcpPacketInformation.applicationSubType; + rtcp_packet_info_.applicationName = rtcpPacketInformation.applicationName; + rtcp_packet_info_.reportBlock = rtcpPacketInformation.reportBlock; + rtcp_packet_info_.fractionLost = rtcpPacketInformation.fractionLost; + rtcp_packet_info_.roundTripTime = rtcpPacketInformation.roundTripTime; + rtcp_packet_info_.lastReceivedExtendedHighSeqNum = + rtcpPacketInformation.lastReceivedExtendedHighSeqNum; + rtcp_packet_info_.jitter = rtcpPacketInformation.jitter; + rtcp_packet_info_.interArrivalJitter = + rtcpPacketInformation.interArrivalJitter; + rtcp_packet_info_.sliPictureId = rtcpPacketInformation.sliPictureId; + rtcp_packet_info_.rpsiPictureId = rtcpPacketInformation.rpsiPictureId; + rtcp_packet_info_.receiverEstimatedMaxBitrate = + rtcpPacketInformation.receiverEstimatedMaxBitrate; + rtcp_packet_info_.ntp_secs = rtcpPacketInformation.ntp_secs; + rtcp_packet_info_.ntp_frac = rtcpPacketInformation.ntp_frac; + rtcp_packet_info_.rtp_timestamp = rtcpPacketInformation.rtp_timestamp; return packet_len; } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver.cc index 52b20fc982..0c82657028 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver.cc @@ -91,6 +91,7 @@ RTPReceiver::RTPReceiver(const WebRtc_Word32 id, last_report_jitter_transmission_time_offset_(0), nack_method_(kNackOff), + max_reordering_threshold_(kDefaultMaxReorderingThreshold), rtx_(false), ssrc_rtx_(0) { assert(incoming_audio_messages_callback && @@ -267,8 +268,16 @@ NACKMethod RTPReceiver::NACK() const { } // Turn negative acknowledgment requests on/off. -WebRtc_Word32 RTPReceiver::SetNACKStatus(const NACKMethod method) { +WebRtc_Word32 RTPReceiver::SetNACKStatus(const NACKMethod method, + int max_reordering_threshold) { CriticalSectionScoped lock(critical_section_rtp_receiver_); + if (max_reordering_threshold < 0) { + return -1; + } else if (method == kNackRtcp) { + max_reordering_threshold_ = max_reordering_threshold; + } else { + max_reordering_threshold_ = kDefaultMaxReorderingThreshold; + } nack_method_ = method; return 0; } @@ -572,7 +581,7 @@ bool RTPReceiver::InOrderPacket(const WebRtc_UWord16 sequence_number) const { if (received_seq_max_ >= sequence_number) { // Detect wrap-around. if (!(received_seq_max_ > 0xff00 && sequence_number < 0x0ff)) { - if (received_seq_max_ - NACK_PACKETS_MAX_SIZE > sequence_number) { + if (received_seq_max_ - max_reordering_threshold_ > sequence_number) { // We have a restart of the remote side. } else { // we received a retransmit of a packet we already have. @@ -582,7 +591,7 @@ bool RTPReceiver::InOrderPacket(const WebRtc_UWord16 sequence_number) const { } else { // Detect wrap-around. if (sequence_number > 0xff00 && received_seq_max_ < 0x0ff) { - if (received_seq_max_ - NACK_PACKETS_MAX_SIZE > sequence_number) { + if (received_seq_max_ - max_reordering_threshold_ > sequence_number) { // We have a restart of the remote side } else { // We received a retransmit of a packet we already have diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver.h index aac9e29842..854c556317 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver.h @@ -88,7 +88,8 @@ class RTPReceiver : public Bitrate { NACKMethod NACK() const ; // Turn negative acknowledgement requests on/off. - WebRtc_Word32 SetNACKStatus(const NACKMethod method); + WebRtc_Word32 SetNACKStatus(const NACKMethod method, + int max_reordering_threshold); // Returns the last received timestamp. virtual WebRtc_UWord32 TimeStamp() const; @@ -159,7 +160,6 @@ class RTPReceiver : public Bitrate { virtual WebRtc_Word8 REDPayloadType() const; bool HaveNotReceivedPackets() const; - protected: virtual bool RetransmitOfOldPacket(const WebRtc_UWord16 sequence_number, const WebRtc_UWord32 rtp_time_stamp) const; @@ -184,7 +184,6 @@ class RTPReceiver : public Bitrate { void UpdateNACKBitRate(WebRtc_Word32 bytes, WebRtc_UWord32 now); bool ProcessNACKBitRate(WebRtc_UWord32 now); - private: RTPPayloadRegistry* rtp_payload_registry_; scoped_ptr rtp_media_receiver_; @@ -243,6 +242,7 @@ class RTPReceiver : public Bitrate { mutable WebRtc_UWord32 last_report_jitter_transmission_time_offset_; NACKMethod nack_method_; + int max_reordering_threshold_; bool rtx_; WebRtc_UWord32 ssrc_rtx_; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h index e128a10ffc..c8886365e3 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h @@ -18,8 +18,10 @@ enum { kRtpRtcpMaxIdleTimeProcess = 5, kRtpRtcpPacketTimeoutProcessTimeMs = 100, kRtpRtcpRttProcessTimeMs = 1000 }; -enum { NACK_PACKETS_MAX_SIZE = 256 }; // in packets enum { NACK_BYTECOUNT_SIZE = 60}; // size of our NACK history +// A sanity for the NACK list parsing at the send-side. +enum { kSendSideNackListSizeSanity = 20000 }; +enum { kDefaultMaxReorderingThreshold = 50 }; // In sequence numbers. enum { RTCP_INTERVAL_VIDEO_MS = 1000 }; enum { RTCP_INTERVAL_AUDIO_MS = 5000 }; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 3cb799482b..9408361966 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -1477,14 +1477,15 @@ NACKMethod ModuleRtpRtcpImpl::NACK() const { } // Turn negative acknowledgment requests on/off. -WebRtc_Word32 ModuleRtpRtcpImpl::SetNACKStatus(NACKMethod method) { +WebRtc_Word32 ModuleRtpRtcpImpl::SetNACKStatus( + NACKMethod method, int max_reordering_threshold) { WEBRTC_TRACE(kTraceModuleCall, kTraceRtpRtcp, id_, "SetNACKStatus(%u)", method); nack_method_ = method; - rtp_receiver_->SetNACKStatus(method); + rtp_receiver_->SetNACKStatus(method, max_reordering_threshold); return 0; } @@ -1516,10 +1517,6 @@ WebRtc_Word32 ModuleRtpRtcpImpl::SendNACK(const WebRtc_UWord16* nack_list, id_, "SendNACK(size:%u)", size); - if (size > NACK_PACKETS_MAX_SIZE) { - RequestKeyFrame(); - return -1; - } WebRtc_UWord16 avg_rtt = 0; rtcp_receiver_.RTT(rtp_receiver_->SSRC(), NULL, &avg_rtt, NULL, NULL); @@ -2006,18 +2003,14 @@ WebRtc_UWord32 ModuleRtpRtcpImpl::SendTimeOfSendReport( } void ModuleRtpRtcpImpl::OnReceivedNACK( - const WebRtc_UWord16 nack_sequence_numbers_length, - const WebRtc_UWord16* nack_sequence_numbers) { + const std::list& nack_sequence_numbers) { if (!rtp_sender_.StorePackets() || - nack_sequence_numbers == NULL || - nack_sequence_numbers_length == 0) { + nack_sequence_numbers.size() == 0) { return; } WebRtc_UWord16 avg_rtt = 0; rtcp_receiver_.RTT(rtp_receiver_->SSRC(), NULL, &avg_rtt, NULL, NULL); - rtp_sender_.OnReceivedNACK(nack_sequence_numbers_length, - nack_sequence_numbers, - avg_rtt); + rtp_sender_.OnReceivedNACK(nack_sequence_numbers, avg_rtt); } WebRtc_Word32 ModuleRtpRtcpImpl::LastReceivedNTP( diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 666612bda3..49f7a28da9 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -312,7 +312,8 @@ class ModuleRtpRtcpImpl : public RtpRtcp { virtual NACKMethod NACK() const; // Turn negative acknowledgment requests on/off. - virtual WebRtc_Word32 SetNACKStatus(const NACKMethod method); + virtual WebRtc_Word32 SetNACKStatus(const NACKMethod method, + int max_reordering_threshold); virtual int SelectiveRetransmissions() const; @@ -325,7 +326,7 @@ class ModuleRtpRtcpImpl : public RtpRtcp { // Store the sent packets, needed to answer to a negative acknowledgment // requests. virtual WebRtc_Word32 SetStorePacketsStatus( - const bool enable, const WebRtc_UWord16 number_to_store = 200); + const bool enable, const WebRtc_UWord16 number_to_store); // (APP) Application specific data. virtual WebRtc_Word32 SetRTCPApplicationSpecificData( @@ -447,8 +448,7 @@ class ModuleRtpRtcpImpl : public RtpRtcp { void OnReceivedReferencePictureSelectionIndication( const WebRtc_UWord64 picture_id); - void OnReceivedNACK(const WebRtc_UWord16 nack_sequence_numbers_length, - const WebRtc_UWord16* nack_sequence_numbers); + void OnReceivedNACK(const std::list& nack_sequence_numbers); void OnRequestSendReport(); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index ae67e323d4..991f12ad69 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -514,8 +514,7 @@ int RTPSender::SetSelectiveRetransmissions(uint8_t settings) { } void RTPSender::OnReceivedNACK( - const WebRtc_UWord16 nack_sequence_numbers_length, - const WebRtc_UWord16 *nack_sequence_numbers, + const std::list& nack_sequence_numbers, const WebRtc_UWord16 avg_rtt) { const WebRtc_Word64 now = clock_->TimeInMilliseconds(); WebRtc_UWord32 bytes_re_sent = 0; @@ -528,9 +527,9 @@ void RTPSender::OnReceivedNACK( return; } - for (WebRtc_UWord16 i = 0; i < nack_sequence_numbers_length; ++i) { - const WebRtc_Word32 bytes_sent = ReSendPacket(nack_sequence_numbers[i], - 5 + avg_rtt); + for (std::list::const_iterator it = nack_sequence_numbers.begin(); + it != nack_sequence_numbers.end(); ++it) { + const WebRtc_Word32 bytes_sent = ReSendPacket(*it, 5 + avg_rtt); if (bytes_sent > 0) { bytes_re_sent += bytes_sent; } else if (bytes_sent == 0) { @@ -541,7 +540,7 @@ void RTPSender::OnReceivedNACK( // Failed to send one Sequence number. Give up the rest in this nack. WEBRTC_TRACE(kTraceWarning, kTraceRtpRtcp, id_, "Failed resending RTP packet %d, Discard rest of packets", - nack_sequence_numbers[i]); + *it); break; } // Delay bandwidth estimate (RTT * BW). diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h index 33d91c7940..dc8cf3ad1b 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -159,8 +159,7 @@ class RTPSender : public Bitrate, public RTPSenderInterface { // NACK. int SelectiveRetransmissions() const; int SetSelectiveRetransmissions(uint8_t settings); - void OnReceivedNACK(const WebRtc_UWord16 nack_sequence_numbers_length, - const WebRtc_UWord16 *nack_sequence_numbers, + void OnReceivedNACK(const std::list& nack_sequence_numbers, const WebRtc_UWord16 avg_rtt); void SetStorePacketsStatus(const bool enable, diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc index 4c9f36efee..aa822195cd 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc @@ -109,6 +109,6 @@ TEST_F(RtpRtcpAPITest, RTCP) { EXPECT_FALSE(module->TMMBR()); EXPECT_EQ(kNackOff, module->NACK()); - EXPECT_EQ(0, module->SetNACKStatus(kNackRtcp)); + EXPECT_EQ(0, module->SetNACKStatus(kNackRtcp, 450)); EXPECT_EQ(kNackRtcp, module->NACK()); } diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_nack.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_nack.cc index b72d0b349d..16e4601486 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_nack.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_nack.cc @@ -116,8 +116,8 @@ class RtpRtcpNackTest : public ::testing::Test { EXPECT_EQ(0, video_module_->SetRTCPStatus(kRtcpCompound)); EXPECT_EQ(0, video_module_->SetSSRC(kTestSsrc)); - EXPECT_EQ(0, video_module_->SetNACKStatus(kNackRtcp)); - EXPECT_EQ(0, video_module_->SetStorePacketsStatus(true)); + EXPECT_EQ(0, video_module_->SetNACKStatus(kNackRtcp, 450)); + EXPECT_EQ(0, video_module_->SetStorePacketsStatus(true, 600)); EXPECT_EQ(0, video_module_->SetSendingStatus(true)); EXPECT_EQ(0, video_module_->SetSequenceNumber(kTestSequenceNumber)); EXPECT_EQ(0, video_module_->SetStartTimestamp(111111)); diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc index b87904ae9e..cf181a66e1 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc @@ -47,8 +47,8 @@ class RtpRtcpVideoTest : public ::testing::Test { EXPECT_EQ(0, video_module_->SetRTCPStatus(kRtcpCompound)); EXPECT_EQ(0, video_module_->SetSSRC(test_ssrc_)); - EXPECT_EQ(0, video_module_->SetNACKStatus(kNackRtcp)); - EXPECT_EQ(0, video_module_->SetStorePacketsStatus(true)); + EXPECT_EQ(0, video_module_->SetNACKStatus(kNackRtcp, 450)); + EXPECT_EQ(0, video_module_->SetStorePacketsStatus(true, 600)); EXPECT_EQ(0, video_module_->SetSendingStatus(true)); transport_->SetSendModule(video_module_); diff --git a/webrtc/modules/video_coding/main/interface/video_coding.h b/webrtc/modules/video_coding/main/interface/video_coding.h index b5adf35e52..77bd9cebd1 100644 --- a/webrtc/modules/video_coding/main/interface/video_coding.h +++ b/webrtc/modules/video_coding/main/interface/video_coding.h @@ -546,6 +546,13 @@ public: virtual int SetReceiverRobustnessMode(ReceiverRobustness robustnessMode, DecodeErrors errorMode) = 0; + // Sets the maximum number of sequence numbers that we are allowed to NACK + // and the oldest sequence number that we will consider to NACK. If a + // sequence number older than |max_packet_age_to_nack| is missing + // a key frame will be requested. + virtual void SetNackSettings(size_t max_nack_list_size, + int max_packet_age_to_nack) = 0; + // Enables recording of debugging information. virtual int StartDebugRecording(const char* file_name_utf8) = 0; diff --git a/webrtc/modules/video_coding/main/interface/video_coding_defines.h b/webrtc/modules/video_coding/main/interface/video_coding_defines.h index 273c625e56..f36c990cb0 100644 --- a/webrtc/modules/video_coding/main/interface/video_coding_defines.h +++ b/webrtc/modules/video_coding/main/interface/video_coding_defines.h @@ -41,10 +41,6 @@ namespace webrtc { #define VCM_VP8_PAYLOAD_TYPE 120 #define VCM_I420_PAYLOAD_TYPE 124 -enum VCMNackProperties { - kNackHistoryLength = 450 -}; - enum VCMVideoProtection { kProtectionNack, // Both send-side and receive-side kProtectionNackSender, // Send-side only diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer.cc b/webrtc/modules/video_coding/main/source/jitter_buffer.cc index 1a9e29ffde..d8aefe8bf5 100644 --- a/webrtc/modules/video_coding/main/source/jitter_buffer.cc +++ b/webrtc/modules/video_coding/main/source/jitter_buffer.cc @@ -95,12 +95,14 @@ VCMJitterBuffer::VCMJitterBuffer(Clock* clock, nack_mode_(kNoNack), low_rtt_nack_threshold_ms_(-1), high_rtt_nack_threshold_ms_(-1), + nack_seq_nums_internal_(), nack_seq_nums_(), nack_seq_nums_length_(0), + max_nack_list_size_(0), + max_packet_age_to_nack_(0), waiting_for_key_frame_(false) { memset(frame_buffers_, 0, sizeof(frame_buffers_)); memset(receive_statistics_, 0, sizeof(receive_statistics_)); - memset(nack_seq_nums_internal_, -1, sizeof(nack_seq_nums_internal_)); for (int i = 0; i < kStartNumberOfFrames; i++) { frame_buffers_[i] = new VCMFrameBuffer(); @@ -144,11 +146,17 @@ void VCMJitterBuffer::CopyFrom(const VCMJitterBuffer& rhs) { first_packet_ = rhs.first_packet_; last_decoded_state_ = rhs.last_decoded_state_; num_not_decodable_packets_ = rhs.num_not_decodable_packets_; + assert(max_nack_list_size_ == rhs.max_nack_list_size_); + assert(max_packet_age_to_nack_ == rhs.max_packet_age_to_nack_); memcpy(receive_statistics_, rhs.receive_statistics_, sizeof(receive_statistics_)); - memcpy(nack_seq_nums_internal_, rhs.nack_seq_nums_internal_, - sizeof(nack_seq_nums_internal_)); - memcpy(nack_seq_nums_, rhs.nack_seq_nums_, sizeof(nack_seq_nums_)); + nack_seq_nums_internal_.resize(rhs.nack_seq_nums_internal_.size()); + std::copy(rhs.nack_seq_nums_internal_.begin(), + rhs.nack_seq_nums_internal_.end(), + nack_seq_nums_internal_.begin()); + nack_seq_nums_.resize(rhs.nack_seq_nums_.size()); + std::copy(rhs.nack_seq_nums_.begin(), rhs.nack_seq_nums_.end(), + nack_seq_nums_.begin()); for (int i = 0; i < kMaxNumberOfFrames; i++) { if (frame_buffers_[i] != NULL) { delete frame_buffers_[i]; @@ -810,6 +818,20 @@ void VCMJitterBuffer::SetNackMode(VCMNackMode mode, } } +void VCMJitterBuffer::SetNackSettings(size_t max_nack_list_size, + int max_packet_age_to_nack) { + CriticalSectionScoped cs(crit_sect_); + assert(max_packet_age_to_nack >= 0); + if (max_packet_age_to_nack <= 0) { + return; + } + max_nack_list_size_ = max_nack_list_size; + max_packet_age_to_nack_ = max_packet_age_to_nack; + nack_seq_nums_internal_.resize(max_packet_age_to_nack_); + std::fill(nack_seq_nums_internal_.begin(), nack_seq_nums_internal_.end(), -1); + nack_seq_nums_.resize(max_nack_list_size_); +} + VCMNackMode VCMJitterBuffer::nack_mode() const { CriticalSectionScoped cs(crit_sect_); return nack_mode_; @@ -861,8 +883,8 @@ uint16_t* VCMJitterBuffer::CreateNackList(uint16_t* nack_list_size, number_of_seq_num = high_seq_num - low_seq_num; } - if (number_of_seq_num > kNackHistoryLength) { - // NACK list has grown too big, flush and try to restart. + if (number_of_seq_num > max_packet_age_to_nack_) { + // Some of the missing packets are too old. WEBRTC_TRACE(webrtc::kTraceWarning, webrtc::kTraceVideoCoding, VCMId(vcm_id_, receiver_id_), "Nack list too large, try to find a key frame and restart " @@ -872,14 +894,14 @@ uint16_t* VCMJitterBuffer::CreateNackList(uint16_t* nack_list_size, // This NACK size will trigger a key frame request. bool found_key_frame = false; - while (number_of_seq_num > kNackHistoryLength) { + while (number_of_seq_num > max_packet_age_to_nack_) { found_key_frame = RecycleFramesUntilKeyFrame(); if (!found_key_frame) { break; } - // Check if we still have too many packets in the jitter buffer. + // Check if we are still missing too old sequence numbers. low_seq_num = -1; high_seq_num = -1; GetLowHighSequenceNumbers(&low_seq_num, &high_seq_num); @@ -937,16 +959,15 @@ uint16_t* VCMJitterBuffer::CreateNackList(uint16_t* nack_list_size, // Reaching thus far means we are going to update the NACK list // When in hybrid mode, we use the soft NACKing feature. if (nack_mode_ == kNackHybrid) { - nack_seq_nums_index = (*it)->BuildSoftNackList(nack_seq_nums_internal_, - number_of_seq_num, - nack_seq_nums_index, - rtt_ms_); + nack_seq_nums_index = (*it)->BuildSoftNackList( + &nack_seq_nums_internal_[0], number_of_seq_num, + nack_seq_nums_index, rtt_ms_); } else { // Used when the frame is being processed by the decoding thread // don't need to use that info in this loop. - nack_seq_nums_index = (*it)->BuildHardNackList(nack_seq_nums_internal_, - number_of_seq_num, - nack_seq_nums_index); + nack_seq_nums_index = (*it)->BuildHardNackList( + &nack_seq_nums_internal_[0], number_of_seq_num, + nack_seq_nums_index); } } } @@ -979,6 +1000,26 @@ uint16_t* VCMJitterBuffer::CreateNackList(uint16_t* nack_list_size, *nack_list_size = empty_index; } + if (*nack_list_size > max_nack_list_size_) { + // Too many packets missing. Better to skip ahead to the next key frame or + // to request one. + bool found_key_frame = RecycleFramesUntilKeyFrame(); + if (!found_key_frame) { + // Set the last decoded sequence number to current high. + // This is to not get a large nack list again right away. + last_decoded_state_.SetSeqNum(static_cast(high_seq_num)); + // Set to trigger key frame signal. + *nack_list_size = 0xffff; + *list_extended = true; + WEBRTC_TRACE(webrtc::kTraceDebug, webrtc::kTraceVideoCoding, -1, + "\tNo key frame found, request one. nack_list_size: " + "%u", *nack_list_size); + } else { + *nack_list_size = 0; + } + return NULL; + } + if (*nack_list_size > nack_seq_nums_length_) { // Larger list: NACK list was extended since the last call. *list_extended = true; @@ -1006,7 +1047,7 @@ uint16_t* VCMJitterBuffer::CreateNackList(uint16_t* nack_list_size, nack_seq_nums_length_ = *nack_list_size; - return nack_seq_nums_; + return &nack_seq_nums_[0]; } int64_t VCMJitterBuffer::LastDecodedTimestamp() const { diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer.h b/webrtc/modules/video_coding/main/source/jitter_buffer.h index 88631e49a7..e0b7c47cb5 100644 --- a/webrtc/modules/video_coding/main/source/jitter_buffer.h +++ b/webrtc/modules/video_coding/main/source/jitter_buffer.h @@ -12,6 +12,7 @@ #define WEBRTC_MODULES_VIDEO_CODING_MAIN_SOURCE_JITTER_BUFFER_H_ #include +#include #include "webrtc/modules/interface/module_common_types.h" #include "webrtc/modules/video_coding/main/interface/video_coding_defines.h" @@ -49,8 +50,10 @@ struct VCMJitterSample { class VCMJitterBuffer { public: - VCMJitterBuffer(Clock* clock, int vcm_id = -1, int receiver_id = -1, - bool master = true); + VCMJitterBuffer(Clock* clock, + int vcm_id, + int receiver_id, + bool master); virtual ~VCMJitterBuffer(); // Makes |this| a deep copy of |rhs|. @@ -144,6 +147,9 @@ class VCMJitterBuffer { void SetNackMode(VCMNackMode mode, int low_rtt_nack_threshold_ms, int high_rtt_nack_threshold_ms); + void SetNackSettings(size_t max_nack_list_size, + int max_packet_age_to_nack); + // Returns the current NACK mode. VCMNackMode nack_mode() const; @@ -259,9 +265,11 @@ class VCMJitterBuffer { int low_rtt_nack_threshold_ms_; int high_rtt_nack_threshold_ms_; // Holds the internal NACK list (the missing sequence numbers). - int32_t nack_seq_nums_internal_[kNackHistoryLength]; - uint16_t nack_seq_nums_[kNackHistoryLength]; + std::vector nack_seq_nums_internal_; + std::vector nack_seq_nums_; unsigned int nack_seq_nums_length_; + size_t max_nack_list_size_; + int max_packet_age_to_nack_; // Measured in sequence numbers. bool waiting_for_key_frame_; DISALLOW_COPY_AND_ASSIGN(VCMJitterBuffer); diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc b/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc index 7264b5ea26..3d2ad0c140 100644 --- a/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc +++ b/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc @@ -146,10 +146,14 @@ class TestRunningJitterBuffer : public ::testing::Test { virtual void SetUp() { clock_.reset(new SimulatedClock(0)); - jitter_buffer_ = new VCMJitterBuffer(clock_.get()); + max_nack_list_size_ = 250; + oldest_packet_to_nack_ = 450; + jitter_buffer_ = new VCMJitterBuffer(clock_.get(), -1, -1, true); stream_generator = new StreamGenerator(0, 0, clock_->TimeInMilliseconds()); jitter_buffer_->Start(); + jitter_buffer_->SetNackSettings(max_nack_list_size_, + oldest_packet_to_nack_); memset(data_buffer_, 0, kDataBufferSize); } @@ -223,6 +227,8 @@ class TestRunningJitterBuffer : public ::testing::Test { VCMJitterBuffer* jitter_buffer_; StreamGenerator* stream_generator; scoped_ptr clock_; + size_t max_nack_list_size_; + int oldest_packet_to_nack_; uint8_t data_buffer_[kDataBufferSize]; }; @@ -300,18 +306,19 @@ TEST_F(TestJitterBufferNack, TestEmptyPackets) { EXPECT_TRUE(DecodeCompleteFrame()); } -TEST_F(TestJitterBufferNack, TestNackListFull) { +TEST_F(TestJitterBufferNack, TestNackTooOldPackets) { // Insert a key frame and decode it. InsertFrame(kVideoFrameKey); EXPECT_TRUE(DecodeCompleteFrame()); - // Generate and drop |kNackHistoryLength| packets to fill the NACK list. - DropFrame(kNackHistoryLength); + // Drop one frame and insert |kNackHistoryLength| to trigger NACKing a too + // old packet. + DropFrame(1); // Insert a frame which should trigger a recycle until the next key frame. - InsertFrame(kVideoFrameDelta); + InsertFrames(oldest_packet_to_nack_, kVideoFrameDelta); EXPECT_FALSE(DecodeCompleteFrame()); - uint16_t nack_list_length = kNackHistoryLength; + uint16_t nack_list_length = max_nack_list_size_; bool extended; uint16_t* nack_list = jitter_buffer_->CreateNackList(&nack_list_length, &extended); @@ -319,8 +326,47 @@ TEST_F(TestJitterBufferNack, TestNackListFull) { EXPECT_TRUE(nack_list_length == 0xffff && nack_list == NULL); InsertFrame(kVideoFrameDelta); + // Waiting for a key frame. EXPECT_FALSE(DecodeCompleteFrame()); EXPECT_FALSE(DecodeFrame()); + + InsertFrame(kVideoFrameKey); + // The next complete continuous frame isn't a key frame, but we're waiting + // for one. + EXPECT_FALSE(DecodeCompleteFrame()); + // Skipping ahead to the key frame. + EXPECT_TRUE(DecodeFrame()); +} + +TEST_F(TestJitterBufferNack, TestNackListFull) { + // Insert a key frame and decode it. + InsertFrame(kVideoFrameKey); + EXPECT_TRUE(DecodeCompleteFrame()); + + // Generate and drop |kNackHistoryLength| packets to fill the NACK list. + DropFrame(max_nack_list_size_); + // Insert a frame which should trigger a recycle until the next key frame. + InsertFrame(kVideoFrameDelta); + EXPECT_FALSE(DecodeCompleteFrame()); + + uint16_t nack_list_length = max_nack_list_size_; + bool extended; + uint16_t* nack_list = jitter_buffer_->CreateNackList(&nack_list_length, + &extended); + // Verify that the jitter buffer requests a key frame. + EXPECT_TRUE(nack_list_length == 0xffff && nack_list == NULL); + + InsertFrame(kVideoFrameDelta); + // Waiting for a key frame. + EXPECT_FALSE(DecodeCompleteFrame()); + EXPECT_FALSE(DecodeFrame()); + + InsertFrame(kVideoFrameKey); + // The next complete continuous frame isn't a key frame, but we're waiting + // for one. + EXPECT_FALSE(DecodeCompleteFrame()); + // Skipping ahead to the key frame. + EXPECT_TRUE(DecodeFrame()); } TEST_F(TestJitterBufferNack, TestNackBeforeDecode) { diff --git a/webrtc/modules/video_coding/main/source/receiver.cc b/webrtc/modules/video_coding/main/source/receiver.cc index efb3ecdc19..fc5357f829 100644 --- a/webrtc/modules/video_coding/main/source/receiver.cc +++ b/webrtc/modules/video_coding/main/source/receiver.cc @@ -345,6 +345,12 @@ void VCMReceiver::SetNackMode(VCMNackMode nackMode) { } } +void VCMReceiver::SetNackSettings(size_t max_nack_list_size, + int max_packet_age_to_nack) { + jitter_buffer_.SetNackSettings(max_nack_list_size, + max_packet_age_to_nack); +} + VCMNackMode VCMReceiver::NackMode() const { CriticalSectionScoped cs(crit_sect_); return jitter_buffer_.nack_mode(); diff --git a/webrtc/modules/video_coding/main/source/receiver.h b/webrtc/modules/video_coding/main/source/receiver.h index 9d18e247b4..492d616492 100644 --- a/webrtc/modules/video_coding/main/source/receiver.h +++ b/webrtc/modules/video_coding/main/source/receiver.h @@ -59,6 +59,8 @@ class VCMReceiver { // NACK. void SetNackMode(VCMNackMode nackMode); + void SetNackSettings(size_t max_nack_list_size, + int max_packet_age_to_nack); VCMNackMode NackMode() const; VCMNackStatus NackList(uint16_t* nackList, uint16_t* size); diff --git a/webrtc/modules/video_coding/main/source/video_coding_impl.cc b/webrtc/modules/video_coding/main/source/video_coding_impl.cc index 11725ad6ca..25e6c5f603 100644 --- a/webrtc/modules/video_coding/main/source/video_coding_impl.cc +++ b/webrtc/modules/video_coding/main/source/video_coding_impl.cc @@ -68,6 +68,7 @@ _bitStreamBeforeDecoder(NULL), _frameFromFile(), _keyRequestMode(kKeyOnError), _scheduleKeyRequest(false), +max_nack_list_size_(0), _sendCritSect(CriticalSectionWrapper::CreateCriticalSection()), _encoder(), @@ -166,21 +167,27 @@ VideoCodingModuleImpl::Process() } // Packet retransmission requests + // TODO(holmer): Add API for changing Process interval and make sure it's + // disabled when NACK is off. if (_retransmissionTimer.TimeUntilProcess() == 0) { _retransmissionTimer.Processed(); if (_packetRequestCallback != NULL) { - WebRtc_UWord16 nackList[kNackHistoryLength]; - WebRtc_UWord16 length = kNackHistoryLength; - const WebRtc_Word32 ret = NackList(nackList, length); + WebRtc_UWord16 length; + { + CriticalSectionScoped cs(_receiveCritSect); + length = max_nack_list_size_; + } + std::vector nackList(length); + const WebRtc_Word32 ret = NackList(&nackList[0], length); if (ret != VCM_OK && returnValue == VCM_OK) { returnValue = ret; } if (length > 0) { - _packetRequestCallback->ResendPackets(nackList, length); + _packetRequestCallback->ResendPackets(&nackList[0], length); } } } @@ -517,7 +524,6 @@ WebRtc_Word32 VideoCodingModuleImpl::SetVideoProtection(VCMVideoProtection videoProtection, bool enable) { - switch (videoProtection) { @@ -1372,6 +1378,17 @@ int VideoCodingModuleImpl::SetReceiverRobustnessMode( return VCM_OK; } +void VideoCodingModuleImpl::SetNackSettings( + size_t max_nack_list_size, int max_packet_age_to_nack) { + if (max_nack_list_size != 0) { + CriticalSectionScoped cs(_receiveCritSect); + max_nack_list_size_ = max_nack_list_size; + } + _receiver.SetNackSettings(max_nack_list_size, max_packet_age_to_nack); + _dualReceiver.SetNackSettings(max_nack_list_size, + max_packet_age_to_nack); +} + int VideoCodingModuleImpl::StartDebugRecording(const char* file_name_utf8) { CriticalSectionScoped cs(_sendCritSect); _encoderInputFile = fopen(file_name_utf8, "wb"); diff --git a/webrtc/modules/video_coding/main/source/video_coding_impl.h b/webrtc/modules/video_coding/main/source/video_coding_impl.h index e09872e287..24a1f83085 100644 --- a/webrtc/modules/video_coding/main/source/video_coding_impl.h +++ b/webrtc/modules/video_coding/main/source/video_coding_impl.h @@ -58,8 +58,7 @@ enum VCMKeyRequestMode class VideoCodingModuleImpl : public VideoCodingModule { public: - VideoCodingModuleImpl(const WebRtc_Word32 id, - Clock* clock); + VideoCodingModuleImpl(const WebRtc_Word32 id, Clock* clock); virtual ~VideoCodingModuleImpl(); @@ -259,6 +258,10 @@ public: // Set the receiver robustness mode. virtual int SetReceiverRobustnessMode(ReceiverRobustness robustnessMode, DecodeErrors errorMode); + + virtual void SetNackSettings(size_t max_nack_list_size, + int max_packet_age_to_nack); + // Enables recording of debugging information. virtual int StartDebugRecording(const char* file_name_utf8); @@ -295,6 +298,7 @@ private: VCMFrameBuffer _frameFromFile; VCMKeyRequestMode _keyRequestMode; bool _scheduleKeyRequest; + size_t max_nack_list_size_; CriticalSectionWrapper* _sendCritSect; // Critical section for send side VCMGenericEncoder* _encoder; diff --git a/webrtc/modules/video_coding/main/source/video_coding_impl_unittest.cc b/webrtc/modules/video_coding/main/source/video_coding_impl_unittest.cc index 8143fae47c..14878e5c58 100644 --- a/webrtc/modules/video_coding/main/source/video_coding_impl_unittest.cc +++ b/webrtc/modules/video_coding/main/source/video_coding_impl_unittest.cc @@ -46,6 +46,9 @@ class TestVideoCodingModule : public ::testing::Test { false)); EXPECT_EQ(0, vcm_->RegisterExternalDecoder(&decoder_, kUnusedPayloadType, true)); + const size_t kMaxNackListSize = 250; + const int kMaxPacketAgeToNack = 450; + vcm_->SetNackSettings(kMaxNackListSize, kMaxPacketAgeToNack); memset(&settings_, 0, sizeof(settings_)); EXPECT_EQ(0, vcm_->Codec(kVideoCodecVP8, &settings_)); settings_.numberOfSimulcastStreams = kNumberOfStreams; diff --git a/webrtc/modules/video_coding/main/source/video_coding_robustness_unittest.cc b/webrtc/modules/video_coding/main/source/video_coding_robustness_unittest.cc index ccf37eb076..ca7672fe6e 100644 --- a/webrtc/modules/video_coding/main/source/video_coding_robustness_unittest.cc +++ b/webrtc/modules/video_coding/main/source/video_coding_robustness_unittest.cc @@ -37,6 +37,9 @@ class VCMRobustnessTest : public ::testing::Test { vcm_ = VideoCodingModule::Create(0, clock_.get()); ASSERT_TRUE(vcm_ != NULL); ASSERT_EQ(0, vcm_->InitializeReceiver()); + const size_t kMaxNackListSize = 250; + const int kMaxPacketAgeToNack = 450; + vcm_->SetNackSettings(kMaxNackListSize, kMaxPacketAgeToNack); ASSERT_EQ(0, vcm_->RegisterFrameTypeCallback(&frame_type_callback_)); ASSERT_EQ(0, vcm_->RegisterPacketRequestCallback(&request_callback_)); ASSERT_EQ(VCM_OK, vcm_->Codec(kVideoCodecVP8, &video_codec_)); diff --git a/webrtc/modules/video_coding/main/test/jitter_buffer_test.cc b/webrtc/modules/video_coding/main/test/jitter_buffer_test.cc index 69c4c20bc8..538818357b 100644 --- a/webrtc/modules/video_coding/main/test/jitter_buffer_test.cc +++ b/webrtc/modules/video_coding/main/test/jitter_buffer_test.cc @@ -106,7 +106,7 @@ int JitterBufferTest(CmdArgs& args) WebRtc_UWord8 data[1500]; VCMPacket packet(data, size, seqNum, timeStamp, true); - VCMJitterBuffer jb(clock); + VCMJitterBuffer jb(clock, -1, -1, true); seqNum = 1234; timeStamp = 123*90; diff --git a/webrtc/modules/video_coding/main/test/mt_rx_tx_test.cc b/webrtc/modules/video_coding/main/test/mt_rx_tx_test.cc index b99c31ad61..fd959dda0d 100644 --- a/webrtc/modules/video_coding/main/test/mt_rx_tx_test.cc +++ b/webrtc/modules/video_coding/main/test/mt_rx_tx_test.cc @@ -239,7 +239,7 @@ int MTRxTxTest(CmdArgs& args) FecProtectionParams delta_params = protectionCallback.DeltaFecParameters(); FecProtectionParams key_params = protectionCallback.KeyFecParameters(); rtp->SetFecParameters(&delta_params, &key_params); - rtp->SetNACKStatus(nackEnabled ? kNackRtcp : kNackOff); + rtp->SetNACKStatus(nackEnabled ? kNackRtcp : kNackOff, kMaxPacketAgeToNack); vcm->SetChannelParameters((WebRtc_UWord32) bitRate, (WebRtc_UWord8) lossRate, rttMS); diff --git a/webrtc/modules/video_coding/main/test/rtp_player.cc b/webrtc/modules/video_coding/main/test/rtp_player.cc index de062249b1..a52a1cdd9d 100644 --- a/webrtc/modules/video_coding/main/test/rtp_player.cc +++ b/webrtc/modules/video_coding/main/test/rtp_player.cc @@ -8,7 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "rtp_player.h" +#include "webrtc/modules/video_coding/main/test/rtp_player.h" #include #ifdef WIN32 @@ -18,9 +18,10 @@ #include #endif -#include "../source/internal_defines.h" #include "gtest/gtest.h" -#include "rtp_rtcp.h" +#include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h" +#include "webrtc/modules/video_coding/main/source/internal_defines.h" +#include "webrtc/modules/video_coding/main/test/test_util.h" #include "webrtc/system_wrappers/interface/clock.h" using namespace webrtc; @@ -191,7 +192,8 @@ WebRtc_Word32 RTPPlayer::Initialize(const PayloadTypeList* payloadList) _randVec[i] = rand(); } _randVecPos = 0; - WebRtc_Word32 ret = _rtpModule->SetNACKStatus(kNackOff); + WebRtc_Word32 ret = _rtpModule->SetNACKStatus(kNackOff, + kMaxPacketAgeToNack); if (ret < 0) { return -1; diff --git a/webrtc/modules/video_coding/main/test/test_util.h b/webrtc/modules/video_coding/main/test/test_util.h index c5b8545bb0..525f1e3e78 100644 --- a/webrtc/modules/video_coding/main/test/test_util.h +++ b/webrtc/modules/video_coding/main/test/test_util.h @@ -22,6 +22,9 @@ #include "module_common_types.h" #include "testsupport/fileutils.h" +enum { kMaxNackListSize = 250 }; +enum { kMaxPacketAgeToNack = 450 }; + // Class used for passing command line arguments to tests class CmdArgs { diff --git a/webrtc/modules/video_coding/main/test/video_rtp_play.cc b/webrtc/modules/video_coding/main/test/video_rtp_play.cc index bd22168778..cc9194939a 100644 --- a/webrtc/modules/video_coding/main/test/video_rtp_play.cc +++ b/webrtc/modules/video_coding/main/test/video_rtp_play.cc @@ -194,6 +194,7 @@ int RtpPlay(CmdArgs& args) vcm->SetVideoProtection(protectionMethod, protectionEnabled); vcm->SetRenderDelay(renderDelayMs); vcm->SetMinimumPlayoutDelay(minPlayoutDelayMs); + vcm->SetNackSettings(kMaxNackListSize, kMaxPacketAgeToNack); ret = 0; diff --git a/webrtc/modules/video_coding/main/test/video_rtp_play_mt.cc b/webrtc/modules/video_coding/main/test/video_rtp_play_mt.cc index 911eb5c2b3..7e4e065aa1 100644 --- a/webrtc/modules/video_coding/main/test/video_rtp_play_mt.cc +++ b/webrtc/modules/video_coding/main/test/video_rtp_play_mt.cc @@ -84,8 +84,7 @@ int RtpPlayMT(CmdArgs& args, int releaseTestNo, webrtc::VideoCodecType releaseTe protection == kProtectionNack || kProtectionNackFEC)); Clock* clock = Clock::GetRealTimeClock(); - VideoCodingModule* vcm = - VideoCodingModule::Create(1, clock); + VideoCodingModule* vcm = VideoCodingModule::Create(1, clock); RtpDataCallback dataCallback(vcm); std::string rtpFilename; rtpFilename = args.inputFile; @@ -227,6 +226,7 @@ int RtpPlayMT(CmdArgs& args, int releaseTestNo, webrtc::VideoCodecType releaseTe vcm->SetVideoProtection(protection, protectionEnabled); vcm->SetRenderDelay(renderDelayMs); vcm->SetMinimumPlayoutDelay(minPlayoutDelayMs); + vcm->SetNackSettings(kMaxNackListSize, kMaxPacketAgeToNack); EventWrapper& waitEvent = *EventWrapper::Create(); diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc index eeee2f2833..0c137095a6 100644 --- a/webrtc/video_engine/vie_channel.cc +++ b/webrtc/video_engine/vie_channel.cc @@ -123,6 +123,7 @@ ViEChannel::ViEChannel(WebRtc_Word32 channel_id, rtp_rtcp_.reset(RtpRtcp::CreateRtpRtcp(configuration)); vie_receiver_.SetRtpRtcpModule(rtp_rtcp_.get()); + vcm_.SetNackSettings(kMaxNackListSize, kMaxPacketAgeToNack); } WebRtc_Word32 ViEChannel::Init() { @@ -150,7 +151,8 @@ WebRtc_Word32 ViEChannel::Init() { "%s: RTP::SetRTCPStatus failure", __FUNCTION__); } if (paced_sender_) { - if (rtp_rtcp_->SetStorePacketsStatus(true, kNackHistorySize) != 0) { + if (rtp_rtcp_->SetStorePacketsStatus(true, kSendSidePacketHistorySize) != + 0) { WEBRTC_TRACE(kTraceWarning, kTraceVideo, ViEId(engine_id_, channel_id_), "%s:SetStorePacketsStatus failure", __FUNCTION__); } @@ -293,10 +295,10 @@ WebRtc_Word32 ViEChannel::SetSendCodec(const VideoCodec& video_codec, "%s: RTP::SetRTCPStatus failure", __FUNCTION__); } if (nack_method != kNackOff) { - rtp_rtcp->SetStorePacketsStatus(true, kNackHistorySize); - rtp_rtcp->SetNACKStatus(nack_method); + rtp_rtcp->SetStorePacketsStatus(true, kSendSidePacketHistorySize); + rtp_rtcp->SetNACKStatus(nack_method, kMaxPacketAgeToNack); } else if (paced_sender_) { - rtp_rtcp->SetStorePacketsStatus(true, kNackHistorySize); + rtp_rtcp->SetStorePacketsStatus(true, kSendSidePacketHistorySize); } if (fec_enabled) { rtp_rtcp->SetGenericFECStatus(fec_enabled, payload_type_red, @@ -618,7 +620,7 @@ WebRtc_Word32 ViEChannel::ProcessNACKRequest(const bool enable) { "%s: Could not enable NACK, RTPC not on ", __FUNCTION__); return -1; } - if (rtp_rtcp_->SetNACKStatus(nackMethod) != 0) { + if (rtp_rtcp_->SetNACKStatus(nackMethod, kMaxPacketAgeToNack) != 0) { WEBRTC_TRACE(kTraceError, kTraceVideo, ViEId(engine_id_, channel_id_), "%s: Could not set NACK method %d", __FUNCTION__, nackMethod); @@ -626,7 +628,7 @@ WebRtc_Word32 ViEChannel::ProcessNACKRequest(const bool enable) { } WEBRTC_TRACE(kTraceInfo, kTraceVideo, ViEId(engine_id_, channel_id_), "%s: Using NACK method %d", __FUNCTION__, nackMethod); - rtp_rtcp_->SetStorePacketsStatus(true, kNackHistorySize); + rtp_rtcp_->SetStorePacketsStatus(true, kSendSidePacketHistorySize); vcm_.RegisterPacketRequestCallback(this); @@ -636,8 +638,8 @@ WebRtc_Word32 ViEChannel::ProcessNACKRequest(const bool enable) { it != simulcast_rtp_rtcp_.end(); it++) { RtpRtcp* rtp_rtcp = *it; - rtp_rtcp->SetNACKStatus(nackMethod); - rtp_rtcp->SetStorePacketsStatus(true, kNackHistorySize); + rtp_rtcp->SetNACKStatus(nackMethod, kMaxPacketAgeToNack); + rtp_rtcp->SetStorePacketsStatus(true, kSendSidePacketHistorySize); } } else { CriticalSectionScoped cs(rtp_rtcp_cs_.get()); @@ -648,13 +650,13 @@ WebRtc_Word32 ViEChannel::ProcessNACKRequest(const bool enable) { if (paced_sender_ == NULL) { rtp_rtcp->SetStorePacketsStatus(false, 0); } - rtp_rtcp->SetNACKStatus(kNackOff); + rtp_rtcp->SetNACKStatus(kNackOff, kMaxPacketAgeToNack); } vcm_.RegisterPacketRequestCallback(NULL); if (paced_sender_ == NULL) { rtp_rtcp_->SetStorePacketsStatus(false, 0); } - if (rtp_rtcp_->SetNACKStatus(kNackOff) != 0) { + if (rtp_rtcp_->SetNACKStatus(kNackOff, kMaxPacketAgeToNack) != 0) { WEBRTC_TRACE(kTraceError, kTraceVideo, ViEId(engine_id_, channel_id_), "%s: Could not turn off NACK", __FUNCTION__); return -1; diff --git a/webrtc/video_engine/vie_defines.h b/webrtc/video_engine/vie_defines.h index 42c91b5616..7495fcf394 100644 --- a/webrtc/video_engine/vie_defines.h +++ b/webrtc/video_engine/vie_defines.h @@ -75,7 +75,11 @@ enum { kViEMinRenderTimeoutTimeMs = 33 }; enum { kViEDefaultRenderDelayMs = 10 }; // ViERTP_RTCP -enum { kNackHistorySize = 400 }; +enum { kSendSidePacketHistorySize = 600 }; + +// NACK +enum { kMaxPacketAgeToNack = 450 }; // In sequence numbers. +enum { kMaxNackListSize = 250 }; // Id definitions enum {