From 66fcd16a416d755377cca39f9703970fdbc3e026 Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Wed, 15 Jun 2022 08:25:22 +0000 Subject: [PATCH] FrameBuffer::InsertFrame returns true on successful insertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is cleaner than checking the size before and after, as is currently done in FrameBufferProxy Bug: webrtc:14168 Change-Id: Iac896ddf7b1b0b8513159451de7cd8a10668a49a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265663 Reviewed-by: Niels Moller Reviewed-by: Philip Eliasson Commit-Queue: Evan Shrubsole Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/main@{#37222} --- api/video/frame_buffer.cc | 11 +- api/video/frame_buffer.h | 9 +- api/video/frame_buffer_unittest.cc | 236 ++++++++++++++++------------- video/frame_buffer_proxy.cc | 4 +- 4 files changed, 142 insertions(+), 118 deletions(-) diff --git a/api/video/frame_buffer.cc b/api/video/frame_buffer.cc index 67d5f6787f..4cdf2212a6 100644 --- a/api/video/frame_buffer.cc +++ b/api/video/frame_buffer.cc @@ -67,11 +67,11 @@ FrameBuffer::FrameBuffer(int max_size, max_size_(max_size), decoded_frame_history_(max_decode_history) {} -void FrameBuffer::InsertFrame(std::unique_ptr frame) { +bool FrameBuffer::InsertFrame(std::unique_ptr frame) { if (!ValidReferences(*frame)) { RTC_DLOG(LS_WARNING) << "Frame " << frame->Id() << " has invalid references, dropping frame."; - return; + return false; } if (frame->Id() <= decoded_frame_history_.GetLastDecodedFrameId()) { @@ -84,7 +84,7 @@ void FrameBuffer::InsertFrame(std::unique_ptr frame) { Clear(); } else { // Already decoded past this frame. - return; + return false; } } @@ -95,7 +95,7 @@ void FrameBuffer::InsertFrame(std::unique_ptr frame) { Clear(); } else { // No space for this frame. - return; + return false; } } @@ -103,7 +103,7 @@ void FrameBuffer::InsertFrame(std::unique_ptr frame) { auto insert_res = frames_.emplace(frame_id, FrameInfo{std::move(frame)}); if (!insert_res.second) { // Frame has already been inserted. - return; + return false; } if (frames_.size() == max_size_) { @@ -113,6 +113,7 @@ void FrameBuffer::InsertFrame(std::unique_ptr frame) { PropagateContinuity(insert_res.first); FindNextAndLastDecodableTemporalUnit(); + return true; } absl::InlinedVector, 4> diff --git a/api/video/frame_buffer.h b/api/video/frame_buffer.h index 5ab88c50ee..94edf64d5a 100644 --- a/api/video/frame_buffer.h +++ b/api/video/frame_buffer.h @@ -18,7 +18,6 @@ #include "absl/container/inlined_vector.h" #include "absl/types/optional.h" #include "api/field_trials_view.h" -#include "api/units/timestamp.h" #include "api/video/encoded_frame.h" #include "modules/video_coding/utility/decoded_frames_history.h" @@ -36,7 +35,7 @@ class FrameBuffer { uint32_t last_rtp_timestamp; }; - // The `max_size` determines the maxmimum number of frames the buffer will + // The `max_size` determines the maximum 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. FrameBuffer(int max_size, @@ -48,8 +47,10 @@ class FrameBuffer { ~FrameBuffer() = default; // Inserted frames may only reference backwards, and must have no duplicate - // references. - void InsertFrame(std::unique_ptr frame); + // references. Frame insertion will fail if `frame` is a duplicate, has + // already been decoded, invalid, or if the buffer is full and the frame is + // not a keyframe. Returns true if the frame was successfully inserted. + bool InsertFrame(std::unique_ptr frame); // Mark all frames belonging to the next decodable temporal unit as decoded // and returns them. diff --git a/api/video/frame_buffer_unittest.cc b/api/video/frame_buffer_unittest.cc index 41a486f192..92e2f67540 100644 --- a/api/video/frame_buffer_unittest.cc +++ b/api/video/frame_buffer_unittest.cc @@ -13,7 +13,6 @@ #include "api/video/encoded_frame.h" #include "test/fake_encoded_frame.h" -#include "test/field_trial.h" #include "test/gmock.h" #include "test/gtest.h" #include "test/scoped_key_value_config.h" @@ -35,14 +34,15 @@ TEST(FrameBuffer3Test, RejectInvalidRefs) { FrameBuffer buffer(/*max_frame_slots=*/10, /*max_decode_history=*/100, field_trials); // Ref must be less than the id of this frame. - buffer.InsertFrame( - test::FakeFrameBuilder().Time(0).Id(0).Refs({0}).AsLast().Build()); + EXPECT_FALSE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(0).Id(0).Refs({0}).AsLast().Build())); EXPECT_THAT(buffer.LastContinuousFrameId(), Eq(absl::nullopt)); // Duplicate ids are also invalid. - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(20).Id(2).Refs({1, 1}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build())); + EXPECT_FALSE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(2).Refs({1, 1}).AsLast().Build())); EXPECT_THAT(buffer.LastContinuousFrameId(), Eq(1)); } @@ -53,12 +53,13 @@ TEST(FrameBuffer3Test, LastContinuousUpdatesOnInsertedFrames) { EXPECT_THAT(buffer.LastContinuousFrameId(), Eq(absl::nullopt)); EXPECT_THAT(buffer.LastContinuousTemporalUnitFrameId(), Eq(absl::nullopt)); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).Build()); + EXPECT_TRUE( + buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).Build())); EXPECT_THAT(buffer.LastContinuousFrameId(), Eq(1)); EXPECT_THAT(buffer.LastContinuousTemporalUnitFrameId(), Eq(absl::nullopt)); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(10).Id(2).Refs({1}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(2).Refs({1}).AsLast().Build())); EXPECT_THAT(buffer.LastContinuousFrameId(), Eq(2)); EXPECT_THAT(buffer.LastContinuousTemporalUnitFrameId(), Eq(2)); } @@ -68,13 +69,14 @@ TEST(FrameBuffer3Test, LastContinuousFrameReordering) { FrameBuffer buffer(/*max_frame_slots=*/10, /*max_decode_history=*/100, field_trials); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(30).Id(3).Refs({2}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(30).Id(3).Refs({2}).AsLast().Build())); EXPECT_THAT(buffer.LastContinuousFrameId(), Eq(1)); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build())); EXPECT_THAT(buffer.LastContinuousFrameId(), Eq(3)); } @@ -83,10 +85,11 @@ TEST(FrameBuffer3Test, LastContinuousTemporalUnit) { FrameBuffer buffer(/*max_frame_slots=*/10, /*max_decode_history=*/100, field_trials); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).Build()); + EXPECT_TRUE( + buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).Build())); EXPECT_THAT(buffer.LastContinuousTemporalUnitFrameId(), Eq(absl::nullopt)); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(10).Id(2).Refs({1}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(2).Refs({1}).AsLast().Build())); EXPECT_THAT(buffer.LastContinuousTemporalUnitFrameId(), Eq(2)); } @@ -95,14 +98,16 @@ TEST(FrameBuffer3Test, LastContinuousTemporalUnitReordering) { FrameBuffer buffer(/*max_frame_slots=*/10, /*max_decode_history=*/100, field_trials); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).Build()); - buffer.InsertFrame(test::FakeFrameBuilder().Time(20).Id(3).Refs({1}).Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(20).Id(4).Refs({2, 3}).AsLast().Build()); + EXPECT_TRUE( + buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(3).Refs({1}).Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(4).Refs({2, 3}).AsLast().Build())); EXPECT_THAT(buffer.LastContinuousTemporalUnitFrameId(), Eq(absl::nullopt)); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(10).Id(2).Refs({1}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(2).Refs({1}).AsLast().Build())); EXPECT_THAT(buffer.LastContinuousTemporalUnitFrameId(), Eq(4)); } @@ -112,7 +117,8 @@ TEST(FrameBuffer3Test, NextDecodable) { field_trials); EXPECT_THAT(buffer.DecodableTemporalUnitsInfo(), Eq(absl::nullopt)); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build())); EXPECT_THAT(buffer.DecodableTemporalUnitsInfo()->next_rtp_timestamp, Eq(10U)); } @@ -121,10 +127,12 @@ TEST(FrameBuffer3Test, AdvanceNextDecodableOnExtraction) { FrameBuffer buffer(/*max_frame_slots=*/10, /*max_decode_history=*/100, field_trials); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); - buffer.InsertFrame(test::FakeFrameBuilder().Time(20).Id(2).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(30).Id(3).Refs({2}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(2).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(30).Id(3).Refs({2}).AsLast().Build())); EXPECT_THAT(buffer.DecodableTemporalUnitsInfo()->next_rtp_timestamp, Eq(10U)); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), @@ -142,11 +150,12 @@ TEST(FrameBuffer3Test, AdvanceLastDecodableOnExtraction) { FrameBuffer buffer(/*max_frame_slots=*/10, /*max_decode_history=*/100, field_trials); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(30).Id(3).Refs({1}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(30).Id(3).Refs({1}).AsLast().Build())); EXPECT_THAT(buffer.DecodableTemporalUnitsInfo()->last_rtp_timestamp, Eq(10U)); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), @@ -159,10 +168,12 @@ TEST(FrameBuffer3Test, FrameUpdatesNextDecodable) { FrameBuffer buffer(/*max_frame_slots=*/10, /*max_decode_history=*/100, field_trials); - buffer.InsertFrame(test::FakeFrameBuilder().Time(20).Id(2).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(2).AsLast().Build())); EXPECT_THAT(buffer.DecodableTemporalUnitsInfo()->next_rtp_timestamp, Eq(20U)); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build())); EXPECT_THAT(buffer.DecodableTemporalUnitsInfo()->next_rtp_timestamp, Eq(10U)); } @@ -170,23 +181,25 @@ TEST(FrameBuffer3Test, KeyframeClearsFullBuffer) { test::ScopedKeyValueConfig field_trials; FrameBuffer buffer(/*max_frame_slots=*/5, /*max_decode_history=*/10, field_trials); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(30).Id(3).Refs({2}).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(40).Id(4).Refs({3}).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(50).Id(5).Refs({4}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(30).Id(3).Refs({2}).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(40).Id(4).Refs({3}).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(50).Id(5).Refs({4}).AsLast().Build())); EXPECT_THAT(buffer.LastContinuousFrameId(), Eq(5)); // Frame buffer is full - buffer.InsertFrame( - test::FakeFrameBuilder().Time(60).Id(6).Refs({5}).AsLast().Build()); + EXPECT_FALSE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(60).Id(6).Refs({5}).AsLast().Build())); EXPECT_THAT(buffer.LastContinuousFrameId(), Eq(5)); - buffer.InsertFrame(test::FakeFrameBuilder().Time(70).Id(7).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(70).Id(7).AsLast().Build())); EXPECT_THAT(buffer.LastContinuousFrameId(), Eq(7)); } @@ -194,11 +207,12 @@ TEST(FrameBuffer3Test, DropNextDecodableTemporalUnit) { test::ScopedKeyValueConfig field_trials; FrameBuffer buffer(/*max_frame_slots=*/10, /*max_decode_history=*/100, field_trials); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(30).Id(3).Refs({1}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(30).Id(3).Refs({1}).AsLast().Build())); buffer.ExtractNextDecodableTemporalUnit(); buffer.DropNextDecodableTemporalUnit(); @@ -210,19 +224,20 @@ TEST(FrameBuffer3Test, OldFramesAreIgnored) { test::ScopedKeyValueConfig field_trials; FrameBuffer buffer(/*max_frame_slots=*/10, /*max_decode_history=*/100, field_trials); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build())); buffer.ExtractNextDecodableTemporalUnit(); buffer.ExtractNextDecodableTemporalUnit(); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(30).Id(3).Refs({1}).AsLast().Build()); - + EXPECT_FALSE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build())); + EXPECT_FALSE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(30).Id(3).Refs({1}).AsLast().Build())); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), ElementsAre(FrameWithId(3))); } @@ -231,15 +246,17 @@ TEST(FrameBuffer3Test, ReturnFullTemporalUnitKSVC) { test::ScopedKeyValueConfig field_trials; FrameBuffer buffer(/*max_frame_slots=*/10, /*max_decode_history=*/100, field_trials); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).Build()); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(2).Refs({1}).Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(10).Id(3).Refs({2}).AsLast().Build()); + EXPECT_TRUE( + buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(2).Refs({1}).Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(3).Refs({2}).AsLast().Build())); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), ElementsAre(FrameWithId(1), FrameWithId(2), FrameWithId(3))); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(20).Id(4).Refs({3}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(4).Refs({3}).AsLast().Build())); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), ElementsAre(FrameWithId(4))); } @@ -248,15 +265,16 @@ TEST(FrameBuffer3Test, InterleavedStream) { test::ScopedKeyValueConfig field_trials; FrameBuffer buffer(/*max_frame_slots=*/10, /*max_decode_history=*/100, field_trials); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(30).Id(3).Refs({1}).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(40).Id(4).Refs({2}).AsLast().Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(50).Id(5).Refs({3}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(30).Id(3).Refs({1}).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(40).Id(4).Refs({2}).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(50).Id(5).Refs({3}).AsLast().Build())); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), ElementsAre(FrameWithId(1))); @@ -269,15 +287,15 @@ TEST(FrameBuffer3Test, InterleavedStream) { EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), ElementsAre(FrameWithId(5))); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(70).Id(7).Refs({5}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(70).Id(7).Refs({5}).AsLast().Build())); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), ElementsAre(FrameWithId(7))); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(60).Id(6).Refs({4}).AsLast().Build()); + EXPECT_FALSE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(60).Id(6).Refs({4}).AsLast().Build())); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), IsEmpty()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(90).Id(9).Refs({7}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(90).Id(9).Refs({7}).AsLast().Build())); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), ElementsAre(FrameWithId(9))); } @@ -289,12 +307,12 @@ TEST(FrameBuffer3Test, LegacyFrameIdJumpBehavior) { FrameBuffer buffer(/*max_frame_slots=*/10, /*max_decode_history=*/100, field_trials); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(20).Id(3).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(3).AsLast().Build())); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), ElementsAre(FrameWithId(3))); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(30).Id(2).AsLast().Build()); + EXPECT_FALSE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(30).Id(2).AsLast().Build())); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), IsEmpty()); } @@ -304,15 +322,15 @@ TEST(FrameBuffer3Test, LegacyFrameIdJumpBehavior) { FrameBuffer buffer(/*max_frame_slots=*/10, /*max_decode_history=*/100, field_trials); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(20).Id(3).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(3).AsLast().Build())); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), ElementsAre(FrameWithId(3))); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(30).Id(2).Refs({1}).AsLast().Build()); + EXPECT_FALSE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(30).Id(2).Refs({1}).AsLast().Build())); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), IsEmpty()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(40).Id(1).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(40).Id(1).AsLast().Build())); EXPECT_THAT(buffer.ExtractNextDecodableTemporalUnit(), ElementsAre(FrameWithId(1))); } @@ -324,20 +342,23 @@ TEST(FrameBuffer3Test, TotalNumberOfContinuousTemporalUnits) { field_trials); EXPECT_THAT(buffer.GetTotalNumberOfContinuousTemporalUnits(), Eq(0)); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build())); EXPECT_THAT(buffer.GetTotalNumberOfContinuousTemporalUnits(), Eq(1)); - buffer.InsertFrame(test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).Build())); EXPECT_THAT(buffer.GetTotalNumberOfContinuousTemporalUnits(), Eq(1)); - buffer.InsertFrame(test::FakeFrameBuilder().Time(40).Id(4).Refs({2}).Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(40).Id(5).Refs({3, 4}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(40).Id(4).Refs({2}).Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(40).Id(5).Refs({3, 4}).AsLast().Build())); EXPECT_THAT(buffer.GetTotalNumberOfContinuousTemporalUnits(), Eq(1)); // Reordered - buffer.InsertFrame( - test::FakeFrameBuilder().Time(20).Id(3).Refs({2}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(3).Refs({2}).AsLast().Build())); EXPECT_THAT(buffer.GetTotalNumberOfContinuousTemporalUnits(), Eq(3)); } @@ -347,13 +368,16 @@ TEST(FrameBuffer3Test, TotalNumberOfDroppedFrames) { field_trials); EXPECT_THAT(buffer.GetTotalNumberOfDroppedFrames(), Eq(0)); - buffer.InsertFrame(test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build()); - buffer.InsertFrame(test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(20).Id(3).Refs({2}).AsLast().Build()); - buffer.InsertFrame(test::FakeFrameBuilder().Time(40).Id(4).Refs({1}).Build()); - buffer.InsertFrame( - test::FakeFrameBuilder().Time(40).Id(5).Refs({4}).AsLast().Build()); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(10).Id(1).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(2).Refs({1}).Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(20).Id(3).Refs({2}).AsLast().Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(40).Id(4).Refs({1}).Build())); + EXPECT_TRUE(buffer.InsertFrame( + test::FakeFrameBuilder().Time(40).Id(5).Refs({4}).AsLast().Build())); buffer.ExtractNextDecodableTemporalUnit(); EXPECT_THAT(buffer.GetTotalNumberOfDroppedFrames(), Eq(0)); diff --git a/video/frame_buffer_proxy.cc b/video/frame_buffer_proxy.cc index 601e66ed7f..3657fba36c 100644 --- a/video/frame_buffer_proxy.cc +++ b/video/frame_buffer_proxy.cc @@ -252,9 +252,7 @@ class FrameBuffer3Proxy : public FrameBufferProxy { RTC_DCHECK_RUN_ON(&worker_sequence_checker_); FrameMetadata metadata(*frame); int complete_units = buffer_->GetTotalNumberOfContinuousTemporalUnits(); - size_t size = buffer_->CurrentSize(); - buffer_->InsertFrame(std::move(frame)); - if (size != buffer_->CurrentSize()) { + if (buffer_->InsertFrame(std::move(frame))) { RTC_DCHECK(metadata.receive_time) << "Frame receive time must be set!"; if (!metadata.delayed_by_retransmission && metadata.receive_time) timing_->IncomingTimestamp(metadata.rtp_timestamp,