M115: Update VideoTrackAdapter frame rate reduction logic

VideoTrackAdapter drops frames in order to make sure that the output frame rate is close to the target frame rate despite an arbitrary
input frame rate.

However, too many frames were dropped occasionally. After digging into this I've found that the logic was very sensitive to jitter in the
input.

If the input timestamps are 10 fps with the following pattern:
0, 10 ms, 200 ms, 210 ms, 400 ms, 410 ms, 600 ms, 610 ms,...

and the max fps is set to 5 fps, VideoTrackAdapter produced something
like this:
0, 400 ms, 1000 ms, 1800 ms, 2610 ms, 3800 ms, 4800 ms, 5800 ms,...

which is roughly 1 fps. With the current CL, the output is as expected:
0, 200 ms, 400 ms, 600 ms, 800 ms, etc.


(cherry picked from commit affcc0f)

Bug: chromium:1448046
Change-Id: I71289a7a3986299c447e3d505cf07b52641e4aff
Low-Coverage-Reason: Removing unused field.
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4555847
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Johannes Kron <kron@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1150071}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4575214
Cr-Commit-Position: refs/branch-heads/5790@{#252}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}

media/capture/mojom/video_capture_types.mojom

@@ -281,7 +281,7 @@ enum VideoCaptureFrameDropReason {
   kVideoTrackAdapterHasNoResolutionAdapters,
   kResolutionAdapterFrameIsNotValid,
   kResolutionAdapterWrappingFrameForCroppingFailed,
-  kResolutionAdapterTimestampTooCloseToPrevious,
+  kResolutionAdapterTimestampTooCloseToPrevious_DEPRECATED,
   kResolutionAdapterFrameRateIsHigherThanRequested,
   kResolutionAdapterHasNoCallbacks,
   kVideoTrackFrameDelivererNotEnabledReplacingWithBlackFrame,

media/capture/mojom/video_capture_types_mojom_traits.cc

@@ -1525,10 +1525,6 @@ EnumTraits<media::mojom::VideoCaptureFrameDropReason,
         kResolutionAdapterWrappingFrameForCroppingFailed:
       return media::mojom::VideoCaptureFrameDropReason::
           kResolutionAdapterWrappingFrameForCroppingFailed;
-    case media::VideoCaptureFrameDropReason::
-        kResolutionAdapterTimestampTooCloseToPrevious:
-      return media::mojom::VideoCaptureFrameDropReason::
-          kResolutionAdapterTimestampTooCloseToPrevious;
     case media::VideoCaptureFrameDropReason::
         kResolutionAdapterFrameRateIsHigherThanRequested:
       return media::mojom::VideoCaptureFrameDropReason::
@@ -1659,10 +1655,9 @@ bool EnumTraits<media::mojom::VideoCaptureFrameDropReason,
           kResolutionAdapterWrappingFrameForCroppingFailed;
       return true;
     case media::mojom::VideoCaptureFrameDropReason::
-        kResolutionAdapterTimestampTooCloseToPrevious:
-      *output = media::VideoCaptureFrameDropReason::
-          kResolutionAdapterTimestampTooCloseToPrevious;
-      return true;
+        kResolutionAdapterTimestampTooCloseToPrevious_DEPRECATED:
+      NOTREACHED();
+      return false;
     case media::mojom::VideoCaptureFrameDropReason::
         kResolutionAdapterFrameRateIsHigherThanRequested:
       *output = media::VideoCaptureFrameDropReason::

media/capture/video_capture_types.h

@@ -236,7 +236,7 @@ enum class VideoCaptureFrameDropReason {
   kVideoTrackAdapterHasNoResolutionAdapters = 19,
   kResolutionAdapterFrameIsNotValid = 20,
   kResolutionAdapterWrappingFrameForCroppingFailed = 21,
-  kResolutionAdapterTimestampTooCloseToPrevious = 22,
+  // kResolutionAdapterTimestampTooCloseToPrevious = 22, // combined into 23.
   kResolutionAdapterFrameRateIsHigherThanRequested = 23,
   kResolutionAdapterHasNoCallbacks = 24,
   kVideoTrackFrameDelivererNotEnabledReplacingWithBlackFrame = 25,

third_party/blink/renderer/modules/mediastream/media_stream_video_source_test.cc

@@ -550,18 +550,17 @@ TEST_F(MediaStreamVideoSourceTest, ForwardsAtMaxFrameRateAndDropsWhenTooClose) {
   base::OnceClosure quit_closure = run_loop.QuitClosure();
 
   EXPECT_CALL(sink, OnVideoFrame).Times(3).WillRepeatedly(Return());
-  EXPECT_CALL(*mock_source(),
-              OnFrameDropped(media::VideoCaptureFrameDropReason::
-                                 kResolutionAdapterTimestampTooCloseToPrevious))
+  EXPECT_CALL(
+      *mock_source(),
+      OnFrameDropped(media::VideoCaptureFrameDropReason::
+                         kResolutionAdapterFrameRateIsHigherThanRequested))
       .Times(1)
       .WillOnce([&] { std::move(quit_closure).Run(); });
 
   DeliverVideoFrame(100, 100, base::Milliseconds(100));
   DeliverVideoFrame(100, 100, base::Milliseconds(200));
   DeliverVideoFrame(100, 100, base::Milliseconds(300));
-  DeliverVideoFrame(
-      100, 100,
-      base::Milliseconds(300 + VideoTrackAdapter::kMinTimeBetweenFramesMs - 1));
+  DeliverVideoFrame(100, 100, base::Milliseconds(304));
   run_loop.Run();
   EXPECT_EQ(3, sink.number_of_frames());
 

third_party/blink/renderer/modules/mediastream/video_track_adapter.cc

@@ -24,6 +24,7 @@
 #include "build/build_config.h"
 #include "media/base/limits.h"
 #include "media/base/video_util.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 #include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/renderer/modules/mediastream/video_track_adapter_settings.h"
@@ -57,10 +58,11 @@ namespace {
 const float kFirstFrameTimeoutInFrameIntervals = 100.0f;
 const float kNormalFrameTimeoutInFrameIntervals = 25.0f;
 
-// Min delta time between two frames allowed without being dropped if a max
-// frame rate is specified.
-constexpr base::TimeDelta kMinTimeBetweenFrames =
-    base::Milliseconds(VideoTrackAdapter::kMinTimeBetweenFramesMs);
+// |kMaxDeltaDeviationFactor| is used to determine |max_delta_deviation_| which
+// specifies the allowed deviation from |target_delta_| before dropping a frame.
+// It's set to 20% to be aligned with the previous logic in this file.
+constexpr float kMaxDeltaDeviationFactor = 0.2;
+
 // If the delta between two frames is bigger than this, we will consider it to
 // be invalid and reset the fps calculation.
 constexpr base::TimeDelta kMaxTimeBetweenFrames = base::Milliseconds(1000);
@@ -128,6 +130,20 @@ bool MaybeUpdateFrameRate(ComputedSettings* settings) {
   return false;
 }
 
+VideoTrackAdapterSettings ReturnSettingsMaybeOverrideMaxFps(
+    const VideoTrackAdapterSettings& settings) {
+  VideoTrackAdapterSettings new_settings = settings;
+  absl::optional<double> max_fps_override =
+      Platform::Current()->GetWebRtcMaxCaptureFrameRate();
+  if (max_fps_override) {
+    DVLOG(1) << "Overriding max frame rate.  Was="
+             << settings.max_frame_rate().value_or(-1)
+             << ", Now=" << *max_fps_override;
+    new_settings.set_max_frame_rate(*max_fps_override);
+  }
+  return new_settings;
+}
+
 }  // anonymous namespace
 
 // VideoFrameResolutionAdapter is created on and lives on the video task runner.
@@ -242,10 +258,25 @@ class VideoTrackAdapter::VideoFrameResolutionAdapter
 
   base::WeakPtr<MediaStreamVideoSource> media_stream_video_source_;
 
-  VideoTrackAdapterSettings settings_;
-  double measured_input_frame_rate_ = MediaStreamVideoSource::kDefaultFrameRate;
-  base::TimeDelta last_time_stamp_ = base::TimeDelta::Max();
-  double fractional_frames_due_ = 0.0;
+  const VideoTrackAdapterSettings settings_;
+
+  // The target timestamp delta between video frames, corresponding to the max
+  // fps.
+  const absl::optional<base::TimeDelta> target_delta_;
+
+  // The maximum allowed deviation from |target_delta_| before dropping a frame.
+  const absl::optional<base::TimeDelta> max_delta_deviation_;
+
+  // The timestamp of the last delivered video frame.
+  base::TimeDelta timestamp_last_delivered_frame_ = base::TimeDelta::Max();
+
+  // Stores the accumulated difference between |target_delta_| and the actual
+  // timestamp delta between frames that are delivered. Clamped to
+  // [-max_delta_deviation, target_delta_ / 2]. This is used to allow some
+  // frames to be closer than |target_delta_| in order to maintain
+  // |target_delta_| on average. Without it we may end up with an average fps
+  // that is half of max fps.
+  base::TimeDelta accumulated_drift_;
 
   ComputedSettings track_settings_;
   ComputedSettings source_format_settings_;
@@ -259,21 +290,20 @@ VideoTrackAdapter::VideoFrameResolutionAdapter::VideoFrameResolutionAdapter(
     base::WeakPtr<MediaStreamVideoSource> media_stream_video_source)
     : renderer_task_runner_(reader_task_runner),
       media_stream_video_source_(media_stream_video_source),
-      settings_(settings) {
+      settings_(ReturnSettingsMaybeOverrideMaxFps(settings)),
+      target_delta_(settings_.max_frame_rate()
+                        ? absl::make_optional(base::Seconds(
+                              1.0 / settings_.max_frame_rate().value()))
+                        : absl::nullopt),
+      max_delta_deviation_(target_delta_
+                               ? absl::make_optional(kMaxDeltaDeviationFactor *
+                                                     target_delta_.value())
+                               : absl::nullopt) {
   DVLOG(1) << __func__ << " max_framerate "
            << settings.max_frame_rate().value_or(-1);
   DCHECK(renderer_task_runner_.get());
   DCHECK_CALLED_ON_VALID_SEQUENCE(video_sequence_checker_);
   CHECK_NE(0, settings_.max_aspect_ratio());
-
-  absl::optional<double> max_fps_override =
-      Platform::Current()->GetWebRtcMaxCaptureFrameRate();
-  if (max_fps_override) {
-    DVLOG(1) << "Overriding max frame rate.  Was="
-             << settings_.max_frame_rate().value_or(-1)
-             << ", Now=" << *max_fps_override;
-    settings_.set_max_frame_rate(*max_fps_override);
-  }
 }
 
 VideoTrackAdapter::VideoFrameResolutionAdapter::~VideoFrameResolutionAdapter() {
@@ -488,67 +518,40 @@ bool VideoTrackAdapter::VideoFrameResolutionAdapter::MaybeDropFrame(
 
   // Never drop frames if the max frame rate has not been specified.
   if (!settings_.max_frame_rate().has_value()) {
-    last_time_stamp_ = frame.timestamp();
+    timestamp_last_delivered_frame_ = frame.timestamp();
     return false;
   }
 
-  const base::TimeDelta delta = (frame.timestamp() - last_time_stamp_);
+  const base::TimeDelta delta =
+      (frame.timestamp() - timestamp_last_delivered_frame_);
 
   // Keep the frame if the time since the last frame is completely off.
   if (delta.is_negative() || delta > kMaxTimeBetweenFrames) {
-    // Reset |last_time_stamp_| and fps calculation.
-    last_time_stamp_ = frame.timestamp();
-    measured_input_frame_rate_ = *settings_.max_frame_rate();
-    fractional_frames_due_ = 0.0;
+    // Reset |timestamp_last_delivered_frame_| and |accumulated_drift|.
+    timestamp_last_delivered_frame_ = frame.timestamp();
+    accumulated_drift_ = base::Milliseconds(0.0);
     return false;
   }
 
-  if (delta < kMinTimeBetweenFrames) {
-    // We have seen video frames being delivered from camera devices back to
-    // back. The simple EMA filter for frame rate calculation is too short to
-    // handle that. https://crbug/394315
-    // TODO(perkj): Can we come up with a way to fix the times stamps and the
-    // timing when frames are delivered so all frames can be used?
-    // The time stamps are generated by Chrome and not the actual device.
-    // Most likely the back to back problem is caused by software and not the
-    // actual camera.
-    *reason = media::VideoCaptureFrameDropReason::
-        kResolutionAdapterTimestampTooCloseToPrevious;
-    return true;
-  }
-
-  last_time_stamp_ = frame.timestamp();
-
-  // Calculate the frame rate from the source using an exponential moving
-  // average (EMA) filter. Use a simple filter with 0.1 weight for the current
-  // sample.
-  double rate_for_current_frame = 1000.0 / delta.InMillisecondsF();
-  measured_input_frame_rate_ =
-      0.1 * rate_for_current_frame + 0.9 * measured_input_frame_rate_;
-
-  // Keep the frame if the input frame rate is lower than the requested frame
-  // rate or if it exceeds the target frame rate by no more than a small amount.
-  if (*settings_.max_frame_rate() + 0.5f > measured_input_frame_rate_) {
-    return false;  // Keep this frame.
-  }
-
-  // At this point, the input frame rate is known to be greater than the maximum
-  // requested frame rate by a potentially large amount. Accumulate the fraction
-  // of a frame that we should keep given the input rate. Drop the frame if a
-  // full frame has not been accumulated yet.
-  fractional_frames_due_ +=
-      *settings_.max_frame_rate() / measured_input_frame_rate_;
-  if (fractional_frames_due_ < 1.0) {
+  DCHECK(target_delta_ && max_delta_deviation_);
+  if (delta < target_delta_.value() - max_delta_deviation_.value() -
+                  accumulated_drift_) {
+    // Drop the frame because the input frame rate is too high.
     *reason = media::VideoCaptureFrameDropReason::
         kResolutionAdapterFrameRateIsHigherThanRequested;
     return true;
   }
 
-  // A full frame is due to be delivered. Keep the frame and remove it
-  // from the accumulator of due frames. The number of due frames stays in the
-  // [0.0, 1.0) range.
-  fractional_frames_due_ -= 1.0;
-
+  // Keep the frame and store the accumulated drift.
+  timestamp_last_delivered_frame_ = frame.timestamp();
+  accumulated_drift_ += delta - target_delta_.value();
+  DCHECK_GE(accumulated_drift_, -max_delta_deviation_.value());
+  // Limit the maximum accumulated drift to half of the target delta. If we
+  // don't do this, it may happen that we output a series of frames too quickly
+  // after a period of no frames. There is no need to actively limit the minimum
+  // accumulated drift because that happens automatically when we drop frames
+  // that are too close in time.
+  accumulated_drift_ = std::min(accumulated_drift_, target_delta_.value() / 2);
   return false;
 }
 

third_party/blink/renderer/modules/mediastream/video_track_adapter.h

@@ -43,10 +43,6 @@ class MODULES_EXPORT VideoTrackAdapter
  public:
   using OnMutedCallback = base::RepeatingCallback<void(bool mute_state)>;
 
-  // Min delta time between two frames allowed without being dropped if a max
-  // frame rate is specified. Exposed globally for testability.
-  static constexpr int kMinTimeBetweenFramesMs = 5;
-
   VideoTrackAdapter(
       scoped_refptr<base::SequencedTaskRunner> video_task_runner,
       base::WeakPtr<MediaStreamVideoSource> media_stream_video_source);

third_party/blink/renderer/modules/mediastream/video_track_adapter_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <limits>
 
+#include "base/functional/callback.h"
 #include "base/functional/callback_helpers.h"
 #include "base/numerics/ranges.h"
 #include "base/run_loop.h"
@@ -333,13 +334,13 @@ class VideoTrackAdapterFixtureTest : public ::testing::Test {
   }
 
   // Configures a track and an adapter with the given target frame rate,
-  // generates `num_frames` frames with 10x10 resolution at the given
-  // `actual_input_frame_rate` and returns the number of delivered and dropped
-  // frames.
+  // generates `num_frames` frames with 10x10 resolution. The timestamps are
+  // generated by calling the lambda function `index_to_timestamp` for each
+  // frame. Returns the number of delivered and dropped frames.
   std::tuple<int, int> GenerateAndCountFrames(
       int num_frames,
       absl::optional<double> target_frame_rate,
-      double actual_input_frame_rate) {
+      base::RepeatingCallback<base::TimeDelta(int)> index_to_timestamp) {
     const gfx::Size resolution(10, 10);
     // Any capture format will work. Frames will generated at the
     // `actual_input_frame_rate`.
@@ -380,14 +381,30 @@ class VideoTrackAdapterFixtureTest : public ::testing::Test {
           /*natural_size*/ resolution,
           /*storage_type=*/media::VideoFrame::STORAGE_OWNED_MEMORY,
           media::PIXEL_FORMAT_I420,
-          /*timestamp=*/i * base::Seconds(1.0 / actual_input_frame_rate));
+          /*timestamp=*/index_to_timestamp.Run(i));
       DeliverAndValidateFrame(std::move(frame), base::TimeTicks());
     }
 
     did_process_all_frames.Wait();
     return {num_delivered, num_dropped};
   }
 
+  // Configures a track and an adapter with the given target frame rate,
+  // generates `num_frames` frames with 10x10 resolution at the given
+  // `actual_input_frame_rate` and returns the number of delivered and dropped
+  // frames.
+  std::tuple<int, int> GenerateAndCountFrames(
+      int num_frames,
+      absl::optional<double> target_frame_rate,
+      double actual_input_frame_rate) {
+    auto index_to_timestamp =
+        base::BindLambdaForTesting([actual_input_frame_rate](int i) {
+          return i * base::Seconds(1.0 / actual_input_frame_rate);
+        });
+    return GenerateAndCountFrames(num_frames, target_frame_rate,
+                                  index_to_timestamp);
+  }
+
   void TestDeliversFrameWithVisibleRectWithEvenOriginAndSize(
       scoped_refptr<media::VideoFrame> frame,
       media::VideoPixelFormat pixel_format,
@@ -618,15 +635,56 @@ TEST_F(VideoTrackAdapterFixtureTest, FrameRateReduction) {
       kTargetFrameRate / kInputFrameRate, /*tolerance=*/0.05));
 }
 
-TEST_F(VideoTrackAdapterFixtureTest,
-       DoNotDropFramesWithSlightlyHighInputFrameRate) {
+TEST_F(VideoTrackAdapterFixtureTest, ArbitraryFrameRateReduction) {
+  const double kInputFrameRate = 22.0;
+  const double kTargetFrameRate = 10.0;
+  const int kNumFrames = 1000;
+
+  auto [num_delivered, num_dropped] =
+      GenerateAndCountFrames(kNumFrames, kTargetFrameRate, kInputFrameRate);
+  EXPECT_EQ(num_delivered + num_dropped, kNumFrames);
+  EXPECT_NEAR(static_cast<double>(num_delivered) / kNumFrames * kInputFrameRate,
+              kTargetFrameRate, /*abs_error=*/0.5);
+}
+
+TEST_F(VideoTrackAdapterFixtureTest, StreamWithJitterFrameRateReduction) {
+  const double kInputFrameRate = 10.0;
+  const double kTargetFrameRate = 5.0;
+  const int kNumFrames = 1000;
+
+  // Creates a 10 fps timestamp series: 0, 10 ms, 200 ms, 210 ms, ...
+  auto index_to_timestamp =
+      base::BindLambdaForTesting([kTargetFrameRate](int i) {
+        return (i / 2) * base::Seconds(1.0 / kTargetFrameRate) +
+               (i % 2) * base::Seconds(0.05 / kTargetFrameRate);
+      });
+  auto [num_delivered, num_dropped] =
+      GenerateAndCountFrames(kNumFrames, kTargetFrameRate, index_to_timestamp);
+
+  EXPECT_EQ(num_delivered + num_dropped, kNumFrames);
+  EXPECT_NEAR(static_cast<double>(num_delivered) / kNumFrames * kInputFrameRate,
+              kTargetFrameRate, /*abs_error=*/0.5);
+}
+
+TEST_F(VideoTrackAdapterFixtureTest, DoNotDropFramesIfFrameRatesMatch) {
   const int kNumFrames = 1000;
   auto [num_delivered, num_dropped] = GenerateAndCountFrames(
-      kNumFrames, /*target_frame_rate=*/10.0, /*actual_input_frame_rate=*/10.4);
+      kNumFrames, /*target_frame_rate=*/5.0, /*actual_input_frame_rate=*/5.0);
   EXPECT_EQ(num_delivered, kNumFrames);
   EXPECT_EQ(num_dropped, 0);
 }
 
+TEST_F(VideoTrackAdapterFixtureTest, FrameRateReductionSlightMismatch) {
+  const double kInputFrameRate = 10.4;
+  const double kTargetFrameRate = 10.0;
+  const int kNumFrames = 1000;
+  auto [num_delivered, num_dropped] =
+      GenerateAndCountFrames(kNumFrames, kInputFrameRate, kTargetFrameRate);
+  EXPECT_EQ(num_delivered + num_dropped, kNumFrames);
+  EXPECT_NEAR(static_cast<double>(num_delivered) / kNumFrames * kInputFrameRate,
+              kTargetFrameRate, /*abs_error=*/0.5);
+}
+
 TEST_F(VideoTrackAdapterFixtureTest, DoNotDropFramesIfNoTargetFrameRate) {
   const int kNumFrames = 100;
   auto [num_delivered, num_dropped] =