From 93e451b0f5b09784c29d90407078c5e6ab771d94 Mon Sep 17 00:00:00 2001
From: philipel <philipel@webrtc.org>
Date: Thu, 6 Oct 2016 12:25:13 +0200
Subject: [PATCH] Skip non-continuous FrameInfos when finding the next frame to
 return from FrameBuffer::NextFrame.

Also added DCHECKS to make sure that FrameInfos are not incorrectly updated.

BUG=webrtc:5514
R=stefan@webrtc.org

Review URL: https://codereview.webrtc.org/2394823002 .

Cr-Commit-Position: refs/heads/master@{#14549}
---
 webrtc/modules/video_coding/frame_buffer2.cc  | 23 ++++++++++++++++---
 .../video_coding/frame_buffer2_unittest.cc    | 16 +++++++++++++
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/webrtc/modules/video_coding/frame_buffer2.cc b/webrtc/modules/video_coding/frame_buffer2.cc
index b2b4c213fe..8bb19ba003 100644
--- a/webrtc/modules/video_coding/frame_buffer2.cc
+++ b/webrtc/modules/video_coding/frame_buffer2.cc
@@ -85,8 +85,10 @@ FrameBuffer::ReturnReason FrameBuffer::NextFrame(
         ++continuous_end_it;
 
       for (; frame_it != continuous_end_it; ++frame_it) {
-        if (frame_it->second.num_missing_decodable > 0)
+        if (!frame_it->second.continuous ||
+            frame_it->second.num_missing_decodable > 0) {
           continue;
+        }
 
         FrameObject* frame = frame_it->second.frame.get();
         next_frame_it = frame_it;
@@ -151,6 +153,8 @@ void FrameBuffer::Stop() {
 
 int FrameBuffer::InsertFrame(std::unique_ptr<FrameObject> frame) {
   rtc::CritScope lock(&crit_);
+  RTC_DCHECK(frame);
+
   FrameKey key(frame->picture_id, frame->spatial_layer);
   int last_continuous_picture_id =
       last_continuous_frame_it_ == frames_.end()
@@ -186,11 +190,16 @@ int FrameBuffer::InsertFrame(std::unique_ptr<FrameObject> frame) {
 
   auto info = frames_.insert(std::make_pair(key, FrameInfo())).first;
 
-  if (!UpdateFrameInfoWithIncomingFrame(*frame, info)) {
-    frames_.erase(info);
+  if (info->second.frame) {
+    LOG(LS_WARNING) << "Frame with (picture_id:spatial_id) (" << key.picture_id
+                    << ":" << static_cast<int>(key.spatial_layer)
+                    << ") already inserted, dropping frame.";
     return last_continuous_picture_id;
   }
 
+  if (!UpdateFrameInfoWithIncomingFrame(*frame, info))
+    return last_continuous_picture_id;
+
   info->second.frame = std::move(frame);
   ++num_frames_buffered_;
 
@@ -241,6 +250,7 @@ void FrameBuffer::PropagateContinuity(FrameMap::iterator start) {
 void FrameBuffer::PropagateDecodability(const FrameInfo& info) {
   for (size_t d = 0; d < info.num_dependent_frames; ++d) {
     auto ref_info = frames_.find(info.dependent_frames[d]);
+    RTC_DCHECK(ref_info != frames_.end());
     RTC_DCHECK_GT(ref_info->second.num_missing_decodable, 0U);
     --ref_info->second.num_missing_decodable;
   }
@@ -311,6 +321,8 @@ bool FrameBuffer::UpdateFrameInfoWithIncomingFrame(const FrameObject& frame,
           key;
       ++ref_info->second.num_dependent_frames;
     }
+    RTC_DCHECK_LE(ref_info->second.num_missing_continuous,
+                  ref_info->second.num_missing_decodable);
   }
 
   // Check if we have the lower spatial layer frame.
@@ -331,8 +343,13 @@ bool FrameBuffer::UpdateFrameInfoWithIncomingFrame(const FrameObject& frame,
           key;
       ++ref_info->second.num_dependent_frames;
     }
+    RTC_DCHECK_LE(ref_info->second.num_missing_continuous,
+                  ref_info->second.num_missing_decodable);
   }
 
+  RTC_DCHECK_LE(info->second.num_missing_continuous,
+                info->second.num_missing_decodable);
+
   return true;
 }
 
diff --git a/webrtc/modules/video_coding/frame_buffer2_unittest.cc b/webrtc/modules/video_coding/frame_buffer2_unittest.cc
index 7f8ac67440..ebcbeb0254 100644
--- a/webrtc/modules/video_coding/frame_buffer2_unittest.cc
+++ b/webrtc/modules/video_coding/frame_buffer2_unittest.cc
@@ -263,6 +263,22 @@ TEST_F(TestFrameBuffer2, ExtractFromEmptyBuffer) {
   CheckNoFrame(0);
 }
 
+TEST_F(TestFrameBuffer2, MissingFrame) {
+  uint16_t pid = Rand();
+  uint32_t ts = Rand();
+
+  InsertFrame(pid, 0, ts, false);
+  InsertFrame(pid + 2, 0, ts, false, pid);
+  InsertFrame(pid + 3, 0, ts, false, pid + 1, pid + 2);
+  ExtractFrame();
+  ExtractFrame();
+  ExtractFrame();
+
+  CheckFrame(0, pid, 0);
+  CheckFrame(1, pid + 2, 0);
+  CheckNoFrame(2);
+}
+
 TEST_F(TestFrameBuffer2, OneLayerStream) {
   uint16_t pid = Rand();
   uint32_t ts = Rand();