Reland "Add a FrameSequenceTracker for Video."

This feature allows tracking frame throughput during video playbacks.
This frame sequence tracker is a special case in that its tracking
process occurs in video_frame_submitter, unlike most other frame
sequence trackers that operate mainly in layer_tree_host_impl.

This reland CL includes a fix for flaky test failures caused by the
original CL (Change-Id: I9a41861e59bbe16d652b49de6b802ccc89833906).
See https://crbug.com/1001364 .

Bug: chromium:986300
Change-Id: I47e2ef9f21e417af8198f0669eaaf18c8e1daa33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1818467
Commit-Queue: Mingjing Zhang <mjzhang@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702144}

cc/metrics/frame_sequence_tracker.cc

@@ -24,6 +24,7 @@ constexpr const char* FrameSequenceTracker::kFrameSequenceTrackerTypeNames[] = {
     [FrameSequenceTrackerType::kRAF] = "RAF",
     [FrameSequenceTrackerType::kTouchScroll] = "TouchScroll",
     [FrameSequenceTrackerType::kUniversal] = "Universal",
+    [FrameSequenceTrackerType::kVideo] = "Video",
     [FrameSequenceTrackerType::kWheelScroll] = "WheelScroll",
     [FrameSequenceTrackerType::kMaxType] = "",
 };

cc/metrics/frame_sequence_tracker.h

@@ -32,16 +32,15 @@ namespace cc {
 class FrameSequenceTracker;
 class CompositorFrameReportingController;
 
-// These values are persisted to logs. Entries should not be renumbered and
-// numeric values should never be reused.
 enum FrameSequenceTrackerType {
   kCompositorAnimation = 0,
   kMainThreadAnimation = 1,
   kPinchZoom = 2,
   kRAF = 3,
   kTouchScroll = 4,
   kUniversal = 5,
-  kWheelScroll = 6,
+  kVideo = 6,
+  kWheelScroll = 7,
   kMaxType
 };
 

third_party/blink/renderer/platform/graphics/video_frame_submitter.cc

@@ -31,7 +31,8 @@ VideoFrameSubmitter::VideoFrameSubmitter(
     std::unique_ptr<VideoFrameResourceProvider> resource_provider)
     : context_provider_callback_(context_provider_callback),
       resource_provider_(std::move(resource_provider)),
-      rotation_(media::VIDEO_ROTATION_0) {
+      rotation_(media::VIDEO_ROTATION_0),
+      frame_trackers_(false, nullptr) {
   DETACH_FROM_THREAD(thread_checker_);
 }
 
@@ -59,6 +60,8 @@ void VideoFrameSubmitter::StartRendering() {
 
   if (compositor_frame_sink_)
     compositor_frame_sink_->SetNeedsBeginFrame(is_rendering_ && ShouldSubmit());
+
+  frame_trackers_.StartSequence(cc::FrameSequenceTrackerType::kVideo);
 }
 
 void VideoFrameSubmitter::StopRendering() {
@@ -67,6 +70,9 @@ void VideoFrameSubmitter::StopRendering() {
   DCHECK(video_frame_provider_);
 
   is_rendering_ = false;
+
+  frame_trackers_.StopSequence(cc::FrameSequenceTrackerType::kVideo);
+
   UpdateSubmissionState();
 }
 
@@ -170,29 +176,43 @@ void VideoFrameSubmitter::OnBeginFrame(
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   TRACE_EVENT0("media", "VideoFrameSubmitter::OnBeginFrame");
 
+  last_begin_frame_args_ = args;
+
   for (const auto& pair : timing_details) {
     if (viz::FrameTokenGT(pair.key, *next_frame_token_))
       continue;
 
     if (base::Contains(frame_token_to_timestamp_map_, pair.key) &&
         !(pair.value->presentation_feedback->flags &
           gfx::PresentationFeedback::kFailure)) {
+      if (!ignorable_submitted_frames_.contains(pair.key)) {
+        frame_trackers_.NotifyFramePresented(
+            pair.key, gfx::PresentationFeedback(
+                          pair.value->presentation_feedback->timestamp,
+                          pair.value->presentation_feedback->interval,
+                          pair.value->presentation_feedback->flags));
+      }
       UMA_HISTOGRAM_TIMES("Media.VideoFrameSubmitter",
                           pair.value->presentation_feedback->timestamp -
                               frame_token_to_timestamp_map_[pair.key]);
       frame_token_to_timestamp_map_.erase(pair.key);
     }
 
+    ignorable_submitted_frames_.erase(pair.key);
+
     TRACE_EVENT_ASYNC_END_WITH_TIMESTAMP0(
         "media", "VideoFrameSubmitter", pair.key,
         pair.value->presentation_feedback->timestamp);
   }
 
+  frame_trackers_.NotifyBeginImplFrame(args);
+
   // Don't call UpdateCurrentFrame() for MISSED BeginFrames. Also don't call it
   // after StopRendering() has been called (forbidden by API contract).
   viz::BeginFrameAck current_begin_frame_ack(args, false);
   if (args.type == viz::BeginFrameArgs::MISSED || !is_rendering_) {
     compositor_frame_sink_->DidNotProduceFrame(current_begin_frame_ack);
+    frame_trackers_.NotifyImplFrameCausedNoDamage(current_begin_frame_ack);
     return;
   }
 
@@ -204,6 +224,7 @@ void VideoFrameSubmitter::OnBeginFrame(
                                     args.frame_time + args.interval,
                                     args.frame_time + 2 * args.interval)) {
     compositor_frame_sink_->DidNotProduceFrame(current_begin_frame_ack);
+    frame_trackers_.NotifyImplFrameCausedNoDamage(current_begin_frame_ack);
     return;
   }
 
@@ -212,6 +233,7 @@ void VideoFrameSubmitter::OnBeginFrame(
   auto video_frame = video_frame_provider_->GetCurrentFrame();
   if (!SubmitFrame(current_begin_frame_ack, std::move(video_frame))) {
     compositor_frame_sink_->DidNotProduceFrame(current_begin_frame_ack);
+    frame_trackers_.NotifyImplFrameCausedNoDamage(current_begin_frame_ack);
     return;
   }
 
@@ -422,10 +444,13 @@ bool VideoFrameSubmitter::SubmitFrame(
 
   // We can pass nullptr for the HitTestData as the CompositorFram will not
   // contain any SurfaceDrawQuads.
+  auto frame_token = compositor_frame.metadata.frame_token;
   compositor_frame_sink_->SubmitCompositorFrame(
       child_local_surface_id_allocator_.GetCurrentLocalSurfaceIdAllocation()
           .local_surface_id(),
       std::move(compositor_frame), nullptr, 0);
+  frame_trackers_.NotifySubmitFrame(frame_token, false, begin_frame_ack,
+                                    last_begin_frame_args_);
   resource_provider_->ReleaseFrameResources();
 
   waiting_for_compositor_ack_ = true;
@@ -444,12 +469,16 @@ void VideoFrameSubmitter::SubmitEmptyFrame() {
     return;
 
   last_frame_id_.reset();
+  auto begin_frame_ack = viz::BeginFrameAck::CreateManualAckWithDamage();
+  auto compositor_frame = CreateCompositorFrame(begin_frame_ack, nullptr);
+
+  auto frame_token = compositor_frame.metadata.frame_token;
   compositor_frame_sink_->SubmitCompositorFrame(
       child_local_surface_id_allocator_.GetCurrentLocalSurfaceIdAllocation()
           .local_surface_id(),
-      CreateCompositorFrame(viz::BeginFrameAck::CreateManualAckWithDamage(),
-                            nullptr),
-      nullptr, 0);
+      std::move(compositor_frame), nullptr, 0);
+  frame_trackers_.NotifySubmitFrame(frame_token, false, begin_frame_ack,
+                                    last_begin_frame_args_);
 
   // We don't set |waiting_for_compositor_ack_| here since we want to allow a
   // subsequent real frame to replace it at any time if needed.
@@ -503,6 +532,10 @@ viz::CompositorFrame VideoFrameSubmitter::CreateCompositorFrame(
                                  *next_frame_token_, "Pre-submit buffering");
 
     frame_token_to_timestamp_map_[*next_frame_token_] = value;
+
+    if (begin_frame_ack.source_id == viz::BeginFrameArgs::kManualSourceId)
+      ignorable_submitted_frames_.insert(*next_frame_token_);
+
     UMA_HISTOGRAM_TIMES("Media.VideoFrameSubmitter.PreSubmitBuffering",
                         base::TimeTicks::Now() - value);
   } else {

third_party/blink/renderer/platform/graphics/video_frame_submitter.h

@@ -13,6 +13,7 @@
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
+#include "cc/metrics/frame_sequence_tracker.h"
 #include "components/viz/client/shared_bitmap_reporter.h"
 #include "components/viz/common/gpu/context_provider.h"
 #include "components/viz/common/resources/shared_bitmap.h"
@@ -178,6 +179,17 @@ class PLATFORM_EXPORT VideoFrameSubmitter
 
   base::Optional<int> last_frame_id_;
 
+  cc::FrameSequenceTrackerCollection frame_trackers_;
+
+  // The BeginFrameArgs passed to the most recent call of OnBeginFrame().
+  // Required for FrameSequenceTrackerCollection::NotifySubmitFrame
+  viz::BeginFrameArgs last_begin_frame_args_;
+
+  // The token of the frames that are submitted outside OnBeginFrame(). These
+  // frames should be ignored by the video tracker even if they are reported as
+  // presented.
+  base::flat_set<uint32_t> ignorable_submitted_frames_;
+
   THREAD_CHECKER(thread_checker_);
 
   base::WeakPtrFactory<VideoFrameSubmitter> weak_ptr_factory_{this};

tools/metrics/histograms/histograms.xml

@@ -177027,6 +177027,7 @@ regressions. -->
   <suffix name="RAF" label="rAF callback driven animation"/>
   <suffix name="TouchScroll" label="Touchscreen driven interaction"/>
   <suffix name="Universal" label="All frame production"/>
+  <suffix name="Video" label="Video playback"/>
   <suffix name="WheelScroll" label="Mousewheel driven interaction"/>
   <affected-histogram name="CompositorLatency"/>
   <affected-histogram name="CompositorLatency.MissedFrame"/>