From cd64886a2fcdd688d2e2589e35a862d3f542e084 Mon Sep 17 00:00:00 2001 From: "mikhal@webrtc.org" Date: Tue, 3 Jan 2012 23:59:42 +0000 Subject: [PATCH] video_coding: Updating NACK functions naming Review URL: http://webrtc-codereview.appspot.com/329018 git-svn-id: http://webrtc.googlecode.com/svn/trunk@1322 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../video_coding/main/source/frame_buffer.cc | 31 ++++++---------- .../video_coding/main/source/frame_buffer.h | 14 ++++---- .../video_coding/main/source/jitter_buffer.cc | 13 ++++--- .../video_coding/main/source/session_info.cc | 11 +++--- .../video_coding/main/source/session_info.h | 17 +++++---- .../main/source/session_info_unittest.cc | 36 +++++++++---------- 6 files changed, 55 insertions(+), 67 deletions(-) diff --git a/src/modules/video_coding/main/source/frame_buffer.cc b/src/modules/video_coding/main/source/frame_buffer.cc index a276b4687d..5fcf1a41ca 100644 --- a/src/modules/video_coding/main/source/frame_buffer.cc +++ b/src/modules/video_coding/main/source/frame_buffer.cc @@ -208,28 +208,19 @@ VCMFrameBuffer::LatestPacketTimeMs() return _latestPacketTimeMs; } -// Zero out all entries in list up to and including the (first) -// entry equal to _lowSeqNum -WebRtc_Word32 -VCMFrameBuffer::ZeroOutSeqNum(WebRtc_Word32* list, WebRtc_Word32 num) -{ - if (_sessionInfo.ZeroOutSeqNum(list, num) != 0) - { - return -1; - } - return 0; +// Build hard NACK list:Zero out all entries in list up to and including the +// (first) entry equal to _lowSeqNum. +int VCMFrameBuffer::BuildHardNackList(int* list, int num) { + if (_sessionInfo.BuildHardNackList(list, num) != 0) { + return -1; + } + return 0; } -// Zero out all entries in list up to and including the (first) entry equal to -// _lowSeqNum. Hybrid mode: 1. Don't NACK FEC packets 2. Make a smart decision -// on whether to NACK or not - -WebRtc_Word32 -VCMFrameBuffer::ZeroOutSeqNumHybrid(WebRtc_Word32* list, - WebRtc_Word32 num, - WebRtc_UWord32 rttMs) -{ - return _sessionInfo.ZeroOutSeqNumHybrid(list, num, rttMs); +// Build selective NACK list: Create a soft (selective) list of entries to zero +// out up to and including the (first) entry equal to _lowSeqNum. +int VCMFrameBuffer::BuildSoftNackList(int* list, int num, int rttMs) { + return _sessionInfo.BuildSoftNackList(list, num, rttMs); } void diff --git a/src/modules/video_coding/main/source/frame_buffer.h b/src/modules/video_coding/main/source/frame_buffer.h index 3901f58fdd..ce2d4c3faf 100644 --- a/src/modules/video_coding/main/source/frame_buffer.h +++ b/src/modules/video_coding/main/source/frame_buffer.h @@ -66,13 +66,13 @@ public: void SetCountedFrame(bool frameCounted); bool GetCountedFrame() const; - // NACK - // Zero out all entries in list up to and including _lowSeqNum - WebRtc_Word32 ZeroOutSeqNum(WebRtc_Word32* list, WebRtc_Word32 num); - // Hybrid extension: only NACK important packets, discard FEC packets - WebRtc_Word32 ZeroOutSeqNumHybrid(WebRtc_Word32* list, - WebRtc_Word32 num, - WebRtc_UWord32 rttMs); + // NACK - Building the NACK lists. + // Build hard NACK list: Zero out all entries in list up to and including + // _lowSeqNum. + int BuildHardNackList(int* list, int num); + // Build soft NACK list: Zero out only a subset of the packets, discard + // empty packets. + int BuildSoftNackList(int* list, int num, int rttMs); void IncrementNackCount(); WebRtc_Word16 GetNackCount() const; diff --git a/src/modules/video_coding/main/source/jitter_buffer.cc b/src/modules/video_coding/main/source/jitter_buffer.cc index 5932e61110..a3766b489c 100644 --- a/src/modules/video_coding/main/source/jitter_buffer.cc +++ b/src/modules/video_coding/main/source/jitter_buffer.cc @@ -1400,20 +1400,19 @@ VCMJitterBuffer::CreateNackList(WebRtc_UWord16& nackSize, bool& listExtended) (kStateDecoding != state)) { // Reaching thus far means we are going to update the nack list - // When in hybrid mode, we also need to check empty frames, so as - // not to add empty packets to the nack list + // When in hybrid mode, we use the soft NACKing feature. if (_nackMode == kNackHybrid) { - _frameBuffers[i]->ZeroOutSeqNumHybrid(_NACKSeqNumInternal, - numberOfSeqNum, - _rttMs); + _frameBuffers[i]->BuildSoftNackList(_NACKSeqNumInternal, + numberOfSeqNum, + _rttMs); } else { // Used when the frame is being processed by the decoding thread // don't need to use that info in this loop. - _frameBuffers[i]->ZeroOutSeqNum(_NACKSeqNumInternal, - numberOfSeqNum); + _frameBuffers[i]->BuildHardNackList(_NACKSeqNumInternal, + numberOfSeqNum); } } } diff --git a/src/modules/video_coding/main/source/session_info.cc b/src/modules/video_coding/main/source/session_info.cc index 0721118959..66b1dc2779 100644 --- a/src/modules/video_coding/main/source/session_info.cc +++ b/src/modules/video_coding/main/source/session_info.cc @@ -347,8 +347,8 @@ int VCMSessionInfo::MakeDecodable() { return return_length; } -int VCMSessionInfo::ZeroOutSeqNum(int* seq_num_list, - int seq_num_list_length) { +int VCMSessionInfo::BuildHardNackList(int* seq_num_list, + int seq_num_list_length) { if (NULL == seq_num_list || seq_num_list_length < 1) { return -1; } @@ -387,10 +387,9 @@ int VCMSessionInfo::ZeroOutSeqNum(int* seq_num_list, return 0; } -// TODO(mikhal): Rename function. -int VCMSessionInfo::ZeroOutSeqNumHybrid(int* seq_num_list, - int seq_num_list_length, - int rtt_ms) { +int VCMSessionInfo::BuildSoftNackList(int* seq_num_list, + int seq_num_list_length, + int rtt_ms) { if (NULL == seq_num_list || seq_num_list_length < 1) { return -1; } diff --git a/src/modules/video_coding/main/source/session_info.h b/src/modules/video_coding/main/source/session_info.h index 555afb8e6d..0dc842f38a 100644 --- a/src/modules/video_coding/main/source/session_info.h +++ b/src/modules/video_coding/main/source/session_info.h @@ -25,14 +25,17 @@ class VCMSessionInfo { VCMSessionInfo(); void UpdateDataPointers(ptrdiff_t address_delta); - int ZeroOutSeqNum(int* seq_num_list, - int seq_num_list_length); + // NACK - Building the NACK lists. + // Build hard NACK list: Zero out all entries in list up to and including + // _lowSeqNum. + int BuildHardNackList(int* seq_num_list, + int seq_num_list_length); - // Hybrid version: Zero out seq num for NACK list - // Selectively NACK packets. - int ZeroOutSeqNumHybrid(int* seq_num_list, - int seq_num_list_length, - int rtt_ms); + // Build soft NACK list: Zero out only a subset of the packets, discard + // empty packets. + int BuildSoftNackList(int* seq_num_list, + int seq_num_list_length, + int rtt_ms); void Reset(); int InsertPacket(const VCMPacket& packet, uint8_t* frame_buffer, diff --git a/src/modules/video_coding/main/source/session_info_unittest.cc b/src/modules/video_coding/main/source/session_info_unittest.cc index 3ecce44142..b72d61c98a 100644 --- a/src/modules/video_coding/main/source/session_info_unittest.cc +++ b/src/modules/video_coding/main/source/session_info_unittest.cc @@ -103,7 +103,7 @@ class TestNalUnits : public TestSessionInfo { } }; -class TestZeroOutSeqNum : public TestSessionInfo { +class TestNackList : public TestSessionInfo { protected: enum { kMaxSeqNumListLength = 30 }; @@ -776,7 +776,7 @@ TEST_F(TestNalUnits, ReorderWrapLosses) { EXPECT_EQ(2, session_.packets_not_decodable()); } -TEST_F(TestZeroOutSeqNum, NoLosses) { +TEST_F(TestNackList, NoLosses) { uint16_t low = 0xFFFF - 5; packet_.seqNum = low; @@ -804,20 +804,19 @@ TEST_F(TestZeroOutSeqNum, NoLosses) { EXPECT_EQ(10 * kPacketBufferSize, session_.SessionLength()); BuildSeqNumList(low, packet_.seqNum); - EXPECT_EQ(0, session_.ZeroOutSeqNum(seq_num_list_, seq_num_list_length_)); + EXPECT_EQ(0, session_.BuildHardNackList(seq_num_list_, seq_num_list_length_)); EXPECT_EQ(false, session_.session_nack()); SCOPED_TRACE("Calling VerifyAll"); VerifyAll(-1); BuildSeqNumList(low, packet_.seqNum); - EXPECT_EQ(0, session_.ZeroOutSeqNumHybrid(seq_num_list_, - seq_num_list_length_, - 60)); + EXPECT_EQ(0, session_.BuildSoftNackList(seq_num_list_, seq_num_list_length_, + 60)); SCOPED_TRACE("Calling VerifyAll"); VerifyAll(-1); } -TEST_F(TestZeroOutSeqNum, FiveLossesSpreadOut) { +TEST_F(TestNackList, FiveLossesSpreadOut) { uint16_t low = 0xFFFF - 5; packet_.seqNum = low; @@ -841,7 +840,7 @@ TEST_F(TestZeroOutSeqNum, FiveLossesSpreadOut) { EXPECT_EQ(5 * kPacketBufferSize, session_.SessionLength()); BuildSeqNumList(low, packet_.seqNum); - EXPECT_EQ(0, session_.ZeroOutSeqNum(seq_num_list_, seq_num_list_length_)); + EXPECT_EQ(0, session_.BuildHardNackList(seq_num_list_, seq_num_list_length_)); for (int i = 0; i < seq_num_list_length_; ++i) { if (i % 2) EXPECT_EQ(static_cast(low + i), seq_num_list_[i]); @@ -850,9 +849,8 @@ TEST_F(TestZeroOutSeqNum, FiveLossesSpreadOut) { } BuildSeqNumList(low, packet_.seqNum); - EXPECT_EQ(0, session_.ZeroOutSeqNumHybrid(seq_num_list_, - seq_num_list_length_, - 60)); + EXPECT_EQ(0, session_.BuildSoftNackList(seq_num_list_, seq_num_list_length_, + 60)); EXPECT_EQ(true, session_.session_nack()); for (int i = 0; i < seq_num_list_length_; ++i) { if (i % 2) @@ -862,7 +860,7 @@ TEST_F(TestZeroOutSeqNum, FiveLossesSpreadOut) { } } -TEST_F(TestZeroOutSeqNum, FirstAndLastLost) { +TEST_F(TestNackList, FirstAndLastLost) { uint16_t low = 0xFFFF; packet_.seqNum = low + 1; @@ -874,22 +872,21 @@ TEST_F(TestZeroOutSeqNum, FirstAndLastLost) { EXPECT_EQ(kPacketBufferSize, session_.SessionLength()); BuildSeqNumList(low, packet_.seqNum + 1); - EXPECT_EQ(0, session_.ZeroOutSeqNum(seq_num_list_, seq_num_list_length_)); + EXPECT_EQ(0, session_.BuildHardNackList(seq_num_list_, seq_num_list_length_)); EXPECT_EQ(0xFFFF, seq_num_list_[0]); EXPECT_EQ(-1, seq_num_list_[1]); EXPECT_EQ(1, seq_num_list_[2]); BuildSeqNumList(low, packet_.seqNum + 1); - EXPECT_EQ(0, session_.ZeroOutSeqNumHybrid(seq_num_list_, - seq_num_list_length_, - 60)); + EXPECT_EQ(0, session_.BuildSoftNackList(seq_num_list_,seq_num_list_length_, + 60)); EXPECT_EQ(true, session_.session_nack()); EXPECT_EQ(0xFFFF, seq_num_list_[0]); EXPECT_EQ(-1, seq_num_list_[1]); EXPECT_EQ(1, seq_num_list_[2]); } -TEST_F(TestZeroOutSeqNum, LostAllButEmptyPackets) { +TEST_F(TestNackList, LostAllButEmptyPackets) { uint16_t low = 0; packet_.seqNum = low + 1; packet_.isFirstPacket = false; @@ -909,9 +906,8 @@ TEST_F(TestZeroOutSeqNum, LostAllButEmptyPackets) { EXPECT_EQ(0, session_.SessionLength()); BuildSeqNumList(low, packet_.seqNum + 1); - EXPECT_EQ(0, session_.ZeroOutSeqNumHybrid(seq_num_list_, - seq_num_list_length_, - 60)); + EXPECT_EQ(0, session_.BuildSoftNackList(seq_num_list_, seq_num_list_length_, + 60)); EXPECT_EQ(true, session_.session_nack()); EXPECT_EQ(0, seq_num_list_[0]); EXPECT_EQ(-1, seq_num_list_[1]);