[video-raf] Add high fps auto-rescheduling

A lot of frames are missed when using video.rAF to paint high frame rate
videos (60fps). This is partially caused because, even if there is a new
frame ready to be painted, the calls to OnRequestAnimationFrame() after
jumping back on the main thread arrives too late, and we have to wait
until the next rendering steps.

This CL adds the logic to automatically re-schedule video.rAF execution
in the next rendering steps if we have a high enough frame rate, we have
more callbacks to run, and we consistently get new frames.

This CL might be revisited after collecting data from the canary builds.

Bug: 1012063
Change-Id: I90a63e32d774886c9e276b282ee105d1f359f968
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2079302
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745747}

media/blink/video_frame_compositor.cc

@@ -157,6 +157,8 @@ void VideoFrameCompositor::SetCurrentFrame(
     scoped_refptr<VideoFrame> frame,
     base::TimeTicks expected_presentation_time) {
   DCHECK(task_runner_->BelongsToCurrentThread());
+  TRACE_EVENT1("media", "VideoFrameCompositor::SetCurrentFrame", "frame",
+               frame->AsHumanReadableString());
   base::AutoLock lock(current_frame_lock_);
   current_frame_ = std::move(frame);
   last_presentation_time_ = tick_clock_->NowTicks();
@@ -172,6 +174,8 @@ void VideoFrameCompositor::PutCurrentFrame() {
 bool VideoFrameCompositor::UpdateCurrentFrame(base::TimeTicks deadline_min,
                                               base::TimeTicks deadline_max) {
   DCHECK(task_runner_->BelongsToCurrentThread());
+  TRACE_EVENT2("media", "VideoFrameCompositor::UpdateCurrentFrame",
+               "deadline_min", deadline_min, "deadline_max", deadline_max);
   return CallRender(deadline_min, deadline_max, false);
 }
 
@@ -229,6 +233,7 @@ void VideoFrameCompositor::PaintSingleFrame(scoped_refptr<VideoFrame> frame,
 }
 
 void VideoFrameCompositor::UpdateCurrentFrameIfStale() {
+  TRACE_EVENT0("media", "VideoFrameCompositor::UpdateCurrentFrameIfStale");
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   // If we're not rendering, then the frame can't be stale.
@@ -252,9 +257,12 @@ void VideoFrameCompositor::UpdateCurrentFrameIfStale() {
   if (interval < base::TimeDelta::FromMilliseconds(4))
     return;
 
-  // Update the interval based on the time between calls and call background
-  // render which will give this information to the client.
-  last_interval_ = interval;
+  {
+    base::AutoLock lock(callback_lock_);
+    // Update the interval based on the time between calls and call background
+    // render which will give this information to the client.
+    last_interval_ = interval;
+  }
   BackgroundRender();
 }
 
@@ -294,6 +302,15 @@ VideoFrameCompositor::GetLastPresentedFrameMetadata() {
 
   frame_metadata->metadata.MergeMetadataFrom(last_frame->metadata());
 
+  {
+    base::AutoLock lock(callback_lock_);
+    if (callback_) {
+      frame_metadata->average_frame_duration =
+          callback_->GetPreferredRenderInterval();
+    }
+    frame_metadata->rendering_interval = last_interval_;
+  }
+
   return frame_metadata;
 }
 
@@ -347,11 +364,19 @@ void VideoFrameCompositor::SetForceSubmit(bool force_submit) {
   submitter_->SetForceSubmit(force_submit);
 }
 
+base::TimeDelta VideoFrameCompositor::GetLastIntervalWithoutLock()
+    NO_THREAD_SAFETY_ANALYSIS {
+  DCHECK(task_runner_->BelongsToCurrentThread());
+  // |last_interval_| is only updated on the compositor thread, so it's safe to
+  // return it without acquiring |callback_lock_|
+  return last_interval_;
+}
+
 void VideoFrameCompositor::BackgroundRender() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   const base::TimeTicks now = tick_clock_->NowTicks();
   last_background_render_ = now;
-  bool new_frame = CallRender(now, now + last_interval_, true);
+  bool new_frame = CallRender(now, now + GetLastIntervalWithoutLock(), true);
   if (new_frame && IsClientSinkAvailable())
     client_->DidReceiveFrame();
 }

media/blink/video_frame_compositor.h

@@ -201,6 +201,10 @@ class MEDIA_BLINK_EXPORT VideoFrameCompositor : public VideoRendererSink,
                   base::TimeTicks deadline_max,
                   bool background_rendering);
 
+  // Returns |last_interval_| without acquiring a lock.
+  // Can only be called from the compositor thread.
+  base::TimeDelta GetLastIntervalWithoutLock();
+
   // This will run tasks on the compositor thread. If
   // kEnableSurfaceLayerForVideo is enabled, it will instead run tasks on the
   // media thread.
@@ -223,21 +227,20 @@ class MEDIA_BLINK_EXPORT VideoFrameCompositor : public VideoRendererSink,
   bool is_background_rendering_ = false;
   bool new_background_frame_ = false;
 
-  // Assume 60Hz before the first UpdateCurrentFrame() call.
-  base::TimeDelta last_interval_ = base::TimeDelta::FromSecondsD(1.0 / 60);
-
   base::TimeTicks last_background_render_;
   OnNewProcessedFrameCB new_processed_frame_cb_;
   cc::UpdateSubmissionStateCB update_submission_state_callback_;
 
   // Callback used to satisfy video.rAF requests.
   // Set on the main thread, fired on the compositor thread.
+  // TODO(https://crbug.com/1057304): consolidate locks.
   base::Lock new_presented_frame_cb_lock_;
   OnNewFramePresentedCB new_presented_frame_cb_
       GUARDED_BY(new_presented_frame_cb_lock_);
 
   // Set on the compositor thread, but also read on the media thread. Lock is
   // not used when reading |current_frame_| on the compositor thread.
+  // TODO(https://crbug.com/1057304): consolidate locks.
   base::Lock current_frame_lock_;
   scoped_refptr<VideoFrame> current_frame_;
 
@@ -249,10 +252,16 @@ class MEDIA_BLINK_EXPORT VideoFrameCompositor : public VideoRendererSink,
   uint32_t presentation_counter_ GUARDED_BY(current_frame_lock_) = 0u;
 
   // These values are updated and read from the media and compositor threads.
+  // TODO(https://crbug.com/1057304): consolidate locks.
   base::Lock callback_lock_;
   VideoRendererSink::RenderCallback* callback_ GUARDED_BY(callback_lock_) =
       nullptr;
 
+  // Assume 60Hz before the first UpdateCurrentFrame() call.
+  // Updated/read by the compositor thread, but also read on the media thread.
+  base::TimeDelta last_interval_ GUARDED_BY(callback_lock_) =
+      base::TimeDelta::FromSecondsD(1.0 / 60);
+
   // AutoOpenCloseEvent for begin/end events.
   std::unique_ptr<base::trace_event::AutoOpenCloseEvent<kTracingCategory>>
       auto_open_close_;

third_party/blink/public/platform/web_media_player.h

@@ -137,6 +137,8 @@ class WebMediaPlayer {
     int height;
     base::TimeDelta presentation_timestamp;
     media::VideoFrameMetadata metadata;
+    base::TimeDelta rendering_interval;
+    base::TimeDelta average_frame_duration;
   };
 
   // Describes when we use SurfaceLayer for video instead of VideoLayer.

third_party/blink/renderer/modules/video_raf/video_frame_request_callback_collection.h

@@ -83,8 +83,7 @@ class MODULES_EXPORT VideoFrameRequestCallbackCollection final
   // Invokes all callbacks with the provided information.
   void ExecuteFrameCallbacks(double high_res_now_ms, const VideoFrameMetadata*);
 
-  bool HasFrameCallback() const { return frame_callbacks_.size(); }
-  bool IsEmpty() const { return !HasFrameCallback(); }
+  bool IsEmpty() const { return !frame_callbacks_.size(); }
 
   virtual void Trace(Visitor*);
   const char* NameInHeapSnapshot() const override {

third_party/blink/renderer/modules/video_raf/video_frame_request_callback_collection_test.cc

@@ -44,14 +44,12 @@ class VideoFrameRequestCallbackCollectionTest : public PageTestBase {
 };
 
 TEST_F(VideoFrameRequestCallbackCollectionTest, AddSingleCallback) {
-  EXPECT_FALSE(collection()->HasFrameCallback());
   EXPECT_TRUE(collection()->IsEmpty());
 
   auto callback = CreateCallback();
   CallbackId id = collection()->RegisterFrameCallback(callback.Get());
 
   EXPECT_EQ(id, callback->Id());
-  EXPECT_TRUE(collection()->HasFrameCallback());
   EXPECT_FALSE(collection()->IsEmpty());
 }
 
@@ -63,7 +61,7 @@ TEST_F(VideoFrameRequestCallbackCollectionTest, InvokeSingleCallback) {
   EXPECT_CALL(*callback, Invoke(kDefaultTimestamp, metadata));
   collection()->ExecuteFrameCallbacks(kDefaultTimestamp, metadata);
 
-  EXPECT_FALSE(collection()->HasFrameCallback());
+  EXPECT_TRUE(collection()->IsEmpty());
 }
 
 TEST_F(VideoFrameRequestCallbackCollectionTest, CancelSingleCallback) {
@@ -75,12 +73,12 @@ TEST_F(VideoFrameRequestCallbackCollectionTest, CancelSingleCallback) {
 
   // Cancelling an non existent ID should do nothing.
   collection()->CancelFrameCallback(id + 100);
-  EXPECT_TRUE(collection()->HasFrameCallback());
+  EXPECT_FALSE(collection()->IsEmpty());
   EXPECT_FALSE(callback->IsCancelled());
 
   // Cancel the callback this time.
   collection()->CancelFrameCallback(id);
-  EXPECT_FALSE(collection()->HasFrameCallback());
+  EXPECT_TRUE(collection()->IsEmpty());
 
   collection()->ExecuteFrameCallbacks(kDefaultTimestamp,
                                       VideoFrameMetadata::Create());
@@ -121,13 +119,13 @@ TEST_F(VideoFrameRequestCallbackCollectionTest, CreateCallbackDuringExecution) {
                                       VideoFrameMetadata::Create());
 
   EXPECT_NE(created_id, 0);
-  EXPECT_TRUE(collection()->HasFrameCallback());
+  EXPECT_FALSE(collection()->IsEmpty());
 
   // The created callback should be executed the second time around.
   EXPECT_CALL(*created_callback, Invoke(_, _)).Times(1);
   collection()->ExecuteFrameCallbacks(kDefaultTimestamp,
                                       VideoFrameMetadata::Create());
-  EXPECT_FALSE(collection()->HasFrameCallback());
+  EXPECT_TRUE(collection()->IsEmpty());
 }
 
 TEST_F(VideoFrameRequestCallbackCollectionTest,
@@ -158,7 +156,7 @@ TEST_F(VideoFrameRequestCallbackCollectionTest,
                                       VideoFrameMetadata::Create());
 
   // Everything should have been cleared
-  EXPECT_FALSE(collection()->HasFrameCallback());
+  EXPECT_TRUE(collection()->IsEmpty());
 }
 
 }  // namespace blink

third_party/blink/renderer/modules/video_raf/video_request_animation_frame_impl.cc

@@ -7,6 +7,7 @@
 #include <memory>
 #include <utility>
 
+#include "base/trace_event/trace_event.h"
 #include "media/base/media_switches.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_video_frame_metadata.h"
 #include "third_party/blink/renderer/core/dom/document.h"
@@ -21,6 +22,24 @@
 
 namespace blink {
 
+namespace {
+// Returns whether or not a video's frame rate is close to the browser's frame
+// rate, as measured by their rendering intervals. For example, on a 60hz
+// screen, this should return false for a 25fps video and true for a 60fps
+// video. On a 144hz screen, both videos would return false.
+static bool IsFrameRateRelativelyHigh(base::TimeDelta rendering_interval,
+                                      base::TimeDelta average_frame_duration) {
+  if (average_frame_duration.is_zero())
+    return false;
+
+  constexpr double kThreshold = 0.05;
+  return kThreshold >
+         std::abs(1.0 - (rendering_interval.InMillisecondsF() /
+                         average_frame_duration.InMillisecondsF()));
+}
+
+}  // namespace
+
 VideoRequestAnimationFrameImpl::VideoRequestAnimationFrameImpl(
     HTMLVideoElement& element)
     : VideoRequestAnimationFrame(element),
@@ -60,58 +79,60 @@ void VideoRequestAnimationFrameImpl::cancelAnimationFrame(
 
 void VideoRequestAnimationFrameImpl::OnWebMediaPlayerCreated() {
   DCHECK(RuntimeEnabledFeatures::VideoRequestAnimationFrameEnabled());
-  if (callback_collection_->HasFrameCallback())
+  if (!callback_collection_->IsEmpty())
     GetSupplementable()->GetWebMediaPlayer()->RequestAnimationFrame();
 }
 
+void VideoRequestAnimationFrameImpl::ScheduleCallbackExecution() {
+  TRACE_EVENT1("blink",
+               "VideoRequestAnimationFrameImpl::ScheduleCallbackExecution",
+               "did_schedule", !pending_execution_);
+
+  if (pending_execution_)
+    return;
+
+  pending_execution_ = true;
+  if (base::FeatureList::IsEnabled(media::kUseMicrotaskForVideoRAF)) {
+    auto& time_converter =
+        GetSupplementable()->GetDocument().Loader()->GetTiming();
+    Microtask::EnqueueMicrotask(WTF::Bind(
+        &VideoRequestAnimationFrameImpl::OnRenderingSteps,
+        WrapWeakPersistent(this),
+        // TODO(crbug.com/1012063): Now is probably not the right value.
+        GetClampedTimeInMillis(
+            time_converter.MonotonicTimeToZeroBasedDocumentTime(
+                base::TimeTicks::Now()))));
+  } else {
+    GetSupplementable()
+        ->GetDocument()
+        .GetScriptedAnimationController()
+        .ScheduleVideoRafExecution(
+            WTF::Bind(&VideoRequestAnimationFrameImpl::OnRenderingSteps,
+                      WrapWeakPersistent(this)));
+  }
+}
+
 void VideoRequestAnimationFrameImpl::OnRequestAnimationFrame() {
   DCHECK(RuntimeEnabledFeatures::VideoRequestAnimationFrameEnabled());
+  TRACE_EVENT1("blink",
+               "VideoRequestAnimationFrameImpl::OnRequestAnimationFrame",
+               "has_callbacks", !callback_collection_->IsEmpty());
 
   // Skip this work if there are no registered callbacks.
   if (callback_collection_->IsEmpty())
     return;
 
-  if (!pending_execution_) {
-    pending_execution_ = true;
-    if (base::FeatureList::IsEnabled(media::kUseMicrotaskForVideoRAF)) {
-      auto& time_converter =
-          GetSupplementable()->GetDocument().Loader()->GetTiming();
-      Microtask::EnqueueMicrotask(WTF::Bind(
-          &VideoRequestAnimationFrameImpl::ExecuteFrameCallbacks,
-          WrapWeakPersistent(this),
-          // TODO(crbug.com/1012063): Now is probably not the right value.
-          time_converter
-              .MonotonicTimeToZeroBasedDocumentTime(base::TimeTicks::Now())
-              .InMillisecondsF()));
-    } else {
-      GetSupplementable()
-          ->GetDocument()
-          .GetScriptedAnimationController()
-          .ScheduleVideoRafExecution(
-              WTF::Bind(&VideoRequestAnimationFrameImpl::ExecuteFrameCallbacks,
-                        WrapWeakPersistent(this)));
-    }
-  }
+  ScheduleCallbackExecution();
 }
 
 void VideoRequestAnimationFrameImpl::ExecuteFrameCallbacks(
-    double high_res_now_ms) {
-  DCHECK(pending_execution_);
-
-  // Callbacks could have been canceled from the time we scheduled their
-  // execution.
-  if (callback_collection_->IsEmpty()) {
-    pending_execution_ = false;
-    return;
-  }
-
-  auto* player = GetSupplementable()->GetWebMediaPlayer();
-  if (!player) {
-    pending_execution_ = false;
-    return;
-  }
+    double high_res_now_ms,
+    std::unique_ptr<WebMediaPlayer::VideoFramePresentationMetadata>
+        frame_metadata) {
+  TRACE_EVENT0("blink",
+               "VideoRequestAnimationFrameImpl::ExecuteFrameCallbacks");
 
-  auto frame_metadata = player->GetVideoFramePresentationMetadata();
+  last_presented_frames_ = frame_metadata->presented_frames;
 
   auto* metadata = VideoFrameMetadata::Create();
   auto& time_converter =
@@ -155,7 +176,49 @@ void VideoRequestAnimationFrameImpl::ExecuteFrameCallbacks(
   }
 
   callback_collection_->ExecuteFrameCallbacks(high_res_now_ms, metadata);
+}
+
+void VideoRequestAnimationFrameImpl::OnRenderingSteps(double high_res_now_ms) {
+  DCHECK(pending_execution_);
+  TRACE_EVENT1("blink", "VideoRequestAnimationFrameImpl::OnRenderingSteps",
+               "has_callbacks", !callback_collection_->IsEmpty());
+
   pending_execution_ = false;
+
+  // Callbacks could have been canceled from the time we scheduled their
+  // execution.
+  if (callback_collection_->IsEmpty())
+    return;
+
+  auto* player = GetSupplementable()->GetWebMediaPlayer();
+  if (!player)
+    return;
+
+  auto metadata = player->GetVideoFramePresentationMetadata();
+
+  const bool is_hfr = IsFrameRateRelativelyHigh(
+      metadata->rendering_interval, metadata->average_frame_duration);
+
+  // Check if we have a new frame or not.
+  if (last_presented_frames_ == metadata->presented_frames) {
+    ++consecutive_stale_frames_;
+  } else {
+    consecutive_stale_frames_ = 0;
+    ExecuteFrameCallbacks(high_res_now_ms, std::move(metadata));
+  }
+
+  // If the video's frame rate is relatively close to the screen's refresh rate
+  // (or brower's current frame rate), schedule ourselves immediately.
+  // Otherwise, jittering and thread hopping means that the call to
+  // OnRequestAnimationFrame() would barely miss the rendering steps, and we
+  // would miss a frame.
+  // Also check |consecutive_stale_frames_| to make sure we don't schedule
+  // executions when paused, or in other scenarios where potentially scheduling
+  // extra rendering steps would be wasteful.
+  if (is_hfr && !callback_collection_->IsEmpty() &&
+      consecutive_stale_frames_ < 2) {
+    ScheduleCallbackExecution();
+  }
 }
 
 // static
@@ -182,6 +245,9 @@ double VideoRequestAnimationFrameImpl::GetCoarseClampedTimeInSeconds(
 
 int VideoRequestAnimationFrameImpl::requestAnimationFrame(
     V8VideoFrameRequestCallback* callback) {
+  TRACE_EVENT0("blink",
+               "VideoRequestAnimationFrameImpl::requestAnimationFrame");
+
   if (auto* player = GetSupplementable()->GetWebMediaPlayer())
     player->RequestAnimationFrame();
 

third_party/blink/renderer/modules/video_raf/video_request_animation_frame_impl.h

@@ -41,25 +41,48 @@ class MODULES_EXPORT VideoRequestAnimationFrameImpl final
   void OnWebMediaPlayerCreated() override;
   void OnRequestAnimationFrame() override;
 
-  void ExecuteFrameCallbacks(double high_res_now_ms);
+  // Called by ScriptedAnimationController as part of the rendering steps,
+  // right before the execution of window.rAF callbacks.
+  void OnRenderingSteps(double high_res_now_ms);
 
  private:
   friend class VideoRequestAnimationFrameImplTest;
 
+  // Utility functions to limit the clock resolution of fields, for security
+  // reasons.
+  static double GetClampedTimeInMillis(base::TimeDelta time);
+  static double GetCoarseClampedTimeInSeconds(base::TimeDelta time);
+
+  void ExecuteFrameCallbacks(
+      double high_res_now_ms,
+      std::unique_ptr<WebMediaPlayer::VideoFramePresentationMetadata>);
+
   // Register a non-V8 callback for testing. Also sets |pending_execution_| to
   // true, to allow calling into ExecuteFrameCallbacks() directly.
   void RegisterCallbackForTest(
       VideoFrameRequestCallbackCollection::VideoFrameCallback*);
 
-  // Utility functions to limit the clock resolution of fields, for security
-  // reasons.
-  static double GetClampedTimeInMillis(base::TimeDelta time);
-  static double GetCoarseClampedTimeInSeconds(base::TimeDelta time);
+  // Adds |this| to the ScriptedAnimationController's queue of video.rAF
+  // callbacks that should be executed during the next rendering steps.
+  // Also causes rendering steps to be scheduled if needed.
+  void ScheduleCallbackExecution();
 
   // Used to keep track of whether or not we have already scheduled a call to
   // ExecuteFrameCallbacks() in the next rendering steps.
   bool pending_execution_ = false;
 
+  // The value of the |metadata->presented_frames| field the last time called
+  // ExecuteFrameCallbacks. Used to determine whether or not a new frame was
+  // presented since we last executed the frame callbacks.
+  // The values coming from the compositor should start at 1, we can use 0
+  // as a "null" starting value.
+  uint32_t last_presented_frames_ = 0;
+
+  // Number of times OnRenderingSteps() was called in a row, without us having a
+  // new frame. Used to abort auto-rescheduling if we aren't consistently
+  // getting new frames.
+  int consecutive_stale_frames_ = 0;
+
   Member<VideoFrameRequestCallbackCollection> callback_collection_;
 
   DISALLOW_COPY_AND_ASSIGN(VideoRequestAnimationFrameImpl);

third_party/blink/renderer/modules/video_raf/video_request_animation_frame_impl_test.cc

@@ -288,10 +288,12 @@ TEST_F(VideoRequestAnimationFrameImplTest, VerifyRequestAnimationFrame) {
   testing::Mock::VerifyAndClear(function);
 
   // Callbacks should be called during the rendering steps.
+  auto metadata = std::make_unique<VideoFramePresentationMetadata>();
+  metadata->presented_frames = 1;
+
   EXPECT_CALL(*function, Call(_)).Times(1);
   EXPECT_CALL(*media_player(), GetVideoFramePresentationMetadata())
-      .WillOnce(
-          Return(ByMove(std::make_unique<VideoFramePresentationMetadata>())));
+      .WillOnce(Return(ByMove(std::move(metadata))));
   SimulateAnimationFrame(base::TimeTicks::Now());
 
   testing::Mock::VerifyAndClear(function);
@@ -350,7 +352,7 @@ TEST_F(VideoRequestAnimationFrameImplTest, VerifyParameters) {
 
   // Run the callbacks directly, since they weren't scheduled to be run by the
   // ScriptedAnimationController.
-  video_raf().ExecuteFrameCallbacks(now_ms);
+  video_raf().OnRenderingSteps(now_ms);
 
   EXPECT_EQ(callback->last_now(), now_ms);
   EXPECT_TRUE(callback->was_invoked());