diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer.cc b/webrtc/modules/video_coding/main/source/jitter_buffer.cc index 53ff5400b9..c01afcc9b1 100644 --- a/webrtc/modules/video_coding/main/source/jitter_buffer.cc +++ b/webrtc/modules/video_coding/main/source/jitter_buffer.cc @@ -911,33 +911,30 @@ 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. + // Make sure we don't add packets which are already too old to be decoded. 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. 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) { - // Push any missing sequence numbers to the NACK list. - for (uint16_t i = latest_received_sequence_number_ + 1; - i < sequence_number; ++i) { - missing_sequence_numbers_.insert(missing_sequence_numbers_.end(), i); - } - if (TooLargeNackList() && !HandleTooLargeNackList()) { - return false; - } - if (MissingTooOldPacket(sequence_number) && - !HandleTooOldPackets(sequence_number)) { - return false; - } - } else { - missing_sequence_numbers_.erase(sequence_number); + } + bool in_order = LatestSequenceNumber(sequence_number, + latest_received_sequence_number_, NULL) == sequence_number; + if (in_order) { + // Push any missing sequence numbers to the NACK list. + for (uint16_t i = latest_received_sequence_number_ + 1; + i < sequence_number; ++i) { + missing_sequence_numbers_.insert(missing_sequence_numbers_.end(), i); } + if (TooLargeNackList() && !HandleTooLargeNackList()) { + return false; + } + if (MissingTooOldPacket(sequence_number) && + !HandleTooOldPackets(sequence_number)) { + return false; + } + } 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()) { @@ -1279,7 +1276,7 @@ FrameList::iterator VCMJitterBuffer::FindOldestCompleteContinuousFrame( // No complete frame no point to continue. return frame_list_.end(); } else if (waiting_for_key_frame_ && - oldest_frame->FrameType() != kVideoFrameKey) { + oldest_frame->FrameType() != kVideoFrameKey) { // We are waiting for a key frame. return frame_list_.end(); } 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 1b2f37d6d0..f9005c94ff 100644 --- a/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc +++ b/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));