Add freezing metric to roughness reporter.

Also modifies callback to take a `Measurement` struct, since the number
of reported values was getting large.

Last, adds explicit calls to the roughness reporter from the submitter
when we stop rendering, so that it knows to stop the window.

Change-Id: Idb00046511c5cee0c351172ec6e786d35e870457
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2410963Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Eugene Zemtsov <eugene@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810946}

cc/metrics/video_playback_roughness_reporter.cc

@@ -34,7 +34,7 @@ constexpr int VideoPlaybackRoughnessReporter::kMinWindowsBeforeSubmit;
 constexpr int VideoPlaybackRoughnessReporter::kPercentileToSubmit;
 
 VideoPlaybackRoughnessReporter::VideoPlaybackRoughnessReporter(
-    PlaybackRoughnessReportingCallback reporting_cb)
+    ReportingCallback reporting_cb)
     : reporting_cb_(reporting_cb) {}
 
 VideoPlaybackRoughnessReporter::~VideoPlaybackRoughnessReporter() = default;
@@ -92,6 +92,7 @@ void VideoPlaybackRoughnessReporter::FramePresented(TokenType token,
         auto time_since_decode = timestamp - info.decode_time.value();
         UMA_HISTOGRAM_TIMES("Media.VideoFrameSubmitter", time_since_decode);
       }
+
       if (reliable_timestamp)
         info.presentation_time = timestamp;
       break;
@@ -111,20 +112,23 @@ void VideoPlaybackRoughnessReporter::SubmitPlaybackRoughness() {
   }
 
   auto it = worst_windows_.begin() + index_to_submit;
-  reporting_cb_.Run(it->size, it->intended_duration, it->roughness(),
-                    it->refresh_rate_hz, it->frame_size);
+
+  Measurement measurement;
+  measurement.frames = it->size;
+  measurement.duration = it->intended_duration;
+  measurement.roughness = it->roughness();
+  measurement.freezing = max_single_frame_error_;
+  measurement.refresh_rate_hz = it->refresh_rate_hz;
+  measurement.frame_size = it->frame_size;
+  reporting_cb_.Run(measurement);
 
   worst_windows_.clear();
   windows_seen_ = 0;
+  max_single_frame_error_ = base::TimeDelta();
 }
 
 void VideoPlaybackRoughnessReporter::ReportWindow(
     const ConsecutiveFramesWindow& win) {
-  if (win.max_single_frame_error >= win.intended_duration ||
-      win.root_mean_square_error >= win.intended_duration) {
-    // Most likely it's just an effect of the video being paused for some time.
-    return;
-  }
   worst_windows_.insert(win);
   if (worst_windows_.size() > max_worst_windows_size())
     worst_windows_.erase(std::prev(worst_windows_.end()));
@@ -193,8 +197,8 @@ void VideoPlaybackRoughnessReporter::ProcessFrameWindow() {
         win.intended_duration += frame.intended_duration.value();
       }
       total_error += error;
-      win.max_single_frame_error =
-          std::max(win.max_single_frame_error, error.magnitude());
+      max_single_frame_error_ =
+          std::max(max_single_frame_error_, error.magnitude());
       mean_square_error_ms2 +=
           total_error.InMillisecondsF() * total_error.InMillisecondsF();
     }
@@ -212,6 +216,7 @@ void VideoPlaybackRoughnessReporter::ProcessFrameWindow() {
       } else {
         worst_windows_.clear();
         windows_seen_ = 0;
+        max_single_frame_error_ = base::TimeDelta();
       }
     } else {
       ReportWindow(win);
@@ -237,6 +242,7 @@ void VideoPlaybackRoughnessReporter::Reset() {
   frames_.clear();
   worst_windows_.clear();
   windows_seen_ = 0;
+  max_single_frame_error_ = base::TimeDelta();
 }
 
 }  // namespace cc

cc/metrics/video_playback_roughness_reporter.h

@@ -17,34 +17,58 @@
 
 namespace cc {
 
-// Callback to report video playback roughness on a particularly bumpy interval.
-// |frames| - number of frames in the interval
-// |duration| - intended wallclock duration of the interval
-// |roughness| - roughness of the interval
-// |refresh_rate_hz| - display refresh rate, usually 60Hz
-// |frame_size| - size of the video frames in the interval
-using PlaybackRoughnessReportingCallback =
-    base::RepeatingCallback<void(int frames,
-                                 base::TimeDelta duration,
-                                 double roughness,
-                                 int refresh_rate_hz,
-                                 gfx::Size frame_size)>;
-
 // This class tracks moments when each frame was submitted
 // and when it was displayed. Then series of frames split into groups
 // of consecutive frames, where each group takes about one second of playback.
 // Such groups also called 'frame windows'. Each windows is assigned a roughness
 // score that measures how far playback smoothness was from the ideal playback.
+//
 // Information about several windows and their roughness score is aggregated
-// for a couple of playbackminutes and then a window with
-// 95-percentile-max-roughness is reported via the provided callback.
+// for a couple of playback minutes ("measurement interval") and then a window
+// with 95-percentile-max-roughness is reported via the provided callback.
+//
 // This sufficiently bad roughness window is deemed to represent overall
 // playback quality.
 class CC_EXPORT VideoPlaybackRoughnessReporter {
  public:
+  struct Measurement {
+    // 95%-worst window measurements ========
+    // These are taken from the |kPercentileToSubmit| worst window in a
+    // measurement interval.
+
+    // |frames| - number of video frames in the window
+    int frames = 0;
+
+    // |duration| - intended wallclock duration of the window (~1s)
+    base::TimeDelta duration;
+
+    // |roughness| - roughness of the window
+    double roughness = 0;
+
+    // Per-measurement interval measurements ========
+    // These are measured over all windows in the measurement interval, without
+    // regard to which window was chosen above.
+
+    // |frame_size| - size of the video frames in the window
+    gfx::Size frame_size;
+
+    // |freezing| maximum amount of time that any VideoFrame in measurement
+    // interval was on-screen beyond the amount of time it should have been.
+    //
+    // TODO(liberato): Should this be expressed in terms of the playback rate?
+    // As in, "twice as long as it should have been"?
+    base::TimeDelta freezing;
+
+    // |refresh_rate_hz| - display refresh rate, usually 60Hz
+    int refresh_rate_hz = 0;
+  };
+
+  // Callback to report video playback roughness on a particularly bumpy
+  // interval.
+  using ReportingCallback = base::RepeatingCallback<void(const Measurement&)>;
+
   using TokenType = uint32_t;
-  explicit VideoPlaybackRoughnessReporter(
-      PlaybackRoughnessReportingCallback reporting_cb);
+  explicit VideoPlaybackRoughnessReporter(ReportingCallback reporting_cb);
   VideoPlaybackRoughnessReporter(const VideoPlaybackRoughnessReporter&) =
       delete;
   VideoPlaybackRoughnessReporter& operator=(
@@ -102,10 +126,6 @@ class CC_EXPORT VideoPlaybackRoughnessReporter {
     int refresh_rate_hz = 60;
     gfx::Size frame_size;
 
-    // Worst case difference between a frame's intended duration and
-    // actual duration, calculated for all frames in the window.
-    base::TimeDelta max_single_frame_error;
-
     // Root-mean-square error of the differences between the intended
     // duration and the actual duration, calculated for all subwindows
     // starting at the beginning of the smoothness window
@@ -135,9 +155,13 @@ class CC_EXPORT VideoPlaybackRoughnessReporter {
 
   base::circular_deque<FrameInfo> frames_;
   base::flat_set<ConsecutiveFramesWindow> worst_windows_;
-  PlaybackRoughnessReportingCallback reporting_cb_;
+  ReportingCallback reporting_cb_;
   int windows_seen_ = 0;
   int frames_window_size_ = kMinWindowSize;
+
+  // Worst case difference between a frame's intended duration and
+  // actual duration, calculated for all frames in the reporting interval.
+  base::TimeDelta max_single_frame_error_;
 };
 
 }  // namespace cc

content/renderer/media/media_factory.cc

@@ -155,17 +155,17 @@ void PostContextProviderToCallback(
                      unwanted_context_provider));
 }
 
-void LogRoughness(media::MediaLog* media_log,
-                  int size,
-                  base::TimeDelta duration,
-                  double roughness,
-                  int refresh_rate_hz,
-                  gfx::Size frame_size) {
+void LogRoughness(
+    media::MediaLog* media_log,
+    const cc::VideoPlaybackRoughnessReporter::Measurement& measurement) {
   // This function can be called from any thread. Don't do anything that assumes
   // a certain task runner.
-  double fps = std::round(size / duration.InSecondsF());
+  double fps =
+      std::round(measurement.frames / measurement.duration.InSecondsF());
   media_log->SetProperty<media::MediaLogProperty::kVideoPlaybackRoughness>(
-      roughness);
+      measurement.roughness);
+  media_log->SetProperty<media::MediaLogProperty::kVideoPlaybackFreezing>(
+      measurement.freezing);
   media_log->SetProperty<media::MediaLogProperty::kFramerate>(fps);
 
   // TODO(eugene@chromium.org) All of this needs to be moved away from
@@ -183,12 +183,16 @@ void LogRoughness(media::MediaLog* media_log,
   }
 
   // Only report known FPS buckets, on 60Hz displays and at least HD quality.
-  if (suffix != nullptr && refresh_rate_hz == 60 && frame_size.height() > 700) {
+  if (suffix != nullptr && measurement.refresh_rate_hz == 60 &&
+      measurement.frame_size.height() > 700) {
     base::UmaHistogramCustomTimes(
         base::JoinString({kRoughnessHistogramName, suffix}, "."),
-        base::TimeDelta::FromMillisecondsD(roughness),
+        base::TimeDelta::FromMillisecondsD(measurement.roughness),
         base::TimeDelta::FromMilliseconds(1),
         base::TimeDelta::FromMilliseconds(99), 100);
+    // TODO(liberato): Record freezing, once we're sure that we're computing the
+    // score we want.  For now, don't record anything so we don't have a mis-
+    // match of UMA values.
   }
 }
 

media/base/media_log_properties.cc

@@ -35,6 +35,7 @@ std::string MediaLogPropertyKeyToString(MediaLogProperty property) {
     STRINGIFY(kVideoTracks);
     STRINGIFY(kFramerate);
     STRINGIFY(kVideoPlaybackRoughness);
+    STRINGIFY(kVideoPlaybackFreezing);
   }
 #undef STRINGIFY
 }

media/base/media_log_properties.h

@@ -84,6 +84,10 @@ enum class MediaLogProperty {
 
   // A playback quality metric calculated by VideoPlaybackRoughnessReporter
   kVideoPlaybackRoughness,
+
+  // A playback quality metric that tries to account for large pauses and/or
+  // discontinuities during playback.
+  kVideoPlaybackFreezing,
 };
 
 MEDIA_LOG_PROPERTY_SUPPORTS_TYPE(kResolution, gfx::Size);
@@ -109,6 +113,7 @@ MEDIA_LOG_PROPERTY_SUPPORTS_TYPE(kTextTracks, std::vector<TextTrackConfig>);
 MEDIA_LOG_PROPERTY_SUPPORTS_TYPE(kVideoTracks, std::vector<VideoDecoderConfig>);
 MEDIA_LOG_PROPERTY_SUPPORTS_TYPE(kFramerate, double);
 MEDIA_LOG_PROPERTY_SUPPORTS_TYPE(kVideoPlaybackRoughness, double);
+MEDIA_LOG_PROPERTY_SUPPORTS_TYPE(kVideoPlaybackFreezing, base::TimeDelta);
 
 // Convert the enum to a string (used for the front-end enum matching).
 MEDIA_EXPORT std::string MediaLogPropertyKeyToString(MediaLogProperty property);

third_party/blink/public/platform/web_video_frame_submitter.h

@@ -42,7 +42,7 @@ class BLINK_PLATFORM_EXPORT WebVideoFrameSubmitter
  public:
   static std::unique_ptr<WebVideoFrameSubmitter> Create(
       WebContextProviderCallback,
-      cc::PlaybackRoughnessReportingCallback,
+      cc::VideoPlaybackRoughnessReporter::ReportingCallback,
       const cc::LayerTreeSettings&,
       bool use_sync_primitives);
   ~WebVideoFrameSubmitter() override = default;

third_party/blink/renderer/platform/exported/web_video_frame_submitter.cc

@@ -26,7 +26,8 @@ namespace blink {
 
 std::unique_ptr<WebVideoFrameSubmitter> WebVideoFrameSubmitter::Create(
     WebContextProviderCallback context_provider_callback,
-    cc::PlaybackRoughnessReportingCallback roughness_reporting_callback,
+    cc::VideoPlaybackRoughnessReporter::ReportingCallback
+        roughness_reporting_callback,
     const cc::LayerTreeSettings& settings,
     bool use_sync_primitives) {
   return std::make_unique<VideoFrameSubmitter>(

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

@@ -29,7 +29,8 @@ namespace blink {
 
 VideoFrameSubmitter::VideoFrameSubmitter(
     WebContextProviderCallback context_provider_callback,
-    cc::PlaybackRoughnessReportingCallback roughness_reporting_callback,
+    cc::VideoPlaybackRoughnessReporter::ReportingCallback
+        roughness_reporting_callback,
     std::unique_ptr<VideoFrameResourceProvider> resource_provider)
     : context_provider_callback_(context_provider_callback),
       resource_provider_(std::move(resource_provider)),
@@ -77,7 +78,6 @@ void VideoFrameSubmitter::StopRendering() {
   is_rendering_ = false;
 
   frame_trackers_.StopSequence(cc::FrameSequenceTrackerType::kVideo);
-  roughness_reporter_->Reset();
 
   UpdateSubmissionState();
 }
@@ -372,7 +372,13 @@ void VideoFrameSubmitter::UpdateSubmissionState() {
   if (!compositor_frame_sink_)
     return;
 
-  compositor_frame_sink_->SetNeedsBeginFrame(IsDrivingFrameUpdates());
+  const auto is_driving_frame_updates = IsDrivingFrameUpdates();
+  compositor_frame_sink_->SetNeedsBeginFrame(is_driving_frame_updates);
+  // If we're not driving frame updates, then we're paused / off-screen / etc.
+  // Roughness reporting should stop until we resume.  Since the current frame
+  // might be on-screen for a long time, we also discard the current window.
+  if (!is_driving_frame_updates)
+    roughness_reporter_->Reset();
 
   // These two calls are very important; they are responsible for significant
   // memory savings when content is off-screen.

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

@@ -45,7 +45,7 @@ class PLATFORM_EXPORT VideoFrameSubmitter
       public viz::mojom::blink::CompositorFrameSinkClient {
  public:
   VideoFrameSubmitter(WebContextProviderCallback,
-                      cc::PlaybackRoughnessReportingCallback,
+                      cc::VideoPlaybackRoughnessReporter::ReportingCallback,
                       std::unique_ptr<VideoFrameResourceProvider>);
   ~VideoFrameSubmitter() override;
 

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

@@ -167,7 +167,8 @@ class VideoFrameSubmitterTest : public testing::Test {
 
   void MakeSubmitter() { MakeSubmitter(base::DoNothing()); }
 
-  void MakeSubmitter(cc::PlaybackRoughnessReportingCallback reporting_cb) {
+  void MakeSubmitter(
+      cc::VideoPlaybackRoughnessReporter::ReportingCallback reporting_cb) {
     resource_provider_ = new StrictMock<MockVideoFrameResourceProvider>(
         context_provider_.get(), nullptr);
     submitter_ = std::make_unique<VideoFrameSubmitter>(
@@ -956,10 +957,9 @@ TEST_F(VideoFrameSubmitterTest, ProcessTimingDetails) {
   WTF::HashMap<uint32_t, viz::FrameTimingDetails> timing_details;
 
   MakeSubmitter(base::BindLambdaForTesting(
-      [&](int frames, base::TimeDelta duration, double roughness, int hz,
-          gfx::Size frame_size) {
-        ASSERT_EQ(frame_size.width(), 8);
-        ASSERT_EQ(frame_size.height(), 8);
+      [&](const cc::VideoPlaybackRoughnessReporter::Measurement& measurement) {
+        ASSERT_EQ(measurement.frame_size.width(), 8);
+        ASSERT_EQ(measurement.frame_size.height(), 8);
         reports++;
       }));
   EXPECT_CALL(*sink_, SetNeedsBeginFrame(true));