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 126 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_);
+      // Sanity check for RTP timestamp monotonicity.
+      // TODO(brandtr): Consider clearing the entire buffer when we hit these
+      // conditions.
+      if (last_decoded_frame_it_ != frames_.end()) {

[philipel, 2017/04/26 15:26:37]

This should work, right?

if (last_decoded_frame_it_ != frames_.end() &&
    AheadOrAt(last_decoded_frame_timestamp_, frame->timestamp) &&
    next_frame_it_->first <= last_decoded_frame_it_->first) {
  LOG(LS_WARNING) << "Frame with (timestamp:picture_id:spatial_id) ("
                  << frame->timestamp << ":" << frame->picture_id << ":"
                  << static_cast<int>(frame->spatial_layer) << ")"
                  << " sent to decoder after frame with"
                  << " (timestamp:picture_id:spatial_id) ("
                  << last_decoded_frame_timestamp_ << ":"
                  << last_decoded_frame_key.picture_id << ":"
                  << static_cast<int>(
                         last_decoded_frame_key.spatial_layer)
                  << ").";
}

[brandtr, 2017/04/26 15:51:19]

I don't think that would catch this situation:
* frame->timestamp <= last_decoded_frame_timestamp_ (RTP timestamp reordering)
* next_frame_it_->first > last_decoded_frame_it_->first (PictureID correctly
ordered)
?

Note that on lines 156-157, I'm doing neither
* last_decoded_frame_key < frame_key
* last_decoded_frame_key <= frame_key,
since I'm constraining the |picture_id| fields to be equal.

+        const FrameKey& last_decoded_frame_key = last_decoded_frame_it_->first;
+        const FrameKey frame_key(frame->picture_id, frame->spatial_layer);
+
+        const bool frame_is_higher_spatial_layer_of_last_decoded_frame =
+            last_decoded_frame_timestamp_ == frame->timestamp &&
+            last_decoded_frame_key.picture_id == frame_key.picture_id &&
+            last_decoded_frame_key.spatial_layer < frame_key.spatial_layer;
+
+        if (AheadOrAt(last_decoded_frame_timestamp_, frame->timestamp) &&
+            !frame_is_higher_spatial_layer_of_last_decoded_frame) {
+          LOG(LS_WARNING) << "Frame with (timestamp:picture_id:spatial_id) ("
+                          << frame->timestamp << ":" << frame->picture_id << ":"
+                          << static_cast<int>(frame->spatial_layer) << ")"
+                          << " sent to decoder after frame with"
+                          << " (timestamp:picture_id:spatial_id) ("
+                          << last_decoded_frame_timestamp_ << ":"
+                          << last_decoded_frame_key.picture_id << ":"
+                          << static_cast<int>(
+                                 last_decoded_frame_key.spatial_layer)
+                          << ").";
+        }
+      }
       last_decoded_frame_timestamp_ = frame->timestamp;
       *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
@@ skipping 283 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