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

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

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 #include "webrtc/modules/video_coding/main/source/jitter_buffer.h"
@@ skipping 15 matching lines @@
 #include "webrtc/system_wrappers/interface/event_wrapper.h"
 #include "webrtc/system_wrappers/interface/logging.h"
 #include "webrtc/system_wrappers/interface/metrics.h"
 #include "webrtc/system_wrappers/interface/trace_event.h"
 
 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;

[joachim, 2015/09/03 00:05:01]

I based the timeout on the behaviour of
"EndToEndTest.ReceivesPliAndRecoversWithNack" which requests a keyframe after
about 10 seconds.

[stefan-webrtc, 2015/09/18 14:22:41]

Acknowledged.

+
 typedef std::pair<uint32_t, VCMFrameBuffer*> FrameListPair;
 
 bool IsKeyFrame(FrameListPair pair) {
   return pair.second->FrameType() == kVideoFrameKey;
 }
 
 bool HasNonEmptyState(FrameListPair pair) {
   return pair.second->GetState() != kStateEmpty;
 }
 
@@ skipping 84 matching lines @@
       stats_callback_(NULL),
       incoming_frame_rate_(0),
       incoming_frame_count_(0),
       time_last_incoming_frame_count_(0),
       incoming_bit_count_(0),
       incoming_bit_rate_(0),
       num_consecutive_old_packets_(0),
       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()),
       rtt_ms_(kDefaultRtt),
       nack_mode_(kNoNack),
       low_rtt_nack_threshold_ms_(-1),
       high_rtt_nack_threshold_ms_(-1),
       missing_sequence_numbers_(SequenceNumberLessThan()),
       max_nack_list_size_(0),
       max_packet_age_to_nack_(0),
@@ skipping 56 matching lines @@
   incoming_frame_rate_ = 0;
   incoming_bit_count_ = 0;
   incoming_bit_rate_ = 0;
   time_last_incoming_frame_count_ = clock_->TimeInMilliseconds();
   receive_statistics_ = FrameCounts();
 
   num_consecutive_old_packets_ = 0;
   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.
   waiting_for_completion_.frame_size = 0;
   waiting_for_completion_.timestamp = 0;
   waiting_for_completion_.latest_packet_time = -1;
   first_packet_since_reset_ = true;
   rtt_ms_ = kDefaultRtt;
   last_decoded_state_.Reset();
   last_gof_valid_ = false;
@@ skipping 31 matching lines @@
   return running_;
 }
 
 void VCMJitterBuffer::Flush() {
   CriticalSectionScoped cs(crit_sect_);
   decodable_frames_.Reset(&free_frames_);
   incomplete_frames_.Reset(&free_frames_);
   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());
   waiting_for_completion_.frame_size = 0;
   waiting_for_completion_.timestamp = 0;
   waiting_for_completion_.latest_packet_time = -1;
   first_packet_since_reset_ = true;
   missing_sequence_numbers_.clear();
 }
 
@@ skipping 409 matching lines @@
         buffer_state = kFlushIndicator;
       }
 
       latest_received_sequence_number_ = LatestSequenceNumber(
           latest_received_sequence_number_, packet.seqNum);
     }
   }
 
   // Is the frame already in the decodable list?
   bool continuous = IsContinuous(*frame);
+  bool continuous_for_decoding;
+  if (continuous &&
+      (decode_error_mode_ != kWithErrors || nack_mode_ != kNoNack)) {

[stefan-webrtc, 2015/09/18 14:22:41]

Can you comment on this if-statement? Especially I don't understand why we are
checking decode_error_mode_ or nack_mode_.

[joachim, 2015/09/18 20:23:47]

If "IsContinuous" returns true and decode_error_mode_ is not kWithErrors, the
frame is always continuous for decoding (already checked in "IsContinuous").

Also if nack_mode_ is enabled, NACKs will be used to report missing data to the
sender and the value of "continuous_for_decoding" is not used below. Added
comment about that.

+    continuous_for_decoding = true;
+    time_last_decodable_frame_ = now_ms;
+  } else {
+    continuous_for_decoding = IsContinuousForDecoding(*frame);
+    if (continuous_for_decoding) {
+      time_last_decodable_frame_ = now_ms;
+    }
+  }
   switch (buffer_state) {
     case kGeneralError:
     case kTimeStampError:
     case kSizeError: {
       free_frames_.push_back(frame);
       break;
     }
     case kCompleteSession: {
       if (previous_state != kStateDecodable &&
           previous_state != kStateComplete) {
         CountFrame(*frame);
         if (continuous) {
           // Signal that we have a complete session.
           frame_event_->Set();
         }
       }
       FALLTHROUGH();
     }
     // Note: There is no break here - continuing to kDecodableSession.
     case kDecodableSession: {
       *retransmitted = (frame->GetNackCount() > 0);
       if (continuous) {
         decodable_frames_.InsertFrame(frame);
         FindAndInsertContinuousFrames(*frame);
       } else {
         incomplete_frames_.InsertFrame(frame);
       }
+      if (nack_mode_ == kNoNack && continuous && !continuous_for_decoding) {

[stefan-webrtc, 2015/09/18 14:22:42]

Why do you want to do this only for kDecodableSession? Is it not worth doing for
kIncomplete too?

[joachim, 2015/09/18 20:23:47]

Right, added there too.

+        // 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 (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 kFlushIndicator;
+        }
+      }
       break;
     }
     case kIncomplete: {
       if (frame->GetState() == kStateEmpty &&
           last_decoded_state_.UpdateEmptyFrame(frame)) {
         free_frames_.push_back(frame);
         return kNoError;
       } else {
         incomplete_frames_.InsertFrame(frame);
       }
@@ skipping 12 matching lines @@
       break;
     }
     case kFlushIndicator:
       free_frames_.push_back(frame);
       return kFlushIndicator;
     default: assert(false);
   }
   return buffer_state;
 }
 
 bool VCMJitterBuffer::IsContinuousInState(const VCMFrameBuffer& frame,

[stefan-webrtc, 2015/09/18 14:22:42]

Should this method have a different name? I'd say it rather checks if a frame is
considered decodable or not? Doesn't have to be Continuous

[joachim, 2015/09/18 20:23:47]

As the functionality of "IsContinuousInState" is the same as before, I didn't
want to change the name.

     const VCMDecodingState& decoding_state) const {
-  if (decode_error_mode_ == kWithErrors)
+  if (decode_error_mode_ == kWithErrors) {
     return true;
+  }
+  return IsContinuousInStateForDecoding(frame, decoding_state);

[stefan-webrtc, 2015/09/18 14:22:41]

return decode_error_mode_ == kWithErrors || IsContinuous...();

[joachim, 2015/09/18 20:23:47]

Refactored, so that doesn't apply any more.

+}
+
+bool VCMJitterBuffer::IsContinuousInStateForDecoding(
+    const VCMFrameBuffer& frame, const VCMDecodingState& decoding_state) const {
   // 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
   // completeness).
   return (frame.GetState() == kStateComplete ||
           frame.GetState() == kStateDecodable) &&
          decoding_state.ContinuousFrame(&frame);
 }
 
 bool VCMJitterBuffer::IsContinuous(const VCMFrameBuffer& frame) const {
   if (IsContinuousInState(frame, last_decoded_state_)) {
     return true;
   }
   VCMDecodingState decoding_state;
   decoding_state.CopyFrom(last_decoded_state_);
   for (FrameList::const_iterator it = decodable_frames_.begin();
        it != decodable_frames_.end(); ++it)  {
     VCMFrameBuffer* decodable_frame = it->second;
     if (IsNewerTimestamp(decodable_frame->TimeStamp(), frame.TimeStamp())) {
       break;
     }
     decoding_state.SetState(decodable_frame);
     if (IsContinuousInState(frame, decoding_state)) {
       return true;
     }
   }
   return false;
 }
 
+bool VCMJitterBuffer::IsContinuousForDecoding(

[stefan-webrtc, 2015/09/18 14:22:41]

I don't like how this duplicates the code of IsContinuous()

[joachim, 2015/09/18 20:23:47]

Updated to take a parameter for the two modes.

+    const VCMFrameBuffer& frame) const {
+  if (IsContinuousInStateForDecoding(frame, last_decoded_state_)) {
+    return true;
+  }
+  VCMDecodingState decoding_state;
+  decoding_state.CopyFrom(last_decoded_state_);
+  for (FrameList::const_iterator it = decodable_frames_.begin();
+       it != decodable_frames_.end(); ++it)  {
+    VCMFrameBuffer* decodable_frame = it->second;
+    if (IsNewerTimestamp(decodable_frame->TimeStamp(), frame.TimeStamp())) {
+      break;
+    }
+    decoding_state.SetState(decodable_frame);
+    if (IsContinuousInStateForDecoding(frame, decoding_state)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 void VCMJitterBuffer::FindAndInsertContinuousFrames(
     const VCMFrameBuffer& new_frame) {
   VCMDecodingState decoding_state;
   decoding_state.CopyFrom(last_decoded_state_);
   decoding_state.SetState(&new_frame);
   FindAndInsertContinuousFramesWithState(decoding_state);
 }
 
 void VCMJitterBuffer::FindAndInsertContinuousFramesWithState(
     const VCMDecodingState& original_decoded_state) {
@@ skipping 455 matching lines @@
   }
   // Evaluate if the RTT is higher than |high_rtt_nack_threshold_ms_|, and in
   // that case we don't wait for retransmissions.
   if (high_rtt_nack_threshold_ms_ >= 0 &&
       rtt_ms_ >= high_rtt_nack_threshold_ms_) {
     return false;
   }
   return true;
 }
 }  // namespace webrtc