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

webrtc/modules/video_coding/frame_buffer2.cc

Index: webrtc/modules/video_coding/frame_buffer2.cc
diff --git a/webrtc/modules/video_coding/frame_buffer2.cc b/webrtc/modules/video_coding/frame_buffer2.cc
index acc1bb83c5a0e448e030d42b98eab5d25315b0a1..845a8cd791bebaedd738883dc95b907412f054d0 100644
--- a/webrtc/modules/video_coding/frame_buffer2.cc
+++ b/webrtc/modules/video_coding/frame_buffer2.cc
@@ -43,6 +43,7 @@ FrameBuffer::FrameBuffer(Clock* clock,
       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),
@@ -144,7 +145,18 @@ FrameBuffer::ReturnReason FrameBuffer::NextFrame(
 
       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;
     }