From f5a507955a318fb98ab775233cea96c021ce8a83 Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Thu, 16 Jun 2022 14:25:15 +0200 Subject: [PATCH] FrameCadenceAdapter: ensure frame arrival after drop. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL accomplishes three things: 1) It enables feeding frame drop indications into the AdaptedVideoTrackSource for the benefit of downstream projects. 2) Under zero hertz source delivery, a discarded frame ending a sequence of frames which happened to contain important information can be seen as a capture freeze. Avoid this by starting requesting refresh frames after a grace period. 3) It changes the duration until first refresh frame requests on new streams to three frame periods. Bug: chromium:1324120, chromium:1336952 Change-Id: I0214852f1a26540588f6c193dd88a65c34ec0d99 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265871 Reviewed-by: Erik Språng Commit-Queue: Markus Handell Cr-Commit-Position: refs/heads/main@{#37238} --- media/base/adapted_video_track_source.cc | 4 + media/base/adapted_video_track_source.h | 2 + video/frame_cadence_adapter.cc | 54 +++++++++++-- video/frame_cadence_adapter.h | 3 + video/frame_cadence_adapter_unittest.cc | 99 ++++++++++++++++++++++-- video/video_stream_encoder_unittest.cc | 9 ++- 6 files changed, 156 insertions(+), 15 deletions(-) diff --git a/media/base/adapted_video_track_source.cc b/media/base/adapted_video_track_source.cc index f8f8f2dad1..816ada5f16 100644 --- a/media/base/adapted_video_track_source.cc +++ b/media/base/adapted_video_track_source.cc @@ -61,6 +61,10 @@ void AdaptedVideoTrackSource::OnFrame(const webrtc::VideoFrame& frame) { } } +void AdaptedVideoTrackSource::OnFrameDropped() { + broadcaster_.OnDiscardedFrame(); +} + void AdaptedVideoTrackSource::AddOrUpdateSink( rtc::VideoSinkInterface* sink, const rtc::VideoSinkWants& wants) { diff --git a/media/base/adapted_video_track_source.h b/media/base/adapted_video_track_source.h index 1386fbd9db..1c3e0b68d3 100644 --- a/media/base/adapted_video_track_source.h +++ b/media/base/adapted_video_track_source.h @@ -45,6 +45,8 @@ class RTC_EXPORT AdaptedVideoTrackSource // plain memory frame, it is rotated. Subclasses producing native frames must // handle apply_rotation() themselves. void OnFrame(const webrtc::VideoFrame& frame); + // Indication from source that a frame was dropped. + void OnFrameDropped(); // Reports the appropriate frame size after adaptation. Returns true // if a frame is wanted. Returns false if there are no interested diff --git a/video/frame_cadence_adapter.cc b/video/frame_cadence_adapter.cc index b6e21d66c0..94fd31882b 100644 --- a/video/frame_cadence_adapter.cc +++ b/video/frame_cadence_adapter.cc @@ -120,6 +120,9 @@ class ZeroHertzAdapterMode : public AdapterMode { absl::optional GetInputFrameRateFps() override; void UpdateFrameRate() override {} + // Notified on dropped frames. + void OnDiscardedFrame(); + // Conditionally requests a refresh frame via // Callback::RequestRefreshFrame. void ProcessKeyFrameRequest(); @@ -182,6 +185,10 @@ class ZeroHertzAdapterMode : public AdapterMode { // Returns the repeat duration depending on if it's an idle repeat or not. TimeDelta RepeatDuration(bool idle_repeat) const RTC_RUN_ON(sequence_checker_); + // Unless timer already running, starts repeatedly requesting refresh frames + // after a grace_period. If a frame appears before the grace_period has + // passed, the request is cancelled. + void MaybeStartRefreshFrameRequester() RTC_RUN_ON(sequence_checker_); TaskQueueBase* const queue_; Clock* const clock_; @@ -233,7 +240,7 @@ class FrameCadenceAdapterImpl : public FrameCadenceAdapterInterface { // VideoFrameSink overrides. void OnFrame(const VideoFrame& frame) override; - void OnDiscardedFrame() override { callback_->OnDiscardedFrame(); } + void OnDiscardedFrame() override; void OnConstraintsChanged( const VideoTrackSourceConstraints& constraints) override; @@ -301,12 +308,7 @@ ZeroHertzAdapterMode::ZeroHertzAdapterMode( double max_fps) : queue_(queue), clock_(clock), callback_(callback), max_fps_(max_fps) { sequence_checker_.Detach(); - refresh_frame_requester_ = RepeatingTaskHandle::Start(queue_, [this] { - RTC_DLOG(LS_VERBOSE) << __func__ << " RequestRefreshFrame"; - if (callback_) - callback_->RequestRefreshFrame(); - return frame_delay_; - }); + MaybeStartRefreshFrameRequester(); } void ZeroHertzAdapterMode::ReconfigureParameters( @@ -384,6 +386,17 @@ void ZeroHertzAdapterMode::OnFrame(Timestamp post_time, frame_delay_.ms()); } +void ZeroHertzAdapterMode::OnDiscardedFrame() { + RTC_DCHECK_RUN_ON(&sequence_checker_); + RTC_DLOG(LS_VERBOSE) << "ZeroHertzAdapterMode::" << __func__; + + // Under zero hertz source delivery, a discarded frame ending a sequence of + // frames which happened to contain important information can be seen as a + // capture freeze. Avoid this by starting requesting refresh frames after a + // grace period. + MaybeStartRefreshFrameRequester(); +} + absl::optional ZeroHertzAdapterMode::GetInputFrameRateFps() { RTC_DCHECK_RUN_ON(&sequence_checker_); return max_fps_; @@ -552,6 +565,23 @@ TimeDelta ZeroHertzAdapterMode::RepeatDuration(bool idle_repeat) const { : frame_delay_; } +// RTC_RUN_ON(&sequence_checker_) +void ZeroHertzAdapterMode::MaybeStartRefreshFrameRequester() { + RTC_DLOG(LS_VERBOSE) << __func__; + if (!refresh_frame_requester_.Running()) { + refresh_frame_requester_ = RepeatingTaskHandle::DelayedStart( + queue_, + FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod * + frame_delay_, + [this] { + RTC_DLOG(LS_VERBOSE) << __func__ << " RequestRefreshFrame"; + if (callback_) + callback_->RequestRefreshFrame(); + return frame_delay_; + }); + } +} + FrameCadenceAdapterImpl::FrameCadenceAdapterImpl( Clock* clock, TaskQueueBase* queue, @@ -644,6 +674,16 @@ void FrameCadenceAdapterImpl::OnFrame(const VideoFrame& frame) { })); } +void FrameCadenceAdapterImpl::OnDiscardedFrame() { + callback_->OnDiscardedFrame(); + queue_->PostTask(ToQueuedTask(safety_.flag(), [this] { + RTC_DCHECK_RUN_ON(queue_); + if (zero_hertz_adapter_) { + zero_hertz_adapter_->OnDiscardedFrame(); + } + })); +} + void FrameCadenceAdapterImpl::OnConstraintsChanged( const VideoTrackSourceConstraints& constraints) { RTC_LOG(LS_INFO) << __func__ << " this " << this << " min_fps " diff --git a/video/frame_cadence_adapter.h b/video/frame_cadence_adapter.h index 40e98059c0..149a915624 100644 --- a/video/frame_cadence_adapter.h +++ b/video/frame_cadence_adapter.h @@ -41,6 +41,9 @@ class FrameCadenceAdapterInterface // and some worst case RTT. static constexpr TimeDelta kZeroHertzIdleRepeatRatePeriod = TimeDelta::Millis(1000); + // The number of frame periods to wait for new frames until starting to + // request refresh frames. + static constexpr int kOnDiscardedFrameRefreshFramePeriod = 3; struct ZeroHertzModeParams { // The number of simulcast layers used in this configuration. diff --git a/video/frame_cadence_adapter_unittest.cc b/video/frame_cadence_adapter_unittest.cc index 9bd312a433..3a2767a078 100644 --- a/video/frame_cadence_adapter_unittest.cc +++ b/video/frame_cadence_adapter_unittest.cc @@ -368,9 +368,13 @@ TEST(FrameCadenceAdapterTest, RequestsRefreshFrameOnKeyFrameRequestWhenNew) { adapter->Initialize(&callback); adapter->SetZeroHertzModeEnabled( FrameCadenceAdapterInterface::ZeroHertzModeParams{}); - adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, 10}); + constexpr int kMaxFps = 10; + adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFps}); EXPECT_CALL(callback, RequestRefreshFrame); - time_controller.AdvanceTime(TimeDelta::Zero()); + time_controller.AdvanceTime( + TimeDelta::Seconds(1) * + FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod / + kMaxFps); adapter->ProcessKeyFrameRequest(); } @@ -400,10 +404,14 @@ TEST(FrameCadenceAdapterTest, RequestsRefreshFramesUntilArrival) { constexpr int kMaxFps = 10; adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFps}); - // We should see max_fps + 1 refresh frame requests during the one second we - // wait until we send a single frame, after which refresh frame requests - // should cease (we should see no such requests during a second). - EXPECT_CALL(callback, RequestRefreshFrame).Times(kMaxFps + 1); + // We should see max_fps + 1 - + // FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod refresh + // frame requests during the one second we wait until we send a single frame, + // after which refresh frame requests should cease (we should see no such + // requests during a second). + EXPECT_CALL(callback, RequestRefreshFrame) + .Times(kMaxFps + 1 - + FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod); time_controller.AdvanceTime(TimeDelta::Seconds(1)); Mock::VerifyAndClearExpectations(&callback); adapter->OnFrame(CreateFrame()); @@ -411,6 +419,85 @@ TEST(FrameCadenceAdapterTest, RequestsRefreshFramesUntilArrival) { time_controller.AdvanceTime(TimeDelta::Seconds(1)); } +TEST(FrameCadenceAdapterTest, RequestsRefreshAfterFrameDrop) { + ZeroHertzFieldTrialEnabler enabler; + MockCallback callback; + GlobalSimulatedTimeController time_controller(Timestamp::Millis(0)); + auto adapter = CreateAdapter(enabler, time_controller.GetClock()); + adapter->Initialize(&callback); + adapter->SetZeroHertzModeEnabled( + FrameCadenceAdapterInterface::ZeroHertzModeParams{}); + constexpr int kMaxFps = 10; + adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFps}); + + EXPECT_CALL(callback, RequestRefreshFrame).Times(0); + + // Send a frame through to cancel the initial delayed timer waiting for first + // frame entry. + adapter->OnFrame(CreateFrame()); + time_controller.AdvanceTime(TimeDelta::Seconds(1)); + Mock::VerifyAndClearExpectations(&callback); + + // Send a dropped frame indication without any following frames received. + // After FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod + // frame periods, we should receive a first refresh request. + adapter->OnDiscardedFrame(); + EXPECT_CALL(callback, RequestRefreshFrame); + time_controller.AdvanceTime( + TimeDelta::Seconds(1) * + FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod / + kMaxFps); + Mock::VerifyAndClearExpectations(&callback); + + // We will now receive a refresh frame request for every frame period. + EXPECT_CALL(callback, RequestRefreshFrame).Times(kMaxFps); + time_controller.AdvanceTime(TimeDelta::Seconds(1)); + Mock::VerifyAndClearExpectations(&callback); + + // After a frame is passed the requests will cease. + EXPECT_CALL(callback, RequestRefreshFrame).Times(0); + adapter->OnFrame(CreateFrame()); + time_controller.AdvanceTime(TimeDelta::Seconds(1)); +} + +TEST(FrameCadenceAdapterTest, OmitsRefreshAfterFrameDropWithTimelyFrameEntry) { + ZeroHertzFieldTrialEnabler enabler; + MockCallback callback; + GlobalSimulatedTimeController time_controller(Timestamp::Millis(0)); + auto adapter = CreateAdapter(enabler, time_controller.GetClock()); + adapter->Initialize(&callback); + adapter->SetZeroHertzModeEnabled( + FrameCadenceAdapterInterface::ZeroHertzModeParams{}); + constexpr int kMaxFps = 10; + adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFps}); + + // Send a frame through to cancel the initial delayed timer waiting for first + // frame entry. + EXPECT_CALL(callback, RequestRefreshFrame).Times(0); + adapter->OnFrame(CreateFrame()); + time_controller.AdvanceTime(TimeDelta::Seconds(1)); + Mock::VerifyAndClearExpectations(&callback); + + // Send a frame drop indication. No refresh frames should be requested + // until FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod + // intervals pass. Stop short of this. + EXPECT_CALL(callback, RequestRefreshFrame).Times(0); + adapter->OnDiscardedFrame(); + time_controller.AdvanceTime( + TimeDelta::Seconds(1) * + FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod / + kMaxFps - + TimeDelta::Micros(1)); + Mock::VerifyAndClearExpectations(&callback); + + // Send a frame. The timer to request the refresh frame should be cancelled by + // the reception, so no refreshes should be requested. + EXPECT_CALL(callback, RequestRefreshFrame).Times(0); + adapter->OnFrame(CreateFrame()); + time_controller.AdvanceTime(TimeDelta::Seconds(1)); + Mock::VerifyAndClearExpectations(&callback); +} + class FrameCadenceAdapterSimulcastLayersParamTest : public ::testing::TestWithParam { public: diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 5436df8f99..575a8e24f5 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -26,6 +26,7 @@ #include "api/test/mock_video_encoder.h" #include "api/test/mock_video_encoder_factory.h" #include "api/units/data_rate.h" +#include "api/units/time_delta.h" #include "api/video/builtin_video_bitrate_allocator_factory.h" #include "api/video/i420_buffer.h" #include "api/video/nv12_buffer.h" @@ -9410,8 +9411,12 @@ TEST(VideoStreamEncoderFrameCadenceTest, // synchronized. EXPECT_CALL(mock_source, RequestRefreshFrame); video_stream_encoder->SendKeyFrame(); - adapter_ptr->OnConstraintsChanged(VideoTrackSourceConstraints{0, 30}); - factory.DepleteTaskQueues(); + constexpr int kMaxFps = 30; + adapter_ptr->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFps}); + factory.GetTimeController()->AdvanceTime( + TimeDelta::Seconds(1) * + FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod / + kMaxFps); } } // namespace webrtc