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}
diff --git a/video/task_queue_frame_decode_scheduler.cc b/video/task_queue_frame_decode_scheduler.cc
index c53a530..cd109c2 100644
--- a/video/task_queue_frame_decode_scheduler.cc
+++ b/video/task_queue_frame_decode_scheduler.cc
@@ -49,8 +49,8 @@
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
- // this scheduled release should be skipped.
+ // If the next frame rtp has changed since this task was
+ // this scheduled release should be skipped.
if (scheduled_rtp_ != rtp)
return;
scheduled_rtp_ = absl::nullopt;
diff --git a/video/video_stream_buffer_controller.cc b/video/video_stream_buffer_controller.cc
index 0e0f2cc..f7d3acd 100644
--- a/video/video_stream_buffer_controller.cc
+++ b/video/video_stream_buffer_controller.cc
@@ -191,7 +191,8 @@
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();
@@ -275,11 +276,29 @@
void VideoStreamBufferController::FrameReadyForDecode(uint32_t rtp_timestamp,
Timestamp render_time) {
RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
- auto frames = buffer_->ExtractNextDecodableTemporalUnit();
- RTC_DCHECK(frames[0]->Timestamp() == rtp_timestamp)
+ // Check that the frame to decode is still valid before passing the frame for
+ // 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="
- << rtp_timestamp << " extracted rtp=" << frames[0]->Timestamp();
+ "extraction.";
+ 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);
}
diff --git a/video/video_stream_buffer_controller_unittest.cc b/video/video_stream_buffer_controller_unittest.cc
index ea9eaed..3e6c352 100644
--- a/video/video_stream_buffer_controller_unittest.cc
+++ b/video/video_stream_buffer_controller_unittest.cc
@@ -754,6 +754,50 @@
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(),
