Fixes a jitter buffer NACK bug.

If no frame has been decoded the jitter buffer might generate huge
erroneous NACK lists.

Adds a couple of new jitter buffer unittests (some ported from
jitter_buffer_test.cc).

Adds a test to the VCM robustness tests.

BUG=226
TEST=VCMRobustnessTest, TestJitterBufferFull, TestNackListFull, TestNackBeforeDecode, TestNormalOperation

Review URL: http://webrtc-codereview.appspot.com/352002

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

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

@@ -1223,7 +1223,9 @@ VCMJitterBuffer::GetLowHighSequenceNumbers(WebRtc_Word32& lowSeqNum,
     WebRtc_Word32 seqNum = -1;
 
     highSeqNum = -1;
-    lowSeqNum = _lastDecodedState.sequence_num();
+    lowSeqNum = -1;
+    if (!_lastDecodedState.init())
+      lowSeqNum = _lastDecodedState.sequence_num();
 
     // find highest seq numbers
     for (i = 0; i < _maxNumberOfFrames; ++i)

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

@@ -0,0 +1,238 @@
+/*
+ *  Copyright (c) 2011 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include <string.h>
+
+#include "gtest/gtest.h"
+#include "modules/video_coding/main/source/jitter_buffer.h"
+#include "modules/video_coding/main/source/media_opt_util.h"
+#include "modules/video_coding/main/source/mock/fake_tick_time.h"
+#include "modules/video_coding/main/source/packet.h"
+
+namespace webrtc {
+
+class StreamGenerator {
+ public:
+  StreamGenerator(uint16_t start_seq_num, uint32_t start_timestamp,
+                  int64_t current_time)
+      : sequence_number_(start_seq_num),
+        timestamp_(start_timestamp),
+        start_time_(current_time),
+        num_packets_(0),
+        type_(kVideoFrameKey),
+        first_packet_(true) {}
+
+  void GenerateFrame(FrameType type, int num_packets, int64_t current_time) {
+    timestamp_ += 90 * (current_time - start_time_);
+    // Move the sequence number counter if all packets from the previous frame
+    // wasn't collected.
+    sequence_number_ += num_packets_;
+    num_packets_ = num_packets;
+    type_ = type;
+    first_packet_ = true;
+  }
+
+  bool NextPacket(VCMPacket* packet) {
+    if (num_packets_ == 0) {
+      return false;
+    }
+    --num_packets_;
+    if (packet) {
+      packet->seqNum = sequence_number_;
+      packet->timestamp = timestamp_;
+      packet->frameType = type_;
+      packet->isFirstPacket = first_packet_;
+      packet->markerBit = (num_packets_ == 0);
+      if (packet->isFirstPacket)
+        packet->completeNALU = kNaluStart;
+      else if (packet->markerBit)
+        packet->completeNALU = kNaluEnd;
+      else
+        packet->completeNALU = kNaluIncomplete;
+    }
+    ++sequence_number_;
+    first_packet_ = false;
+    return true;
+  }
+
+  int PacketsRemaining() const {
+    return num_packets_;
+  }
+
+ private:
+  uint16_t sequence_number_;
+  uint32_t timestamp_;
+  int64_t start_time_;
+  int num_packets_;
+  FrameType type_;
+  bool first_packet_;
+
+  DISALLOW_COPY_AND_ASSIGN(StreamGenerator);
+};
+
+class TestRunningJitterBuffer : public ::testing::Test {
+ protected:
+  enum { kDataBufferSize = 10 };
+  enum { kDefaultFrameRate = 25 };
+  enum { kDefaultFramePeriodMs = 1000 / kDefaultFrameRate };
+
+  virtual void SetUp() {
+    clock_ = new FakeTickTime(0);
+    jitter_buffer_ = new VCMJitterBuffer(clock_);
+    stream_generator = new StreamGenerator(0, 0,
+                                           clock_->MillisecondTimestamp());
+    jitter_buffer_->Start();
+    memset(data_buffer_, 0, kDataBufferSize);
+  }
+
+  virtual void TearDown() {
+    jitter_buffer_->Stop();
+    delete stream_generator;
+    delete jitter_buffer_;
+    delete clock_;
+  }
+
+  VCMFrameBufferEnum InsertNextPacket() {
+    VCMPacket packet;
+    packet.dataPtr = data_buffer_;
+    VCMEncodedFrame* frame;
+    bool packet_available = stream_generator->NextPacket(&packet);
+    EXPECT_TRUE(packet_available);
+    if (!packet_available)
+      return kStateError;  // Return here to avoid crashes below.
+    EXPECT_EQ(VCM_OK, jitter_buffer_->GetFrame(packet, frame));
+    return jitter_buffer_->InsertPacket(frame, packet);
+  }
+
+  void InsertFrame(FrameType frame_type) {
+    stream_generator->GenerateFrame(frame_type, 1,
+                                    clock_->MillisecondTimestamp());
+    EXPECT_EQ(kFirstPacket, InsertNextPacket());
+    clock_->IncrementDebugClock(kDefaultFramePeriodMs);
+  }
+
+  void InsertFrames(int num_frames) {
+    for (int i = 0; i < num_frames; ++i) {
+      InsertFrame(kVideoFrameDelta);
+    }
+  }
+
+  void DropFrame(int num_packets) {
+    stream_generator->GenerateFrame(kVideoFrameDelta, num_packets,
+                                    clock_->MillisecondTimestamp());
+    clock_->IncrementDebugClock(kDefaultFramePeriodMs);
+  }
+
+  VCMJitterBuffer* jitter_buffer_;
+  StreamGenerator* stream_generator;
+  FakeTickTime* clock_;
+  uint8_t data_buffer_[kDataBufferSize];
+};
+
+class TestNack : public TestRunningJitterBuffer {
+ protected:
+  virtual void SetUp() {
+    TestRunningJitterBuffer::SetUp();
+    jitter_buffer_->SetNackMode(kNackInfinite, -1, -1);
+  }
+
+  virtual void TearDown() {
+    TestRunningJitterBuffer::TearDown();
+  }
+};
+
+TEST_F(TestNack, TestJitterBufferFull) {
+  // Insert a key frame and decode it.
+  InsertFrame(kVideoFrameKey);
+  EXPECT_TRUE(jitter_buffer_->GetCompleteFrameForDecoding(0) != NULL);
+  DropFrame(1);
+  // Fill the jitter buffer.
+  InsertFrames(kMaxNumberOfFrames);
+  // Make sure we can't decode these frames.
+  EXPECT_TRUE(jitter_buffer_->GetCompleteFrameForDecoding(0) == NULL);
+  // This frame will make the jitter buffer recycle frames until a key frame.
+  // Since none is found it will have to wait until the next key frame before
+  // decoding.
+  InsertFrame(kVideoFrameDelta);
+  EXPECT_TRUE(jitter_buffer_->GetCompleteFrameForDecoding(0) == NULL);
+}
+
+TEST_F(TestNack, TestNackListFull) {
+  // Insert a key frame and decode it.
+  InsertFrame(kVideoFrameKey);
+  EXPECT_TRUE(jitter_buffer_->GetCompleteFrameForDecoding(0) != NULL);
+
+  // Generate and drop |kNackHistoryLength| packets to fill the NACK list.
+  DropFrame(kNackHistoryLength);
+  // Insert a frame which should trigger a recycle until the next key frame.
+  InsertFrame(kVideoFrameDelta);
+  EXPECT_TRUE(jitter_buffer_->GetCompleteFrameForDecoding(0) == NULL);
+
+  uint16_t nack_list_length = kNackHistoryLength;
+  bool extended;
+  uint16_t* nack_list = jitter_buffer_->GetNackList(nack_list_length, extended);
+  // Verify that the jitter buffer requests a key frame.
+  EXPECT_TRUE(nack_list_length == 0xffff && nack_list == NULL);
+
+  InsertFrame(kVideoFrameDelta);
+  EXPECT_TRUE(jitter_buffer_->GetCompleteFrameForDecoding(0) == NULL);
+  EXPECT_TRUE(jitter_buffer_->GetFrameForDecoding() == NULL);
+}
+
+TEST_F(TestNack, TestNackBeforeDecode) {
+  DropFrame(10);
+  // Insert a frame and try to generate a NACK list. Shouldn't get one.
+  InsertFrame(kVideoFrameDelta);
+  uint16_t nack_list_size = 0;
+  bool extended = false;
+  uint16_t* list = jitter_buffer_->GetNackList(nack_list_size, extended);
+  // No list generated, and a key frame request is signaled.
+  EXPECT_TRUE(list == NULL);
+  EXPECT_TRUE(nack_list_size == 0xFFFF);
+}
+
+TEST_F(TestNack, TestNormalOperation) {
+  EXPECT_EQ(kNackInfinite, jitter_buffer_->GetNackMode());
+
+  InsertFrame(kVideoFrameKey);
+  EXPECT_TRUE(jitter_buffer_->GetCompleteFrameForDecoding(0) != NULL);
+
+  //  ----------------------------------------------------------------
+  // | 1 | 2 | .. | 8 | 9 | x | 11 | 12 | .. | 19 | x | 21 | .. | 100 |
+  //  ----------------------------------------------------------------
+  stream_generator->GenerateFrame(kVideoFrameKey, 100,
+                                  clock_->MillisecondTimestamp());
+  clock_->IncrementDebugClock(kDefaultFramePeriodMs);
+  EXPECT_EQ(kFirstPacket, InsertNextPacket());
+  // Verify that the frame is incomplete.
+  EXPECT_TRUE(jitter_buffer_->GetCompleteFrameForDecoding(0) == NULL);
+  int i = 2;
+  while (stream_generator->PacketsRemaining() > 1) {
+    if (i % 10 != 0)
+      EXPECT_EQ(kIncomplete, InsertNextPacket());
+    else
+      stream_generator->NextPacket(NULL);  // Drop packet
+    ++i;
+  }
+  EXPECT_EQ(kIncomplete, InsertNextPacket());
+  EXPECT_EQ(0, stream_generator->PacketsRemaining());
+  EXPECT_TRUE(jitter_buffer_->GetCompleteFrameForDecoding(0) == NULL);
+  EXPECT_TRUE(jitter_buffer_->GetFrameForDecoding() == NULL);
+  uint16_t nack_list_size = 0;
+  bool extended = false;
+  uint16_t* list = jitter_buffer_->GetNackList(nack_list_size, extended);
+  // Verify the NACK list.
+  const int kExpectedNackSize = 9;
+  ASSERT_EQ(kExpectedNackSize, nack_list_size);
+  for (i = 0; i < nack_list_size; ++i)
+    EXPECT_EQ((1 + i) * 10, list[i]);
+}
+
+}  // namespace webrtc

src/modules/video_coding/main/source/video_coding_robustness_unittest.cc

@@ -132,6 +132,29 @@ TEST_F(VCMRobustnessTest, TestHardNack) {
   ASSERT_EQ(VCM_OK, vcm_->Decode(0));
 }
 
+TEST_F(VCMRobustnessTest, TestHardNackNoneDecoded) {
+  EXPECT_CALL(request_callback_, ResendPackets(_, _))
+      .Times(0);
+  EXPECT_CALL(frame_type_callback_, FrameTypeRequest(kVideoFrameKey))
+        .Times(1);
+
+  ASSERT_EQ(VCM_OK, vcm_->SetReceiverRobustnessMode(
+      VideoCodingModule::kHardNack,
+      VideoCodingModule::kNoDecodeErrors));
+
+  InsertPacket(3000, 3, true, false, kVideoFrameDelta);
+  InsertPacket(3000, 4, false, false, kVideoFrameDelta);
+  InsertPacket(3000, 5, false, true, kVideoFrameDelta);
+
+  EXPECT_EQ(VCM_FRAME_NOT_READY, vcm_->Decode(0));
+  ASSERT_EQ(VCM_OK, vcm_->Process());
+
+  clock_->IncrementDebugClock(10);
+
+  EXPECT_EQ(VCM_FRAME_NOT_READY, vcm_->Decode(0));
+  ASSERT_EQ(VCM_OK, vcm_->Process());
+}
+
 TEST_F(VCMRobustnessTest, TestDualDecoder) {
   Sequence s1, s2;
   EXPECT_CALL(request_callback_, ResendPackets(_, 1))

src/modules/video_coding/main/source/video_coding_test.gypi

@@ -80,6 +80,7 @@
       'sources': [
         '../interface/mock/mock_vcm_callbacks.h',
         'decoding_state_unittest.cc',
+        'jitter_buffer_unittest.cc',
         'session_info_unittest.cc',
         'video_coding_robustness_unittest.cc',
       ],

src/modules/video_coding/main/test/jitter_buffer_test.cc

@@ -2426,55 +2426,6 @@ int JitterBufferTest(CmdArgs& args)
     TEST(frameOut != NULL);
     TEST(CheckOutFrame(frameOut, packet.sizeBytes, false) == 0);
 
-    jb.Flush();
-
-    // Verify that a key frame is the next frame after the nack list gets full.
-    jb.SetNackMode(kNackInfinite, -1, -1);
-    seqNum += 1;
-    timeStamp += 33 * 90;
-    packet.seqNum = seqNum;
-    packet.timestamp = timeStamp;
-    packet.frameType = kVideoFrameKey;
-    packet.isFirstPacket = true;
-    packet.completeNALU = kNaluComplete;
-    packet.markerBit = true;
-    TEST(frameIn = jb.GetFrame(packet));
-    TEST(kFirstPacket == jb.InsertPacket(frameIn, packet));
-
-    TEST(jb.GetCompleteFrameForDecoding(0) != NULL);
-
-    seqNum += kNackHistoryLength + 1;
-    timeStamp += 33 * 90;
-    packet.seqNum = seqNum;
-    packet.timestamp = timeStamp;
-    packet.frameType = kVideoFrameDelta;
-    packet.isFirstPacket = true;
-    packet.completeNALU = kNaluComplete;
-    packet.markerBit = true;
-    TEST(frameIn = jb.GetFrame(packet));
-    TEST(kFirstPacket == jb.InsertPacket(frameIn, packet));
-
-    TEST(jb.GetCompleteFrameForDecoding(0) == NULL);
-
-    uint16_t nack_list_length = kNackHistoryLength;
-    uint16_t *nack_list;
-    nack_list = jb.GetNackList(nack_list_length, extended);
-    TEST(nack_list_length == 0xffff && nack_list == NULL);
-
-    seqNum += 1;
-    timeStamp += 33 * 90;
-    packet.seqNum = seqNum;
-    packet.timestamp = timeStamp;
-    packet.frameType = kVideoFrameDelta;
-    packet.isFirstPacket = true;
-    packet.completeNALU = kNaluComplete;
-    packet.markerBit = true;
-    TEST(frameIn = jb.GetFrame(packet));
-    TEST(kFirstPacket == jb.InsertPacket(frameIn, packet));
-
-    TEST(jb.GetCompleteFrameForDecoding(0) == NULL);
-    TEST(jb.GetFrameForDecoding() == NULL);
-
     jb.Stop();
 
     printf("DONE !!!\n");