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

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

Index: webrtc/modules/video_coding/main/source/jitter_buffer.cc
diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer.cc b/webrtc/modules/video_coding/main/source/jitter_buffer.cc
index 49c2325d80a852971edb1dff629b9c5a997e27e7..5357e4a650f34824e93785bbd27563374ca99cab 100644
--- a/webrtc/modules/video_coding/main/source/jitter_buffer.cc
+++ b/webrtc/modules/video_coding/main/source/jitter_buffer.cc
@@ -33,6 +33,10 @@ namespace webrtc {
 // 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 = 10000;
+
 typedef std::pair<uint32_t, VCMFrameBuffer*> FrameListPair;
 
 bool IsKeyFrame(FrameListPair pair) {
@@ -137,6 +141,7 @@ VCMJitterBuffer::VCMJitterBuffer(Clock* clock,
       num_packets_(0),
       num_duplicated_packets_(0),
       num_discarded_packets_(0),
+      time_last_decodable_frame_(-1),
       time_first_packet_ms_(0),
       jitter_estimate_(clock),
       inter_frame_delay_(clock_->TimeInMilliseconds()),
@@ -213,6 +218,7 @@ void VCMJitterBuffer::Start() {
   num_packets_ = 0;
   num_duplicated_packets_ = 0;
   num_discarded_packets_ = 0;
+  time_last_decodable_frame_ = -1;
   time_first_packet_ms_ = 0;
 
   // Start in a non-signaled state.
@@ -264,6 +270,7 @@ void VCMJitterBuffer::Flush() {
   last_decoded_state_.Reset();  // TODO(mikhal): sync reset.
   last_gof_valid_ = false;
   num_consecutive_old_packets_ = 0;
+  time_last_decodable_frame_ = -1;
   // Also reset the jitter and delay estimates
   jitter_estimate_.Reset();
   inter_frame_delay_.Reset(clock_->TimeInMilliseconds());
@@ -692,7 +699,20 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(const VCMPacket& packet,
   }
 
   // Is the frame already in the decodable list?
-  bool continuous = IsContinuous(*frame);
+  bool continuous = IsContinuous(*frame, false);
+  bool continuous_for_decoding;
+  if (continuous &&

[stefan-webrtc, 2015/09/24 07:05:48]

I think I'm starting to see what you want to do here. You want to see if there
are too many non-decodable frames, and in that case flush and request a key
frame?

If you look at line 983, I think we have code that can do that for you. 

I think it's equally valid to request a key frame if the number of non-decodable
frames has grown too large even if NACK is on, or?

[joachim, 2015/09/24 08:52:46]

Thanks, however with NACKs disabled and "decode_error_mode_ = kWithErrors", no
frames will be added to "incomplete_frames_", so
"NonContinuousOrIncompleteDuration" as used in line 983 always returns "0".
Isn't this already handled in "GetNackList" and "UpdateNackList"?

[stefan-webrtc, 2015/09/28 13:49:24]

I see. In that case, would it make sense to only put complete and continuous
frames in decodable_frames_ also for the decode_error_mode_ and allow picking
frames from incomplete_frames_ for decoding when in that mode? I'd prefer if we
could reuse the NonContinuousOrIncompleteDuration code in some way since we
already have that functionality.

[joachim, 2015/11/05 23:48:01]

I updated the code as you suggested, looks a lot cleaner now imho.

+      (decode_error_mode_ != kWithErrors || nack_mode_ != kNoNack)) {
+    // If NACKs are enabled missing data will be handled through them so it's
+    // fine to always assume frames that are continuous for decoding below.
+    continuous_for_decoding = true;
+    time_last_decodable_frame_ = now_ms;
+  } else {
+    continuous_for_decoding = IsContinuous(*frame, true);
+    if (continuous_for_decoding) {
+      time_last_decodable_frame_ = now_ms;
+    }
+  }
   switch (buffer_state) {
     case kGeneralError:
     case kTimeStampError:
@@ -720,6 +740,11 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(const VCMPacket& packet,
       } else {
         incomplete_frames_.InsertFrame(frame);
       }
+      if (continuous &&
+          !continuous_for_decoding &&
+          TooManyDiscontinuousFrames(now_ms)) {
+        return kFlushIndicator;
+      }
       break;
     }
     case kIncomplete: {
@@ -730,6 +755,9 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(const VCMPacket& packet,
       } else {
         incomplete_frames_.InsertFrame(frame);
       }
+      if (TooManyDiscontinuousFrames(now_ms)) {
+        return kFlushIndicator;
+      }
       break;
     }
     case kNoError:
@@ -753,9 +781,11 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(const VCMPacket& packet,
 }
 
 bool VCMJitterBuffer::IsContinuousInState(const VCMFrameBuffer& frame,
-    const VCMDecodingState& decoding_state) const {
-  if (decode_error_mode_ == kWithErrors)
+    const VCMDecodingState& decoding_state, bool ignore_error_mode) const {
+  if (!ignore_error_mode && 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
@@ -765,8 +795,9 @@ bool VCMJitterBuffer::IsContinuousInState(const VCMFrameBuffer& frame,
          decoding_state.ContinuousFrame(&frame);
 }
 
-bool VCMJitterBuffer::IsContinuous(const VCMFrameBuffer& frame) const {
-  if (IsContinuousInState(frame, last_decoded_state_)) {
+bool VCMJitterBuffer::IsContinuous(const VCMFrameBuffer& frame,
+    bool ignore_error_mode) const {
+  if (IsContinuousInState(frame, last_decoded_state_, ignore_error_mode)) {
     return true;
   }
   VCMDecodingState decoding_state;
@@ -778,7 +809,7 @@ bool VCMJitterBuffer::IsContinuous(const VCMFrameBuffer& frame) const {
       break;
     }
     decoding_state.SetState(decodable_frame);
-    if (IsContinuousInState(frame, decoding_state)) {
+    if (IsContinuousInState(frame, decoding_state, ignore_error_mode)) {
       return true;
     }
   }
@@ -812,7 +843,7 @@ void VCMJitterBuffer::FindAndInsertContinuousFramesWithState(
       ++it;
       continue;
     }
-    if (IsContinuousInState(*frame, decoding_state)) {
+    if (IsContinuousInState(*frame, decoding_state, false)) {
       decodable_frames_.InsertFrame(frame);
       incomplete_frames_.erase(it++);
       decoding_state.SetState(frame);
@@ -824,6 +855,19 @@ void VCMJitterBuffer::FindAndInsertContinuousFramesWithState(
   }
 }
 
+bool VCMJitterBuffer::TooManyDiscontinuousFrames(int64_t now_ms) {
+  // If we didn't receive a continuous decodable frame for too long and
+  // can't report through NACKs to the sender, request a keyframe.
+  if (nack_mode_ == kNoNack &&
+      time_last_decodable_frame_ >= 0 &&
+      now_ms - time_last_decodable_frame_ > kMaxDiscontinuousFramesTime) {
+    // Update to current time so we don't request keyframes too often.
+    time_last_decodable_frame_ = now_ms;
+    return true;
+  }
+  return false;
+}
+
 uint32_t VCMJitterBuffer::EstimatedJitterMs() {
   CriticalSectionScoped cs(crit_sect_);
   // Compute RTT multiplier for estimation.