diff --git a/video/BUILD.gn b/video/BUILD.gn index caa4f6d8dc..e8f7417b08 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -309,6 +309,7 @@ rtc_library("frame_buffer_proxy") { "../api/metronome", "../api/task_queue", "../api/video:encoded_frame", + "../api/video:video_rtp_headers", "../modules/video_coding", "../modules/video_coding:frame_buffer", "../modules/video_coding:frame_helpers", diff --git a/video/frame_buffer_proxy.cc b/video/frame_buffer_proxy.cc index a2e5406ece..31aa86f7ab 100644 --- a/video/frame_buffer_proxy.cc +++ b/video/frame_buffer_proxy.cc @@ -17,6 +17,8 @@ #include "absl/base/attributes.h" #include "absl/functional/bind_front.h" #include "api/sequence_checker.h" +#include "api/video/encoded_frame.h" +#include "api/video/video_content_type.h" #include "modules/video_coding/frame_buffer2.h" #include "modules/video_coding/frame_buffer3.h" #include "modules/video_coding/frame_helpers.h" @@ -138,6 +140,25 @@ static constexpr int kMaxFramesHistory = 1 << 13; // low-latency renderer is used. static constexpr size_t kZeroPlayoutDelayDefaultMaxDecodeQueueSize = 8; +struct FrameMetadata { + explicit FrameMetadata(const EncodedFrame& frame) + : is_last_spatial_layer(frame.is_last_spatial_layer), + is_keyframe(frame.is_keyframe()), + size(frame.size()), + contentType(frame.contentType()), + delayed_by_retransmission(frame.delayed_by_retransmission()), + rtp_timestamp(frame.Timestamp()), + receive_time(frame.ReceivedTime()) {} + + const bool is_last_spatial_layer; + const bool is_keyframe; + const size_t size; + const VideoContentType contentType; + const bool delayed_by_retransmission; + const uint32_t rtp_timestamp; + const int64_t receive_time; +}; + // Encapsulates use of the new frame buffer for use in VideoReceiveStream. This // behaves the same as the FrameBuffer2Proxy but uses frame_buffer3 instead. // Responsibilities from frame_buffer2, like stats, jitter and frame timing @@ -218,14 +239,19 @@ class FrameBuffer3Proxy : public FrameBufferProxy { absl::optional InsertFrame( std::unique_ptr frame) override { RTC_DCHECK_RUN_ON(&worker_sequence_checker_); - if (frame->is_last_spatial_layer) - stats_proxy_->OnCompleteFrame(frame->is_keyframe(), frame->size(), - frame->contentType()); - if (!frame->delayed_by_retransmission()) - timing_->IncomingTimestamp(frame->Timestamp(), frame->ReceivedTime()); - + FrameMetadata metadata(*frame); + int complete_units = buffer_->GetTotalNumberOfContinuousTemporalUnits(); buffer_->InsertFrame(std::move(frame)); - MaybeScheduleFrameForRelease(); + // Don't update stats or frame timing if the inserted frame did not complete + // a new temporal layer. + if (complete_units < buffer_->GetTotalNumberOfContinuousTemporalUnits()) { + stats_proxy_->OnCompleteFrame(metadata.is_keyframe, metadata.size, + metadata.contentType); + if (!metadata.delayed_by_retransmission) + timing_->IncomingTimestamp(metadata.rtp_timestamp, + metadata.receive_time); + MaybeScheduleFrameForRelease(); + } return buffer_->LastContinuousFrameId(); } diff --git a/video/frame_buffer_proxy_unittest.cc b/video/frame_buffer_proxy_unittest.cc index 073d30b5de..81e5bd2ffd 100644 --- a/video/frame_buffer_proxy_unittest.cc +++ b/video/frame_buffer_proxy_unittest.cc @@ -198,6 +198,12 @@ class VCMReceiveStatisticsCallbackMock : public VCMReceiveStatisticsCallback { (const TimingFrameInfo& info), (override)); }; + +bool IsFrameBuffer2Enabled() { + return field_trial::FindFullName("WebRTC-FrameBuffer3") + .find("arm:FrameBuffer2") != std::string::npos; +} + } // namespace constexpr auto kMaxWaitForKeyframe = TimeDelta::Millis(500); @@ -635,6 +641,64 @@ TEST_P(FrameBufferProxyTest, TestStatsCallback) { time_controller_.AdvanceTime(TimeDelta::Zero()); } +TEST_P(FrameBufferProxyTest, FrameCompleteCalledOnceForDuplicateFrame) { + EXPECT_CALL(stats_callback_, + OnCompleteFrame(true, kFrameSize, VideoContentType::UNSPECIFIED)) + .Times(1); + + StartNextDecodeForceKeyframe(); + proxy_->InsertFrame(Builder().Id(0).Time(0).AsLast().Build()); + proxy_->InsertFrame(Builder().Id(0).Time(0).AsLast().Build()); + // Flush stats posted on the decode queue. + time_controller_.AdvanceTime(TimeDelta::Zero()); +} + +TEST_P(FrameBufferProxyTest, FrameCompleteCalledOnceForSingleTemporalUnit) { + StartNextDecodeForceKeyframe(); + + // `OnCompleteFrame` should not be called for the first two frames since they + // do not complete the temporal layer. + EXPECT_CALL(stats_callback_, OnCompleteFrame(_, _, _)).Times(0); + proxy_->InsertFrame(Builder().Id(0).Time(0).Build()); + proxy_->InsertFrame(Builder().Id(1).Time(0).Refs({0}).Build()); + time_controller_.AdvanceTime(TimeDelta::Zero()); + // Flush stats posted on the decode queue. + ::testing::Mock::VerifyAndClearExpectations(&stats_callback_); + + // Note that this frame is not marked as a keyframe since the last spatial + // layer has dependencies. + EXPECT_CALL(stats_callback_, + OnCompleteFrame(false, kFrameSize, VideoContentType::UNSPECIFIED)) + .Times(1); + proxy_->InsertFrame(Builder().Id(2).Time(0).Refs({0, 1}).AsLast().Build()); + // Flush stats posted on the decode queue. + time_controller_.AdvanceTime(TimeDelta::Zero()); +} + +TEST_P(FrameBufferProxyTest, FrameCompleteCalledOnceForCompleteTemporalUnit) { + // FrameBuffer2 logs the complete frame on the arrival of the last layer. + if (IsFrameBuffer2Enabled()) + return; + StartNextDecodeForceKeyframe(); + + // `OnCompleteFrame` should not be called for the first two frames since they + // do not complete the temporal layer. Frame 1 arrives later, at which time + // this frame can finally be considered complete. + EXPECT_CALL(stats_callback_, OnCompleteFrame(_, _, _)).Times(0); + proxy_->InsertFrame(Builder().Id(0).Time(0).Build()); + proxy_->InsertFrame(Builder().Id(2).Time(0).Refs({0, 1}).AsLast().Build()); + time_controller_.AdvanceTime(TimeDelta::Zero()); + // Flush stats posted on the decode queue. + ::testing::Mock::VerifyAndClearExpectations(&stats_callback_); + + EXPECT_CALL(stats_callback_, + OnCompleteFrame(false, kFrameSize, VideoContentType::UNSPECIFIED)) + .Times(1); + proxy_->InsertFrame(Builder().Id(1).Time(0).Refs({0}).Build()); + // Flush stats posted on the decode queue. + time_controller_.AdvanceTime(TimeDelta::Zero()); +} + // Note: This test takes a long time to run if the fake metronome is active. // Since the test needs to wait for the timestamp to rollover, it has a fake // delay of around 6.5 hours. Even though time is simulated, this will be @@ -690,8 +754,7 @@ TEST_P(FrameBufferProxyTest, NextFrameWithOldTimestamp) { .AsLast() .Build()); // FrameBuffer2 drops the frame, while FrameBuffer3 will continue the stream. - if (field_trial::FindFullName("WebRTC-FrameBuffer3") - .find("arm:FrameBuffer2") == std::string::npos) { + if (!IsFrameBuffer2Enabled()) { EXPECT_THAT(WaitForFrameOrTimeout(kFps30Delay), Frame(WithId(2))); } else { EXPECT_THAT(WaitForFrameOrTimeout(kMaxWaitForFrame), TimedOut());