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 625 matching lines @@
 
   if (buffer_state > 0) {
     incoming_bit_count_ += packet.sizeBytes << 3;
     if (first_packet_since_reset_) {
       latest_received_sequence_number_ = packet.seqNum;
       first_packet_since_reset_ = false;
     } else {
       if (IsPacketRetransmitted(packet)) {
         frame->IncrementNackCount();
       }
-      if (!UpdateNackList(packet.seqNum) &&
+      if (!CoverSequenceUpTo(packet.seqNum) &&
           packet.frameType != kVideoFrameKey) {
         buffer_state = kFlushIndicator;
       }
 
       latest_received_sequence_number_ = LatestSequenceNumber(
           latest_received_sequence_number_, packet.seqNum);
     }
   }
 
   // Is the frame already in the decodable list?
@@ skipping 52 matching lines @@
     case kFlushIndicator:
       free_frames_.push_back(frame);
       return kFlushIndicator;
     default: assert(false);
   }
   return buffer_state;
 }
 
 bool VCMJitterBuffer::IsContinuousInState(const VCMFrameBuffer& frame,
     const VCMDecodingState& decoding_state) const {
   if (decode_error_mode_ == kWithErrors)

[stefan-webrtc, 2015/07/01 13:41:02]

I think you have to take a look at how this stuff works to make a proper fix to
this bug.

     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
   // completeness).
   return (frame.GetState() == kStateComplete ||
           frame.GetState() == kStateDecodable) &&
          decoding_state.ContinuousFrame(&frame);
 }
 
@@ skipping 28 matching lines @@
 void VCMJitterBuffer::FindAndInsertContinuousFramesWithState(
     const VCMDecodingState& original_decoded_state) {
   // Copy original_decoded_state so we can move the state forward with each
   // decodable frame we find.
   VCMDecodingState decoding_state;
   decoding_state.CopyFrom(original_decoded_state);
 
   // When temporal layers are available, we search for a complete or decodable
   // frame until we hit one of the following:
   // 1. Continuous base or sync layer.
   // 2. The end of the list was reached.

[stefan-webrtc, 2015/07/01 13:41:02]

Note how this method takes temporal layers into account. We don't want to lose
that ability just because we have disabled NACK.

   for (FrameList::iterator it = incomplete_frames_.begin();
        it != incomplete_frames_.end();)  {
     VCMFrameBuffer* frame = it->second;
     if (IsNewerTimestamp(original_decoded_state.time_stamp(),
                          frame->TimeStamp())) {
       ++it;
       continue;
     }
     if (IsContinuousInState(*frame, decoding_state)) {
       decodable_frames_.InsertFrame(frame);
@@ skipping 153 matching lines @@
 }
 
 VCMFrameBuffer* VCMJitterBuffer::NextFrame() const {
   if (!decodable_frames_.empty())
     return decodable_frames_.Front();
   if (!incomplete_frames_.empty())
     return incomplete_frames_.Front();
   return NULL;
 }
 
-bool VCMJitterBuffer::UpdateNackList(uint16_t sequence_number) {
-  if (nack_mode_ == kNoNack) {
-    return true;
-  }
+bool VCMJitterBuffer::CoverSequenceUpTo(uint16_t sequence_number) {
   // Make sure we don't add packets which are already too old to be decoded.
   if (!last_decoded_state_.in_initial_state()) {
     latest_received_sequence_number_ = LatestSequenceNumber(
         latest_received_sequence_number_,
         last_decoded_state_.sequence_num());
   }
   if (IsNewerSequenceNumber(sequence_number,
                             latest_received_sequence_number_)) {
+    if (nack_mode_ == kNoNack) {
+      // Packets missing but NACK disabled, request key frame.

[stefan-webrtc, 2015/07/01 13:41:02]

This isn't a good way of handling lost packets since a lost packet doesn't
necessarily imply a decodable frame sequence.

Some decoders (vp8 and h.264) can decode frames with missing packets and damaged
references in a (sometimes) satisfactory way. Therefore we have traditionally
left the decision to request a key frame or not to the decoder (in non-NACK
mode, that is). Other ways than a key frame can also be used for the recovery of
a loss.

Some frame sequences may use layering to achieve robustness, so that for
instance base layer frames can be decoded even if higher layers are lost. This
solution will break that functionality when NACK is disabled.

See the other places in the code I commented on.

+      return false;
+    }
     // Push any missing sequence numbers to the NACK list.
     for (uint16_t i = latest_received_sequence_number_ + 1;
          IsNewerSequenceNumber(sequence_number, i); ++i) {
       missing_sequence_numbers_.insert(missing_sequence_numbers_.end(), i);
       TRACE_EVENT_INSTANT1(TRACE_DISABLED_BY_DEFAULT("webrtc_rtp"), "AddNack",
                            "seqnum", i);
     }
     if (TooLargeNackList() && !HandleTooLargeNackList()) {
       LOG(LS_WARNING) << "Requesting key frame due to too large NACK list.";
       return false;
     }
     if (MissingTooOldPacket(sequence_number) &&
         !HandleTooOldPackets(sequence_number)) {
       LOG(LS_WARNING) << "Requesting key frame due to missing too old packets";
       return false;
     }
-  } else {
+  } else if (nack_mode_ != kNoNack) {
     missing_sequence_numbers_.erase(sequence_number);
     TRACE_EVENT_INSTANT1(TRACE_DISABLED_BY_DEFAULT("webrtc_rtp"), "RemoveNack",
                          "seqnum", sequence_number);
   }
   return true;
 }
 
 bool VCMJitterBuffer::TooLargeNackList() const {
   return missing_sequence_numbers_.size() > max_nack_list_size_;
 }
@@ skipping 234 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