Improve how NACK lists are generated before a frame has been decoded.

BUG=1598

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

git-svn-id: http://webrtc.googlecode.com/svn/trunk@3805 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/modules/video_coding/main/source/jitter_buffer.cc

@@ -911,15 +911,13 @@ bool VCMJitterBuffer::UpdateNackList(uint16_t sequence_number) {
   if (nack_mode_ == kNoNack) {
     return true;
   }
-  // We won't build a NACK list until we have decoded a frame since we probably
-  // won't be able to decode them until we've received a complete key frame.
-  if (!last_decoded_state_.in_initial_state()) {
-    // We have decoded at least one frame.
   // Make sure we don't add packets which are already too old to be decoded.
+  if (!last_decoded_state_.in_initial_state()) {
     latest_received_sequence_number_ = LatestSequenceNumber(
         latest_received_sequence_number_,
         last_decoded_state_.sequence_num(),
         NULL);
+  }
   bool in_order = LatestSequenceNumber(sequence_number,
       latest_received_sequence_number_, NULL) == sequence_number;
   if (in_order) {
@@ -938,7 +936,6 @@ bool VCMJitterBuffer::UpdateNackList(uint16_t sequence_number) {
   } else {
     missing_sequence_numbers_.erase(sequence_number);
   }
-  }
   return true;
 }
 
@@ -952,7 +949,7 @@ bool VCMJitterBuffer::HandleTooLargeNackList() {
   LOG_F(LS_INFO) << "NACK list has grown too large: " <<
       missing_sequence_numbers_.size() << " > " << max_nack_list_size_;
   bool key_frame_found = false;
-  while (missing_sequence_numbers_.size() > max_nack_list_size_) {
+  while (TooLargeNackList()) {
     key_frame_found = RecycleFramesUntilKeyFrame();
   }
   return key_frame_found;
@@ -998,7 +995,7 @@ int64_t VCMJitterBuffer::LastDecodedTimestamp() const {
 
 VCMEncodedFrame* VCMJitterBuffer::GetFrameForDecodingNACK() {
   CleanUpOldOrEmptyFrames();
-  // First look for a complete continuous__ frame.
+  // First look for a complete continuous frame.
   // When waiting for nack, wait for a key frame, if a continuous frame cannot
   // be determined (i.e. initial decoding state).
   if (last_decoded_state_.in_initial_state()) {

webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc

@@ -269,7 +269,7 @@ class TestJitterBufferNack : public TestRunningJitterBuffer {
   }
 };
 
-TEST_F(TestRunningJitterBuffer, TestFull) {
+TEST_F(TestRunningJitterBuffer, Full) {
   // Insert a key frame and decode it.
   EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError);
   EXPECT_TRUE(DecodeCompleteFrame());
@@ -285,7 +285,7 @@ TEST_F(TestRunningJitterBuffer, TestFull) {
   EXPECT_FALSE(DecodeCompleteFrame());
 }
 
-TEST_F(TestRunningJitterBuffer, TestEmptyPackets) {
+TEST_F(TestRunningJitterBuffer, EmptyPackets) {
   // Make sure a frame can get complete even though empty packets are missing.
   stream_generator->GenerateFrame(kVideoFrameKey, 3, 3,
                                   clock_->TimeInMilliseconds());
@@ -373,7 +373,7 @@ TEST_F(TestRunningJitterBuffer, StatisticsTest) {
   EXPECT_EQ(kDefaultBitrateKbps, bitrate);
 }
 
-TEST_F(TestJitterBufferNack, TestEmptyPackets) {
+TEST_F(TestJitterBufferNack, EmptyPackets) {
   // Make sure empty packets doesn't clog the jitter buffer.
   jitter_buffer_->SetNackMode(kNackHybrid, media_optimization::kLowRttNackMs,
                               -1);
@@ -382,7 +382,7 @@ TEST_F(TestJitterBufferNack, TestEmptyPackets) {
   EXPECT_TRUE(DecodeCompleteFrame());
 }
 
-TEST_F(TestJitterBufferNack, TestNackTooOldPackets) {
+TEST_F(TestJitterBufferNack, NackTooOldPackets) {
   // Insert a key frame and decode it.
   EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError);
   EXPECT_TRUE(DecodeCompleteFrame());
@@ -417,7 +417,7 @@ TEST_F(TestJitterBufferNack, TestNackTooOldPackets) {
   EXPECT_TRUE(DecodeFrame());
 }
 
-TEST_F(TestJitterBufferNack, TestNackLargeJitterBuffer) {
+TEST_F(TestJitterBufferNack, NackLargeJitterBuffer) {
   // Insert a key frame and decode it.
   EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError);
   EXPECT_TRUE(DecodeCompleteFrame());
@@ -436,7 +436,7 @@ TEST_F(TestJitterBufferNack, TestNackLargeJitterBuffer) {
   EXPECT_TRUE(DecodeCompleteFrame());
 }
 
-TEST_F(TestJitterBufferNack, TestNackListFull) {
+TEST_F(TestJitterBufferNack, NackListFull) {
   // Insert a key frame and decode it.
   EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError);
   EXPECT_TRUE(DecodeCompleteFrame());
@@ -466,7 +466,7 @@ TEST_F(TestJitterBufferNack, TestNackListFull) {
   EXPECT_TRUE(DecodeFrame());
 }
 
-TEST_F(TestJitterBufferNack, TestNackBeforeDecode) {
+TEST_F(TestJitterBufferNack, NoNackListReturnedBeforeFirstDecode) {
   DropFrame(10);
   // Insert a frame and try to generate a NACK list. Shouldn't get one.
   EXPECT_GE(InsertFrame(kVideoFrameDelta), kNoError);
@@ -480,7 +480,22 @@ TEST_F(TestJitterBufferNack, TestNackBeforeDecode) {
   EXPECT_TRUE(request_key_frame);
 }
 
-TEST_F(TestJitterBufferNack, TestNormalOperation) {
+TEST_F(TestJitterBufferNack, NackListBuiltBeforeFirstDecode) {
+  stream_generator->Init(0, 0, clock_->TimeInMilliseconds());
+  InsertFrame(kVideoFrameKey);
+  stream_generator->GenerateFrame(kVideoFrameDelta, 2, 0,
+                                  clock_->TimeInMilliseconds());
+  stream_generator->NextPacket(NULL);  // Drop packet.
+  EXPECT_EQ(kFirstPacket, InsertPacketAndPop(0));
+  EXPECT_TRUE(DecodeCompleteFrame());
+  uint16_t nack_list_size = 0;
+  bool extended = false;
+  uint16_t* list = jitter_buffer_->GetNackList(&nack_list_size, &extended);
+  EXPECT_EQ(1, nack_list_size);
+  EXPECT_TRUE(list != NULL);
+}
+
+TEST_F(TestJitterBufferNack, NormalOperation) {
   EXPECT_EQ(kNack, jitter_buffer_->nack_mode());
 
   EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError);
@@ -517,7 +532,7 @@ TEST_F(TestJitterBufferNack, TestNormalOperation) {
     EXPECT_EQ((1 + i) * 10, list[i]);
 }
 
-TEST_F(TestJitterBufferNack, TestNormalOperationWrap) {
+TEST_F(TestJitterBufferNack, NormalOperationWrap) {
   bool request_key_frame = false;
   //  -------   ------------------------------------------------------------
   // | 65532 | | 65533 | 65534 | 65535 | x | 1 | .. | 9 | x | 11 |.....| 96 |
@@ -534,7 +549,7 @@ TEST_F(TestJitterBufferNack, TestNormalOperationWrap) {
       EXPECT_EQ(kIncomplete, InsertPacketAndPop(0));
       EXPECT_FALSE(request_key_frame);
     } else {
-      stream_generator->NextPacket(NULL);  // Drop packet
+      stream_generator->NextPacket(NULL);  // Drop packet.
     }
   }
   EXPECT_EQ(kIncomplete, InsertPacketAndPop(0));