diff --git a/modules/video_coding/frame_buffer3.cc b/modules/video_coding/frame_buffer3.cc index fab4ca7909..3c26380e45 100644 --- a/modules/video_coding/frame_buffer3.cc +++ b/modules/video_coding/frame_buffer3.cc @@ -11,9 +11,6 @@ #include "modules/video_coding/frame_buffer3.h" #include -#include -#include -#include #include "absl/algorithm/container.h" #include "absl/container/inlined_vector.h" @@ -52,7 +49,7 @@ int64_t GetFrameId(const FrameIteratorT& it) { } template -int64_t GetTimestamp(const FrameIteratorT& it) { +uint32_t GetTimestamp(const FrameIteratorT& it) { return it->second.encoded_frame->Timestamp(); } @@ -158,17 +155,9 @@ absl::optional FrameBuffer::LastContinuousTemporalUnitFrameId() const { return last_continuous_temporal_unit_frame_id_; } -absl::optional FrameBuffer::NextDecodableTemporalUnitRtpTimestamp() - const { - if (!next_decodable_temporal_unit_) { - return absl::nullopt; - } - return GetTimestamp(next_decodable_temporal_unit_->first_frame); -} - -absl::optional FrameBuffer::LastDecodableTemporalUnitRtpTimestamp() - const { - return last_decodable_temporal_unit_timestamp_; +absl::optional +FrameBuffer::DecodableTemporalUnitsInfo() const { + return decodable_temporal_units_info_; } int FrameBuffer::GetTotalNumberOfContinuousTemporalUnits() const { @@ -221,7 +210,7 @@ void FrameBuffer::PropagateContinuity(const FrameIterator& frame_it) { void FrameBuffer::FindNextAndLastDecodableTemporalUnit() { next_decodable_temporal_unit_.reset(); - last_decodable_temporal_unit_timestamp_.reset(); + decodable_temporal_units_info_.reset(); if (!last_continuous_temporal_unit_frame_id_) { return; @@ -230,6 +219,7 @@ void FrameBuffer::FindNextAndLastDecodableTemporalUnit() { FrameIterator first_frame_it = frames_.begin(); FrameIterator last_frame_it = frames_.begin(); absl::InlinedVector frames_in_temporal_unit; + uint32_t last_decodable_temporal_unit_timestamp; for (auto frame_it = frames_.begin(); frame_it != frames_.end();) { if (GetFrameId(frame_it) > *last_continuous_temporal_unit_frame_id_) { break; @@ -264,16 +254,23 @@ void FrameBuffer::FindNextAndLastDecodableTemporalUnit() { next_decodable_temporal_unit_ = {first_frame_it, last_frame_it}; } - last_decodable_temporal_unit_timestamp_ = GetTimestamp(first_frame_it); + last_decodable_temporal_unit_timestamp = GetTimestamp(first_frame_it); } } } + + if (next_decodable_temporal_unit_) { + decodable_temporal_units_info_ = { + .next_rtp_timestamp = + GetTimestamp(next_decodable_temporal_unit_->first_frame), + .last_rtp_timestamp = last_decodable_temporal_unit_timestamp}; + } } void FrameBuffer::Clear() { frames_.clear(); next_decodable_temporal_unit_.reset(); - last_decodable_temporal_unit_timestamp_.reset(); + decodable_temporal_units_info_.reset(); last_continuous_frame_id_.reset(); last_continuous_temporal_unit_frame_id_.reset(); decoded_frame_history_.Clear(); diff --git a/modules/video_coding/frame_buffer3.h b/modules/video_coding/frame_buffer3.h index 5bcfbd21b7..aef26d4f23 100644 --- a/modules/video_coding/frame_buffer3.h +++ b/modules/video_coding/frame_buffer3.h @@ -31,6 +31,11 @@ namespace webrtc { // The FrameBuffer is thread-unsafe. class FrameBuffer { public: + struct DecodabilityInfo { + uint32_t next_rtp_timestamp; + uint32_t last_rtp_timestamp; + }; + // The `max_size` determines the maxmimum number of frames the buffer will // store, and max_decode_history determines how far back (by frame ID) the // buffer will store if a frame was decoded or not. @@ -56,8 +61,7 @@ class FrameBuffer { absl::optional LastContinuousFrameId() const; absl::optional LastContinuousTemporalUnitFrameId() const; - absl::optional NextDecodableTemporalUnitRtpTimestamp() const; - absl::optional LastDecodableTemporalUnitRtpTimestamp() const; + absl::optional DecodableTemporalUnitsInfo() const; int GetTotalNumberOfContinuousTemporalUnits() const; int GetTotalNumberOfDroppedFrames() const; @@ -87,7 +91,7 @@ class FrameBuffer { const size_t max_size_; FrameMap frames_; absl::optional next_decodable_temporal_unit_; - absl::optional last_decodable_temporal_unit_timestamp_; + absl::optional decodable_temporal_units_info_; absl::optional last_continuous_frame_id_; absl::optional last_continuous_temporal_unit_frame_id_; video_coding::DecodedFramesHistory decoded_frame_history_; diff --git a/modules/video_coding/frame_buffer3_unittest.cc b/modules/video_coding/frame_buffer3_unittest.cc index 39fab888aa..305471ffc2 100644 --- a/modules/video_coding/frame_buffer3_unittest.cc +++ b/modules/video_coding/frame_buffer3_unittest.cc @@ -111,10 +111,9 @@ TEST(FrameBuffer3Test, NextDecodable) { FrameBuffer buffer(/*max_frame_slots=*/10, /*max_decode_history=*/100, field_trials); - EXPECT_THAT(buffer.NextDecodableTemporalUnitRtpTimestamp(), - Eq(absl::nullopt)); + EXPECT_THAT(buffer.DecodableTemporalUnitsInfo(), Eq(absl::nullopt)); buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); - EXPECT_THAT(buffer.NextDecodableTemporalUnitRtpTimestamp(), Eq(10U)); + EXPECT_THAT(buffer.DecodableTemporalUnitsInfo()->next_rtp_timestamp, Eq(10U)); } TEST(FrameBuffer3Test, AdvanceNextDecodableOnExtraction) { @@ -126,14 +125,14 @@ TEST(FrameBuffer3Test, AdvanceNextDecodableOnExtraction) { buffer.InsertFrame(test::FakeFrameBuilder().Time(20).Id(2).AsLast().Build()); buffer.InsertFrame( test::FakeFrameBuilder().Time(30).Id(3).Refs({2}).AsLast().Build()); - EXPECT_THAT(buffer.NextDecodableTemporalUnitRtpTimestamp(), Eq(10U)); + EXPECT_THAT(buffer.DecodableTemporalUnitsInfo()->next_rtp_timestamp, Eq(10U)); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), ElementsAre(FrameWithId(1))); - EXPECT_THAT(buffer.NextDecodableTemporalUnitRtpTimestamp(), Eq(20U)); + EXPECT_THAT(buffer.DecodableTemporalUnitsInfo()->next_rtp_timestamp, Eq(20U)); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), ElementsAre(FrameWithId(2))); - EXPECT_THAT(buffer.NextDecodableTemporalUnitRtpTimestamp(), Eq(30U)); + EXPECT_THAT(buffer.DecodableTemporalUnitsInfo()->next_rtp_timestamp, Eq(30U)); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), ElementsAre(FrameWithId(3))); } @@ -148,11 +147,11 @@ TEST(FrameBuffer3Test, AdvanceLastDecodableOnExtraction) { test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build()); buffer.InsertFrame( test::FakeFrameBuilder().Time(30).Id(3).Refs({1}).AsLast().Build()); - EXPECT_THAT(buffer.LastDecodableTemporalUnitRtpTimestamp(), Eq(10U)); + EXPECT_THAT(buffer.DecodableTemporalUnitsInfo()->last_rtp_timestamp, Eq(10U)); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), ElementsAre(FrameWithId(1))); - EXPECT_THAT(buffer.LastDecodableTemporalUnitRtpTimestamp(), Eq(30U)); + EXPECT_THAT(buffer.DecodableTemporalUnitsInfo()->last_rtp_timestamp, Eq(30U)); } TEST(FrameBuffer3Test, FrameUpdatesNextDecodable) { @@ -161,10 +160,10 @@ TEST(FrameBuffer3Test, FrameUpdatesNextDecodable) { field_trials); buffer.InsertFrame(test::FakeFrameBuilder().Time(20).Id(2).AsLast().Build()); - EXPECT_THAT(buffer.NextDecodableTemporalUnitRtpTimestamp(), Eq(20U)); + EXPECT_THAT(buffer.DecodableTemporalUnitsInfo()->next_rtp_timestamp, Eq(20U)); buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); - EXPECT_THAT(buffer.NextDecodableTemporalUnitRtpTimestamp(), Eq(10U)); + EXPECT_THAT(buffer.DecodableTemporalUnitsInfo()->next_rtp_timestamp, Eq(10U)); } TEST(FrameBuffer3Test, KeyframeClearsFullBuffer) { diff --git a/test/fuzzers/frame_buffer3_fuzzer.cc b/test/fuzzers/frame_buffer3_fuzzer.cc index e4222b2fb8..b57be4af70 100644 --- a/test/fuzzers/frame_buffer3_fuzzer.cc +++ b/test/fuzzers/frame_buffer3_fuzzer.cc @@ -39,7 +39,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) { SeqNumUnwrapper unwrapper; while (helper.BytesLeft() > 0) { - int action = helper.ReadOrDefaultValue(0) % 7; + int action = helper.ReadOrDefaultValue(0) % 6; switch (action) { case 0: { @@ -51,22 +51,18 @@ void FuzzOneInput(const uint8_t* data, size_t size) { break; } case 2: { - buffer.NextDecodableTemporalUnitRtpTimestamp(); + buffer.DecodableTemporalUnitsInfo(); break; } case 3: { - buffer.LastDecodableTemporalUnitRtpTimestamp(); - break; - } - case 4: { buffer.ExtractNextDecodableTemporalUnit(); break; } - case 5: { + case 4: { buffer.DropNextDecodableTemporalUnit(); break; } - case 6: { + case 5: { auto frame = std::make_unique(); frame->SetTimestamp(helper.ReadOrDefaultValue(0)); int64_t wire_id = diff --git a/video/frame_buffer_proxy.cc b/video/frame_buffer_proxy.cc index d4da0da2c4..ab08ce9ddf 100644 --- a/video/frame_buffer_proxy.cc +++ b/video/frame_buffer_proxy.cc @@ -384,13 +384,11 @@ class FrameBuffer3Proxy : public FrameBufferProxy { private: void FrameReadyForDecode(uint32_t rtp_timestamp, Timestamp render_time) { RTC_DCHECK_RUN_ON(&worker_sequence_checker_); - RTC_DCHECK(buffer_->NextDecodableTemporalUnitRtpTimestamp() == - rtp_timestamp) + auto frames = buffer_->ExtractNextDecodableTemporalUnit(); + RTC_DCHECK(frames[0]->Timestamp() == rtp_timestamp) << "Frame buffer's next decodable frame was not the one sent for " "extraction rtp=" - << rtp_timestamp << " next=" - << buffer_->NextDecodableTemporalUnitRtpTimestamp().value_or(-1); - auto frames = buffer_->ExtractNextDecodableTemporalUnit(); + << rtp_timestamp << " extracted rtp=" << frames[0]->Timestamp(); OnFrameReady(std::move(frames), render_time); } @@ -431,7 +429,7 @@ class FrameBuffer3Proxy : public FrameBufferProxy { RTC_DCHECK(keyframe_required_); // Iterate through the frame buffer until there is a complete keyframe and // release this right away. - while (buffer_->NextDecodableTemporalUnitRtpTimestamp()) { + while (buffer_->DecodableTemporalUnitsInfo()) { auto next_frame = buffer_->ExtractNextDecodableTemporalUnit(); if (next_frame.empty()) { RTC_DCHECK_NOTREACHED() @@ -449,41 +447,40 @@ class FrameBuffer3Proxy : public FrameBufferProxy { } void MaybeScheduleFrameForRelease() RTC_RUN_ON(&worker_sequence_checker_) { - if (!decoder_ready_for_new_frame_ || - !buffer_->NextDecodableTemporalUnitRtpTimestamp()) + auto decodable_tu_info = buffer_->DecodableTemporalUnitsInfo(); + if (!decoder_ready_for_new_frame_ || !decodable_tu_info) { return; + } if (keyframe_required_) { return ForceKeyFrameReleaseImmediately(); } - // TODO(https://bugs.webrtc.org/13343): Make [next,last] decodable returned - // as an optional pair and remove this check. - RTC_CHECK(buffer_->LastDecodableTemporalUnitRtpTimestamp()); - auto last_rtp = *buffer_->LastDecodableTemporalUnitRtpTimestamp(); - // If already scheduled then abort. if (frame_decode_scheduler_->ScheduledRtpTimestamp() == - buffer_->NextDecodableTemporalUnitRtpTimestamp()) + decodable_tu_info->next_rtp_timestamp) { return; + } absl::optional schedule; - while (buffer_->NextDecodableTemporalUnitRtpTimestamp()) { - auto next_rtp = *buffer_->NextDecodableTemporalUnitRtpTimestamp(); - schedule = decode_timing_.OnFrameBufferUpdated(next_rtp, last_rtp, - IsTooManyFramesQueued()); + while (decodable_tu_info) { + schedule = decode_timing_.OnFrameBufferUpdated( + decodable_tu_info->next_rtp_timestamp, + decodable_tu_info->last_rtp_timestamp, IsTooManyFramesQueued()); if (schedule) { // Don't schedule if already waiting for the same frame. - if (frame_decode_scheduler_->ScheduledRtpTimestamp() != next_rtp) { + if (frame_decode_scheduler_->ScheduledRtpTimestamp() != + decodable_tu_info->next_rtp_timestamp) { frame_decode_scheduler_->CancelOutstanding(); frame_decode_scheduler_->ScheduleFrame( - next_rtp, *schedule, + decodable_tu_info->next_rtp_timestamp, *schedule, absl::bind_front(&FrameBuffer3Proxy::FrameReadyForDecode, this)); } return; } // If no schedule for current rtp, drop and try again. buffer_->DropNextDecodableTemporalUnit(); + decodable_tu_info = buffer_->DecodableTemporalUnitsInfo(); } }