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 <ilnik@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36998}

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",

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<SpatialLayerTracker> 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<int> 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 "
+  RTC_DLOG(LS_INFO) << __func__ << " this " << this << " num_simulcast_layers "
-                   << params.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<uint32_t> 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 we're new and don't have a frame, there's no need to request refresh
-  if (current_frame_id_ == 0) {
+  // frames as this was being triggered for us when zero-hz mode was set up.
-    RTC_LOG(LS_INFO)
+  //
-        << __func__ << " this " << this
-        << " requesting refresh frame due to no frames received yet.";
-    callback_->RequestRefreshFrame();
-    return;
-  }
-
   // 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());

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<int> {
  public: