Ensure video frame buffer is still decodable before decoding

This ensures that if for some reason, the frame buffer becomes
undecodable while waiting to decode a frame, the decoding is halted.
This also guards against receiving an empty temporal unit from the frame
buffer, even though this should never happen when the frame buffer has a
decodable temporal unit.

Bug: chromium:1378253, chromium:1361623
Change-Id: I8c4c897bf474d5cbda5f0f357781bf1dc0701fe4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280701
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38494}

video/task_queue_frame_decode_scheduler.cc

@@ -68,8 +68,8 @@ void TaskQueueFrameDecodeScheduler::ScheduleFrame(
       SafeTask(task_safety_.flag(),
                [this, rtp, schedule, cb = std::move(cb)]() mutable {
                  RTC_DCHECK_RUN_ON(bookkeeping_queue_);
-                 // If the next frame rtp  has changed since this task was
+                 // If the next frame rtp has changed since this task was
-                 // this scheduled  release should be skipped.
+                 // this scheduled release should be skipped.
                  if (scheduled_rtp_ != rtp)
                    return;
                  scheduled_rtp_ = absl::nullopt;

video/video_stream_buffer_controller.cc

@@ -192,7 +192,8 @@ void VideoStreamBufferController::OnFrameReady(
     absl::InlinedVector<std::unique_ptr<EncodedFrame>, 4> frames,
     Timestamp render_time) {
   RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
-  RTC_DCHECK(!frames.empty());
+  RTC_CHECK(!frames.empty())
+      << "Callers must ensure there is at least one frame to decode.";
 
   timeout_tracker_.OnEncodedFrameReleased();
 
@@ -276,11 +277,29 @@ void VideoStreamBufferController::OnTimeout(TimeDelta delay) {
 void VideoStreamBufferController::FrameReadyForDecode(uint32_t rtp_timestamp,
                                                       Timestamp render_time) {
   RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
-  auto frames = buffer_->ExtractNextDecodableTemporalUnit();
+  // Check that the frame to decode is still valid before passing the frame for
-  RTC_DCHECK(frames[0]->Timestamp() == rtp_timestamp)
+  // decoding.
+  auto decodable_tu_info = buffer_->DecodableTemporalUnitsInfo();
+  if (!decodable_tu_info) {
+    RTC_LOG(LS_ERROR)
+        << "The frame buffer became undecodable during the wait "
+           "to decode frame with rtp-timestamp "
+        << rtp_timestamp
+        << ". Cancelling the decode of this frame, decoding "
+           "will resume when the frame buffers become decodable again.";
+    return;
+  }
+  RTC_DCHECK_EQ(rtp_timestamp, decodable_tu_info->next_rtp_timestamp)
       << "Frame buffer's next decodable frame was not the one sent for "
-         "extraction rtp="
+         "extraction.";
-      << rtp_timestamp << " extracted rtp=" << frames[0]->Timestamp();
+  auto frames = buffer_->ExtractNextDecodableTemporalUnit();
+  if (frames.empty()) {
+    RTC_LOG(LS_ERROR)
+        << "The frame buffer should never return an empty temporal until list "
+           "when there is a decodable temporal unit.";
+    RTC_DCHECK_NOTREACHED();
+    return;
+  }
   OnFrameReady(std::move(frames), render_time);
 }
 

video/video_stream_buffer_controller_unittest.cc

@@ -754,6 +754,50 @@ TEST_P(VideoStreamBufferControllerTest, NextFrameWithOldTimestamp) {
   EXPECT_THAT(WaitForFrameOrTimeout(kFps30Delay), Frame(test::WithId(2)));
 }
 
+TEST_P(VideoStreamBufferControllerTest,
+       FrameNotSetForDecodedIfFrameBufferBecomesNonDecodable) {
+  // This can happen if the frame buffer receives non-standard input. This test
+  // will simply clear the frame buffer to replicate this.
+  StartNextDecodeForceKeyframe();
+  // Initial keyframe.
+  buffer_->InsertFrame(WithReceiveTimeFromRtpTimestamp(
+      test::FakeFrameBuilder().Id(0).Time(0).SpatialLayer(1).AsLast().Build()));
+  EXPECT_THAT(WaitForFrameOrTimeout(TimeDelta::Zero()), Frame(test::WithId(0)));
+
+  // Insert a frame that will become non-decodable.
+  buffer_->InsertFrame(WithReceiveTimeFromRtpTimestamp(test::FakeFrameBuilder()
+                                                           .Id(11)
+                                                           .Time(kFps30Rtp)
+                                                           .Refs({0})
+                                                           .SpatialLayer(1)
+                                                           .AsLast()
+                                                           .Build()));
+  StartNextDecode();
+  // Second layer inserted after last layer for the same frame out-of-order.
+  // This second frame requires some older frame to be decoded and so now the
+  // super-frame is no longer decodable despite already being scheduled.
+  buffer_->InsertFrame(WithReceiveTimeFromRtpTimestamp(test::FakeFrameBuilder()
+                                                           .Id(10)
+                                                           .Time(kFps30Rtp)
+                                                           .SpatialLayer(0)
+                                                           .Refs({2})
+                                                           .Build()));
+  EXPECT_THAT(WaitForFrameOrTimeout(kMaxWaitForFrame), TimedOut());
+
+  // Ensure that this frame can be decoded later.
+  StartNextDecode();
+  buffer_->InsertFrame(WithReceiveTimeFromRtpTimestamp(test::FakeFrameBuilder()
+                                                           .Id(2)
+                                                           .Time(kFps30Rtp / 2)
+                                                           .SpatialLayer(0)
+                                                           .Refs({0})
+                                                           .AsLast()
+                                                           .Build()));
+  EXPECT_THAT(WaitForFrameOrTimeout(kFps30Delay), Frame(test::WithId(2)));
+  StartNextDecode();
+  EXPECT_THAT(WaitForFrameOrTimeout(kFps30Delay), Frame(test::WithId(10)));
+}
+
 INSTANTIATE_TEST_SUITE_P(VideoStreamBufferController,
                          VideoStreamBufferControllerTest,
                          ::testing::Combine(::testing::Bool(),