Request keyframe if too many packets are missing and NACK is disabled.

This allows enabling "EndToEndTest.ReceivesPliAndRecoversWithoutNack".

BUG=webrtc:2250

Review URL: https://codereview.webrtc.org/1211873004

Cr-Commit-Position: refs/heads/master@{#10747}

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<uint32_t, VCMFrameBuffer*> 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;
+    if (nack_mode_ != kNoNack || incomplete_frames_.size() <= 1) {
-  }
+      return false;
-  VCMFrameBuffer* oldest_frame = decodable_frames_.Front();
+    }
-  // If we have exactly one frame in the buffer, release it only if it is
+    oldest_frame = incomplete_frames_.Front();
-  // complete. We know decodable_frames_ is  not empty due to the previous
+    // Frame will only be removed from buffer if it is complete (or decodable).
-  // check.
+    if (oldest_frame->GetState() < kStateComplete) {
-  if (decodable_frames_.size() == 1 && incomplete_frames_.empty()
+      return false;
-      && 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

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());

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, ReceivesPliAndRecoversWithoutNack) {
-TEST_F(EndToEndTest, DISABLED_ReceivesPliAndRecoversWithoutNack) {
   ReceivesPliAndRecovers(0);
 }