Add warning about timestamp non-monotonicity in the frame buffer.

webrtc/modules/video_coding/frame_buffer2.cc

 /*
  *  Copyright (c) 2016 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.
  */
 
@@ skipping 25 matching lines @@
 
 FrameBuffer::FrameBuffer(Clock* clock,
                          VCMJitterEstimator* jitter_estimator,
                          VCMTiming* timing,
                          VCMReceiveStatisticsCallback* stats_callback)
     : clock_(clock),
       new_continuous_frame_event_(false, false),
       jitter_estimator_(jitter_estimator),
       timing_(timing),
       inter_frame_delay_(clock_->TimeInMilliseconds()),
+      last_decoded_frame_picture_id_(-1),
       last_decoded_frame_it_(frames_.end()),
       last_continuous_frame_it_(frames_.end()),
       num_frames_history_(0),
       num_frames_buffered_(0),
       stopped_(false),
       protection_mode_(kProtectionNack),
       stats_callback_(stats_callback) {}
 
 FrameBuffer::~FrameBuffer() {}
 
@@ skipping 81 matching lines @@
 
         float rtt_mult = protection_mode_ == kProtectionNackFEC ? 0.0 : 1.0;
         timing_->SetJitterDelay(jitter_estimator_->GetJitterEstimate(rtt_mult));
         timing_->UpdateCurrentDelay(frame->RenderTime(), now_ms);
       }
 
       UpdateJitterDelay();
 
       PropagateDecodability(next_frame_it_->second);
       AdvanceLastDecodedFrame(next_frame_it_);
+      if (last_decoded_frame_picture_id_ != -1 &&
+          AheadOrAt(last_decoded_frame_timestamp_, frame->timestamp)) {
+        LOG(LS_WARNING) << "Sending frame "
+                        << "(timestamp = " << frame->timestamp
+                        << ", picture_id = " << frame->picture_id
+                        << ") to decoder when last decoded frame was "
+                        << "(timestamp = " << last_decoded_frame_timestamp_
+                        << ", picture_id = " << last_decoded_frame_picture_id_
+                        << ").";

[stefan-webrtc, 2017/04/26 09:04:57]

Should we drop the frame if this is the case, or is it right to decode it,
hoping that the timestamp has jumped? Maybe the best thing to do is to reset the
jitter buffer?

[brandtr, 2017/04/26 09:43:51]

In order to avoid distortions due to incorrect decode order, it might be safest
to reset the jitter buffer. This basically means trusting RTP timestamps over
picture_id, when it comes to the final order check. My rationale is that the RTP
timestamp order should correlate perfectly with the encode order, and therefore
the decode order, since we don't (?) support B-frames (*). This might be a naive
view of the system however, so if you know more I would be interested in
learning. Also, of course, this assumes a sane sender, which does not generate
random jumps in RTP timestamps...

(*) Looking through the different packetization RFCs, I noticed that H.264 has a
"Decoding Order Number" in some particular packetization mode. No idea if this
is something that we support?.
https://tools.ietf.org/html/rfc6184#section-5.5

[stefan-webrtc, 2017/04/26 10:44:49]

We have no plans of supporting interleaved packetization, i.e., STAB-B and MTAP.

I agree that making a reset when this happens is likely a good thing to do. WDYT
Philip?

[philipel, 2017/04/26 11:05:31]

I think a reset might be the right thing to do, not sure but I will think on it.

+      }
       last_decoded_frame_timestamp_ = frame->timestamp;
+      last_decoded_frame_picture_id_ = frame->picture_id;
       *frame_out = std::move(frame);
       return kFrameFound;
     }
   }
 
   if (latest_return_time_ms - now_ms > 0) {
     // If |next_frame_it_ == frames_.end()| and there is still time left, it
     // means that the frame buffer was cleared as the thread in this function
     // was waiting to acquire |crit_| in order to return. Wait for the
     // remaining time and then return.
@@ skipping 282 matching lines @@
   frames_.clear();
   last_decoded_frame_it_ = frames_.end();
   last_continuous_frame_it_ = frames_.end();
   next_frame_it_ = frames_.end();
   num_frames_history_ = 0;
   num_frames_buffered_ = 0;
 }
 
 }  // namespace video_coding
 }  // namespace webrtc