From fe307e1332b598b4a69dc22cb3dedbd8c9d16355 Mon Sep 17 00:00:00 2001 From: "hclam@chromium.org" Date: Thu, 16 May 2013 21:19:59 +0000 Subject: [PATCH] Add one unit test for NACKing a key frame Adding a test case that wasn't covered. This new test is passing. R=stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/1475004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@4051 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../video_coding/main/source/jitter_buffer.cc | 7 +++++ .../main/source/jitter_buffer_unittest.cc | 26 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer.cc b/webrtc/modules/video_coding/main/source/jitter_buffer.cc index ac3db7461..bef7bb59e 100644 --- a/webrtc/modules/video_coding/main/source/jitter_buffer.cc +++ b/webrtc/modules/video_coding/main/source/jitter_buffer.cc @@ -844,6 +844,9 @@ uint16_t VCMJitterBuffer::EstimatedLowSequenceNumber( assert(frame.GetLowSeqNum() >= 0); if (frame.HaveFirstPacket()) return frame.GetLowSeqNum(); + + // This estimate is not accurate if more than one packet with lower sequence + // number is lost. return frame.GetLowSeqNum() - 1; } @@ -895,6 +898,8 @@ uint16_t* VCMJitterBuffer::GetNackList(uint16_t* nack_list_size, } else { // Skip to the last key frame. If it's incomplete we will start // NACKing it. + // Note that the estimated low sequence number is correct for VP8 + // streams because only the first packet of a key frame is marked. last_decoded_state_.Reset(); DropPacketsFromNackList(EstimatedLowSequenceNumber(**rit)); } @@ -1129,6 +1134,8 @@ bool VCMJitterBuffer::RecycleFramesUntilKeyFrame() { if (it != frame_list_.end() && (*it)->FrameType() == kVideoFrameKey) { // Reset last decoded state to make sure the next frame decoded is a key // frame, and start NACKing from here. + // Note that the estimated low sequence number is correct for VP8 + // streams because only the first packet of a key frame is marked. last_decoded_state_.Reset(); DropPacketsFromNackList(EstimatedLowSequenceNumber(**it)); return true; 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 ec3e59d25..e1fa12f4f 100644 --- a/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc +++ b/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc @@ -1674,6 +1674,32 @@ TEST_F(TestJitterBufferNack, UseNackToRecoverFirstKeyFrame) { EXPECT_EQ(packet.seqNum, list[0]); } +TEST_F(TestJitterBufferNack, UseNackToRecoverFirstKeyFrameSecondInQueue) { + VCMPacket packet; + stream_generator_->Init(0, 0, clock_->TimeInMilliseconds()); + // First frame is delta. + stream_generator_->GenerateFrame(kVideoFrameDelta, 3, 0, + clock_->TimeInMilliseconds()); + EXPECT_EQ(kFirstPacket, InsertPacketAndPop(0)); + // Drop second packet in frame. + ASSERT_TRUE(stream_generator_->PopPacket(&packet, 0)); + EXPECT_EQ(kIncomplete, InsertPacketAndPop(0)); + // Second frame is key. + stream_generator_->GenerateFrame(kVideoFrameKey, 3, 0, + clock_->TimeInMilliseconds() + 10); + EXPECT_EQ(kFirstPacket, InsertPacketAndPop(0)); + // Drop second packet in frame. + EXPECT_EQ(kIncomplete, InsertPacketAndPop(1)); + EXPECT_FALSE(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); + ASSERT_TRUE(list != NULL); + stream_generator_->GetPacket(&packet, 0); + EXPECT_EQ(packet.seqNum, list[0]); +} + TEST_F(TestJitterBufferNack, NormalOperation) { EXPECT_EQ(kNack, jitter_buffer_->nack_mode()); jitter_buffer_->DecodeWithErrors(true);