From 644025c51f75aebc7cc270f121d2f648bb856863 Mon Sep 17 00:00:00 2001 From: henrika Date: Tue, 7 Nov 2023 18:22:02 +0100 Subject: [PATCH] FrameCadenceAdapter: keeps max_fps in sync with constraints during 0Hz Before this change, the FCA did not not update its cadence when max_fps was changed and zero-hertz was already enabled. See https://paste.googleplex.com/6300124249587712 for more details. Bug: chromium:1400204 Change-Id: I95d80bdfa85ecac8681784b2b29e98d1a587ba53 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/326105 Reviewed-by: Markus Handell Commit-Queue: Henrik Andreassson Cr-Commit-Position: refs/heads/main@{#41100} --- video/frame_cadence_adapter.cc | 11 ++++++----- video/frame_cadence_adapter_unittest.cc | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/video/frame_cadence_adapter.cc b/video/frame_cadence_adapter.cc index 6b24f30e57..0781651fec 100644 --- a/video/frame_cadence_adapter.cc +++ b/video/frame_cadence_adapter.cc @@ -294,7 +294,6 @@ class FrameCadenceAdapterImpl : public FrameCadenceAdapterInterface { // Race checker for incoming frames. This is the network thread in chromium, // but may vary from test contexts. rtc::RaceChecker incoming_frame_race_checker_; - bool has_reported_screenshare_frame_rate_umas_ RTC_GUARDED_BY(queue_) = false; // Number of frames that are currently scheduled for processing on the // `queue_`. @@ -610,8 +609,6 @@ void FrameCadenceAdapterImpl::SetZeroHertzModeEnabled( absl::optional params) { RTC_DCHECK_RUN_ON(queue_); bool was_zero_hertz_enabled = zero_hertz_params_.has_value(); - if (params.has_value() && !was_zero_hertz_enabled) - has_reported_screenshare_frame_rate_umas_ = false; zero_hertz_params_ = params; MaybeReconfigureAdapters(was_zero_hertz_enabled); } @@ -735,11 +732,14 @@ void FrameCadenceAdapterImpl::MaybeReconfigureAdapters( RTC_DCHECK_RUN_ON(queue_); bool is_zero_hertz_enabled = IsZeroHertzScreenshareEnabled(); if (is_zero_hertz_enabled) { - if (!was_zero_hertz_enabled) { + bool max_fps_has_changed = GetInputFrameRateFps().value_or(-1) != + source_constraints_->max_fps.value_or(-1); + if (!was_zero_hertz_enabled || max_fps_has_changed) { zero_hertz_adapter_.emplace(queue_, clock_, callback_, source_constraints_->max_fps.value()); zero_hertz_adapter_is_active_.store(true, std::memory_order_relaxed); - RTC_LOG(LS_INFO) << "Zero hertz mode activated."; + RTC_LOG(LS_INFO) << "Zero hertz mode enabled (max_fps=" + << source_constraints_->max_fps.value() << ")"; zero_hertz_adapter_created_timestamp_ = clock_->CurrentTime(); } zero_hertz_adapter_->ReconfigureParameters(zero_hertz_params_.value()); @@ -748,6 +748,7 @@ void FrameCadenceAdapterImpl::MaybeReconfigureAdapters( if (was_zero_hertz_enabled) { zero_hertz_adapter_ = absl::nullopt; zero_hertz_adapter_is_active_.store(false, std::memory_order_relaxed); + RTC_LOG(LS_INFO) << "Zero hertz mode disabled."; } current_adapter_mode_ = &passthrough_adapter_.value(); } diff --git a/video/frame_cadence_adapter_unittest.cc b/video/frame_cadence_adapter_unittest.cc index 052b0a6c61..11be45070d 100644 --- a/video/frame_cadence_adapter_unittest.cc +++ b/video/frame_cadence_adapter_unittest.cc @@ -184,6 +184,29 @@ TEST(FrameCadenceAdapterTest, FrameRateFollowsMaxFpsWhenZeroHertzActivated) { } } +TEST(FrameCadenceAdapterTest, ZeroHertzAdapterSupportsMaxFpsChange) { + ZeroHertzFieldTrialEnabler enabler; + GlobalSimulatedTimeController time_controller(Timestamp::Zero()); + auto adapter = CreateAdapter(enabler, time_controller.GetClock()); + MockCallback callback; + adapter->Initialize(&callback); + adapter->SetZeroHertzModeEnabled( + FrameCadenceAdapterInterface::ZeroHertzModeParams{}); + adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, 1}); + time_controller.AdvanceTime(TimeDelta::Zero()); + EXPECT_EQ(adapter->GetInputFrameRateFps(), 1u); + adapter->OnFrame(CreateFrame()); + time_controller.AdvanceTime(TimeDelta::Seconds(1)); + adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, 2}); + time_controller.AdvanceTime(TimeDelta::Zero()); + EXPECT_EQ(adapter->GetInputFrameRateFps(), 2u); + adapter->OnFrame(CreateFrame()); + // Ensure that the max_fps has been changed from 1 to 2 fps even if it was + // changed while zero hertz was already active. + EXPECT_CALL(callback, OnFrame); + time_controller.AdvanceTime(TimeDelta::Millis(500)); +} + TEST(FrameCadenceAdapterTest, FrameRateFollowsRateStatisticsAfterZeroHertzDeactivated) { ZeroHertzFieldTrialEnabler enabler;