Validate references of frames inserted into FrameBuffer2.

BUG=chromium:730603

Review-Url: https://codereview.webrtc.org/2937243002
Cr-Commit-Position: refs/heads/master@{#18614}

webrtc/modules/video_coding/frame_buffer2.cc

@@ -238,6 +238,22 @@ void FrameBuffer::Stop() {
   new_continuous_frame_event_.Set();
 }
 
+bool FrameBuffer::ValidReferences(const FrameObject& frame) const {
+  for (size_t i = 0; i < frame.num_references; ++i) {
+    if (AheadOrAt(frame.references[i], frame.picture_id))
+      return false;
+    for (size_t j = i + 1; j < frame.num_references; ++j) {
+      if (frame.references[i] == frame.references[j])
+        return false;
+    }
+  }
+
+  if (frame.inter_layer_predicted && frame.spatial_layer == 0)
+    return false;
+
+  return true;
+}
+
 void FrameBuffer::UpdatePlayoutDelays(const FrameObject& frame) {
   TRACE_EVENT0("webrtc", "FrameBuffer::UpdatePlayoutDelays");
   PlayoutDelay playout_delay = frame.EncodedImage().playout_delay_;
@@ -262,6 +278,13 @@ int FrameBuffer::InsertFrame(std::unique_ptr<FrameObject> frame) {
           ? -1
           : last_continuous_frame_it_->first.picture_id;
 
+  if (!ValidReferences(*frame)) {
+    LOG(LS_WARNING) << "Frame with (picture_id:spatial_id) (" << key.picture_id
+                    << ":" << static_cast<int>(key.spatial_layer)
+                    << ") has invalid frame references, dropping frame.";
+    return last_continuous_picture_id;
+  }
+
   if (num_frames_buffered_ >= kMaxFramesBuffered) {
     LOG(LS_WARNING) << "Frame with (picture_id:spatial_id) (" << key.picture_id
                     << ":" << static_cast<int>(key.spatial_layer)
@@ -270,13 +293,6 @@ int FrameBuffer::InsertFrame(std::unique_ptr<FrameObject> frame) {
     return last_continuous_picture_id;
   }
 
-  if (frame->inter_layer_predicted && frame->spatial_layer == 0) {
-    LOG(LS_WARNING) << "Frame with (picture_id:spatial_id) (" << key.picture_id
-                    << ":" << static_cast<int>(key.spatial_layer)
-                    << ") is marked as inter layer predicted, dropping frame.";
-    return last_continuous_picture_id;
-  }
-
   if (last_decoded_frame_it_ != frames_.end() &&
       key <= last_decoded_frame_it_->first) {
     if (AheadOf(frame->timestamp, last_decoded_frame_timestamp_) &&
@@ -363,14 +379,18 @@ void FrameBuffer::PropagateContinuity(FrameMap::iterator start) {
     // any unfulfilled dependencies then that frame is continuous as well.
     for (size_t d = 0; d < frame->second.num_dependent_frames; ++d) {
       auto frame_ref = frames_.find(frame->second.dependent_frames[d]);
-      --frame_ref->second.num_missing_continuous;
+      RTC_DCHECK(frame_ref != frames_.end());
 
+      // TODO(philipel): Look into why we've seen this happen.
+      if (frame_ref != frames_.end()) {
+        --frame_ref->second.num_missing_continuous;
         if (frame_ref->second.num_missing_continuous == 0) {
           frame_ref->second.continuous = true;
           continuous_frames.push(frame_ref);
         }
       }
     }
+  }
 }
 
 void FrameBuffer::PropagateDecodability(const FrameInfo& info) {

webrtc/modules/video_coding/frame_buffer2.h

@@ -97,6 +97,8 @@ class FrameBuffer {
 
     // Which other frames that have direct unfulfilled dependencies
     // on this frame.
+    // TODO(philipel): Add simple modify/access functions to prevent adding too
+    // many |dependent_frames|.
     FrameKey dependent_frames[kMaxNumDependentFrames];
     size_t num_dependent_frames = 0;
 
@@ -120,6 +122,9 @@ class FrameBuffer {
 
   using FrameMap = std::map<FrameKey, FrameInfo>;
 
+  // Check that the references of |frame| are valid.
+  bool ValidReferences(const FrameObject& frame) const;
+
   // Updates the minimal and maximal playout delays
   // depending on the frame.
   void UpdatePlayoutDelays(const FrameObject& frame)

webrtc/modules/video_coding/frame_buffer2_unittest.cc

@@ -531,5 +531,13 @@ TEST_F(TestFrameBuffer2, DuplicateFrames) {
   EXPECT_EQ(22256, InsertFrame(22256, 0, 1, false));
 }
 
+// TODO(philipel): implement more unittests related to invalid references.
+TEST_F(TestFrameBuffer2, InvalidReferences) {
+  EXPECT_EQ(-1, InsertFrame(0, 0, 1000, false, 2));
+  EXPECT_EQ(1, InsertFrame(1, 0, 2000, false));
+  ExtractFrame();
+  EXPECT_EQ(2, InsertFrame(2, 0, 3000, false, 1));
+}
+
 }  // namespace video_coding
 }  // namespace webrtc

webrtc/modules/video_coding/frame_object.h

@@ -51,6 +51,8 @@ class FrameObject : public webrtc::VCMEncodedFrame {
   uint8_t spatial_layer;
   uint32_t timestamp;
 
+  // TODO(philipel): Add simple modify/access functions to prevent adding too
+  // many |references|.
   size_t num_references;
   uint16_t references[kMaxFrameReferences];
   bool inter_layer_predicted;