From f7f0b2108fed0471423bee53d1cf2b113cccb525 Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Tue, 24 May 2022 18:39:39 +0200 Subject: [PATCH] ZeroHz: Repeat refresh frame requests until a frame is received. When MediaStreamVideoSource::RequestRefreshFrame is called, the capturer most often emits a refresh frame. Due to various conditions such as for example timing of prior delivery, these frames can be dropped at various places in the input pipeline into WebRTC. This change ensures the frame cadence adapter repeatedly requests refresh frames at max fps frequency until one is received, in which case the requests cease. Fixed: chromium:1324120 Change-Id: I90f85d31b132b6c441aa1c28c5eff85e3dc365ad Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/263520 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Markus Handell Cr-Commit-Position: refs/heads/main@{#36998} --- video/BUILD.gn | 1 + video/frame_cadence_adapter.cc | 39 +++++++++++-------------- video/frame_cadence_adapter_unittest.cc | 26 +++++++++++++++-- 3 files changed, 41 insertions(+), 25 deletions(-) diff --git a/video/BUILD.gn b/video/BUILD.gn index 1e462bb011..aaf97cfd4a 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -318,6 +318,7 @@ rtc_library("frame_cadence_adapter") { "../rtc_base/synchronization:mutex", "../rtc_base/system:no_unique_address", "../rtc_base/task_utils:pending_task_safety_flag", + "../rtc_base/task_utils:repeating_task", "../rtc_base/task_utils:to_queued_task", "../system_wrappers", "../system_wrappers:field_trial", diff --git a/video/frame_cadence_adapter.cc b/video/frame_cadence_adapter.cc index 879cff71a1..b6e21d66c0 100644 --- a/video/frame_cadence_adapter.cc +++ b/video/frame_cadence_adapter.cc @@ -30,6 +30,7 @@ #include "rtc_base/synchronization/mutex.h" #include "rtc_base/system/no_unique_address.h" #include "rtc_base/task_utils/pending_task_safety_flag.h" +#include "rtc_base/task_utils/repeating_task.h" #include "rtc_base/task_utils/to_queued_task.h" #include "rtc_base/thread_annotations.h" #include "rtc_base/time_utils.h" @@ -204,6 +205,10 @@ class ZeroHertzAdapterMode : public AdapterMode { // Convergent state of each of the configured simulcast layers. std::vector layer_trackers_ RTC_GUARDED_BY(sequence_checker_); + // Repeating task handle used for requesting refresh frames until arrival, as + // they can be dropped in various places in the capture pipeline. + RepeatingTaskHandle refresh_frame_requester_ + RTC_GUARDED_BY(sequence_checker_); ScopedTaskSafety safety_; }; @@ -286,9 +291,6 @@ class FrameCadenceAdapterImpl : public FrameCadenceAdapterInterface { // `queue_`. std::atomic frames_scheduled_for_processing_{0}; - // Whether to ask for a refresh frame on activation of zero-hertz mode. - bool should_request_refresh_frame_ RTC_GUARDED_BY(queue_) = false; - ScopedTaskSafetyDetached safety_; }; @@ -299,13 +301,19 @@ 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_; + }); } void ZeroHertzAdapterMode::ReconfigureParameters( const FrameCadenceAdapterInterface::ZeroHertzModeParams& params) { RTC_DCHECK_RUN_ON(&sequence_checker_); - RTC_LOG(LS_INFO) << __func__ << " this " << this << " num_simulcast_layers " - << params.num_simulcast_layers; + RTC_DLOG(LS_INFO) << __func__ << " this " << this << " num_simulcast_layers " + << params.num_simulcast_layers; // Start as unconverged. layer_trackers_.clear(); @@ -350,6 +358,7 @@ void ZeroHertzAdapterMode::OnFrame(Timestamp post_time, RTC_DCHECK_RUN_ON(&sequence_checker_); RTC_DLOG(LS_VERBOSE) << "ZeroHertzAdapterMode::" << __func__ << " this " << this; + refresh_frame_requester_.Stop(); // Assume all enabled layers are unconverged after frame entry. ResetQualityConvergenceInfo(); @@ -383,15 +392,9 @@ absl::optional ZeroHertzAdapterMode::GetInputFrameRateFps() { void ZeroHertzAdapterMode::ProcessKeyFrameRequest() { RTC_DCHECK_RUN_ON(&sequence_checker_); - // If no frame was ever passed to us, request a refresh frame from the source. - if (current_frame_id_ == 0) { - RTC_LOG(LS_INFO) - << __func__ << " this " << this - << " requesting refresh frame due to no frames received yet."; - callback_->RequestRefreshFrame(); - return; - } - + // If we're new and don't have a frame, there's no need to request refresh + // frames as this was being triggered for us when zero-hz mode was set up. + // // The next frame encoded will be a key frame. Reset quality convergence so we // don't get idle repeats shortly after, because key frames need a lot of // refinement frames. @@ -609,8 +612,6 @@ void FrameCadenceAdapterImpl::ProcessKeyFrameRequest() { RTC_DCHECK_RUN_ON(queue_); if (zero_hertz_adapter_) zero_hertz_adapter_->ProcessKeyFrameRequest(); - else - should_request_refresh_frame_ = true; } void FrameCadenceAdapterImpl::OnFrame(const VideoFrame& frame) { @@ -682,12 +683,6 @@ void FrameCadenceAdapterImpl::MaybeReconfigureAdapters( zero_hertz_adapter_.emplace(queue_, clock_, callback_, source_constraints_->max_fps.value()); RTC_LOG(LS_INFO) << "Zero hertz mode activated."; - - if (should_request_refresh_frame_) { - // Ensure we get a first frame to work with. - should_request_refresh_frame_ = false; - callback_->RequestRefreshFrame(); - } zero_hertz_adapter_created_timestamp_ = clock_->CurrentTime(); } zero_hertz_adapter_->ReconfigureParameters(zero_hertz_params_.value()); diff --git a/video/frame_cadence_adapter_unittest.cc b/video/frame_cadence_adapter_unittest.cc index 823c1a2a6b..cfd471d6a4 100644 --- a/video/frame_cadence_adapter_unittest.cc +++ b/video/frame_cadence_adapter_unittest.cc @@ -169,7 +169,6 @@ TEST(FrameCadenceAdapterTest, TEST(FrameCadenceAdapterTest, FrameRateFollowsMaxFpsWhenZeroHertzActivated) { ZeroHertzFieldTrialEnabler enabler; - MockCallback callback; GlobalSimulatedTimeController time_controller(Timestamp::Millis(0)); auto adapter = CreateAdapter(enabler, time_controller.GetClock()); adapter->Initialize(nullptr); @@ -186,7 +185,6 @@ TEST(FrameCadenceAdapterTest, FrameRateFollowsMaxFpsWhenZeroHertzActivated) { TEST(FrameCadenceAdapterTest, FrameRateFollowsRateStatisticsAfterZeroHertzDeactivated) { ZeroHertzFieldTrialEnabler enabler; - MockCallback callback; GlobalSimulatedTimeController time_controller(Timestamp::Millis(0)); auto adapter = CreateAdapter(enabler, time_controller.GetClock()); adapter->Initialize(nullptr); @@ -372,8 +370,8 @@ TEST(FrameCadenceAdapterTest, RequestsRefreshFrameOnKeyFrameRequestWhenNew) { adapter->SetZeroHertzModeEnabled( FrameCadenceAdapterInterface::ZeroHertzModeParams{}); adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, 10}); - time_controller.AdvanceTime(TimeDelta::Zero()); EXPECT_CALL(callback, RequestRefreshFrame); + time_controller.AdvanceTime(TimeDelta::Zero()); adapter->ProcessKeyFrameRequest(); } @@ -392,6 +390,28 @@ TEST(FrameCadenceAdapterTest, IgnoresKeyFrameRequestShortlyAfterFrame) { adapter->ProcessKeyFrameRequest(); } +TEST(FrameCadenceAdapterTest, RequestsRefreshFramesUntilArrival) { + 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}); + + // 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); + time_controller.AdvanceTime(TimeDelta::Seconds(1)); + Mock::VerifyAndClearExpectations(&callback); + adapter->OnFrame(CreateFrame()); + EXPECT_CALL(callback, RequestRefreshFrame).Times(0); + time_controller.AdvanceTime(TimeDelta::Seconds(1)); +} + class FrameCadenceAdapterSimulcastLayersParamTest : public ::testing::TestWithParam { public: