diff --git a/webrtc/modules/video_coding/jitter_buffer.cc b/webrtc/modules/video_coding/jitter_buffer.cc index 15195dbfdc..d44d2b68f1 100644 --- a/webrtc/modules/video_coding/jitter_buffer.cc +++ b/webrtc/modules/video_coding/jitter_buffer.cc @@ -38,6 +38,10 @@ static const uint32_t kSsCleanupIntervalSec = 60; // Use this rtt if no value has been reported. static const int64_t kDefaultRtt = 200; +// Request a keyframe if no continuous frame has been received for this +// number of milliseconds and NACKs are disabled. +static const int64_t kMaxDiscontinuousFramesTime = 1000; + typedef std::pair FrameListPair; bool IsKeyFrame(FrameListPair pair) { @@ -528,16 +532,25 @@ bool VCMJitterBuffer::NextMaybeIncompleteTimestamp(uint32_t* timestamp) { CleanUpOldOrEmptyFrames(); + VCMFrameBuffer* oldest_frame; if (decodable_frames_.empty()) { - return false; - } - VCMFrameBuffer* oldest_frame = decodable_frames_.Front(); - // If we have exactly one frame in the buffer, release it only if it is - // complete. We know decodable_frames_ is not empty due to the previous - // check. - if (decodable_frames_.size() == 1 && incomplete_frames_.empty() - && oldest_frame->GetState() != kStateComplete) { - return false; + if (nack_mode_ != kNoNack || incomplete_frames_.size() <= 1) { + return false; + } + oldest_frame = incomplete_frames_.Front(); + // Frame will only be removed from buffer if it is complete (or decodable). + if (oldest_frame->GetState() < kStateComplete) { + return false; + } + } else { + oldest_frame = decodable_frames_.Front(); + // If we have exactly one frame in the buffer, release it only if it is + // complete. We know decodable_frames_ is not empty due to the previous + // check. + if (decodable_frames_.size() == 1 && incomplete_frames_.empty() + && oldest_frame->GetState() != kStateComplete) { + return false; + } } *timestamp = oldest_frame->TimeStamp(); @@ -779,6 +792,11 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(const VCMPacket& packet, FindAndInsertContinuousFrames(*frame); } else { incomplete_frames_.InsertFrame(frame); + // If NACKs are enabled, keyframes are triggered by |GetNackList|. + if (nack_mode_ == kNoNack && NonContinuousOrIncompleteDuration() > + 90 * kMaxDiscontinuousFramesTime) { + return kFlushIndicator; + } } break; } @@ -789,6 +807,11 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(const VCMPacket& packet, return kNoError; } else { incomplete_frames_.InsertFrame(frame); + // If NACKs are enabled, keyframes are triggered by |GetNackList|. + if (nack_mode_ == kNoNack && NonContinuousOrIncompleteDuration() > + 90 * kMaxDiscontinuousFramesTime) { + return kFlushIndicator; + } } break; } @@ -814,8 +837,6 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(const VCMPacket& packet, bool VCMJitterBuffer::IsContinuousInState(const VCMFrameBuffer& frame, const VCMDecodingState& decoding_state) const { - if (decode_error_mode_ == kWithErrors) - return true; // Is this frame (complete or decodable) and continuous? // kStateDecodable will never be set when decode_error_mode_ is false // as SessionInfo determines this state based on the error mode (and frame diff --git a/webrtc/modules/video_coding/jitter_buffer_unittest.cc b/webrtc/modules/video_coding/jitter_buffer_unittest.cc index 52151ea9aa..116bf088c1 100644 --- a/webrtc/modules/video_coding/jitter_buffer_unittest.cc +++ b/webrtc/modules/video_coding/jitter_buffer_unittest.cc @@ -1834,6 +1834,12 @@ TEST_F(TestBasicJitterBuffer, ExceedNumOfFrameWithSeqNumWrap) { // -------------------------------------------------------------- // |<-----------delta frames------------->|<------key frames----->| + // Make sure the jitter doesn't request a keyframe after too much non- + // decodable frames. + jitter_buffer_->SetNackMode(kNack, -1, -1); + jitter_buffer_->SetNackSettings(kMaxNumberOfFrames, + kMaxNumberOfFrames, 0); + int loop = 0; seq_num_ = 65485; uint32_t first_key_frame_timestamp = 0; @@ -2131,6 +2137,11 @@ TEST_F(TestBasicJitterBuffer, NextFrameWhenIncomplete) { } TEST_F(TestRunningJitterBuffer, Full) { + // Make sure the jitter doesn't request a keyframe after too much non- + // decodable frames. + jitter_buffer_->SetNackMode(kNack, -1, -1); + jitter_buffer_->SetNackSettings(kMaxNumberOfFrames, + kMaxNumberOfFrames, 0); // Insert a key frame and decode it. EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError); EXPECT_TRUE(DecodeCompleteFrame()); diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index e541d370d8..365995515b 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -1002,8 +1002,7 @@ TEST_F(EndToEndTest, ReceivesPliAndRecoversWithNack) { ReceivesPliAndRecovers(1000); } -// TODO(pbos): Enable this when 2250 is resolved. -TEST_F(EndToEndTest, DISABLED_ReceivesPliAndRecoversWithoutNack) { +TEST_F(EndToEndTest, ReceivesPliAndRecoversWithoutNack) { ReceivesPliAndRecovers(0); }