media: Report initial video height to UMA

media/blink/webmediaplayer_impl.cc

Index: media/blink/webmediaplayer_impl.cc
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc
index 7c10f9fe3981b98b65561a2e49d7f3a4fe8f9ca8..118ba7bde26c6c1804b87da34d47edf606cdca28 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -81,6 +81,9 @@ using gpu::gles2::GLES2Interface;
   static_assert(static_cast<int>(a) == static_cast<int>(b), \
                 "mismatching enums: " #a)
 
+#define UMA_HISTOGRAM_VIDEO_HEIGHT(name, sample) \

[DaleCurtis, 2017/04/14 22:03:38]

Define near where used and then use #undef afterward.

[xhwang, 2017/04/14 22:18:06]

Done.

+  UMA_HISTOGRAM_CUSTOM_COUNTS(name, sample, 64, 10000, 120)
+
 namespace media {
 
 namespace {
@@ -1398,28 +1401,29 @@ void WebMediaPlayerImpl::OnVideoNaturalSizeChange(const gfx::Size& size) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   DCHECK_NE(ready_state_, WebMediaPlayer::kReadyStateHaveNothing);
 
+  TRACE_EVENT0("media", "WebMediaPlayerImpl::OnNaturalSizeChanged");
+
   // The input |size| is from the decoded video frame, which is the original
   // natural size and need to be rotated accordingly.
   gfx::Size rotated_size =
       GetRotatedVideoSize(pipeline_metadata_.video_rotation, size);
 
-  if (rotated_size == pipeline_metadata_.natural_size)
+  RecordVideoNaturalSize(rotated_size);
+
+  gfx::Size old_size = pipeline_metadata_.natural_size;
+  if (rotated_size == old_size)
     return;
 
-  TRACE_EVENT0("media", "WebMediaPlayerImpl::OnNaturalSizeChanged");
-  media_log_->AddEvent(media_log_->CreateVideoSizeSetEvent(
-      rotated_size.width(), rotated_size.height()));
+  pipeline_metadata_.natural_size = rotated_size;
+
+  // WatchTimeReporter doesn't report metrics for empty videos. Re-create
+  // |watch_time_reporter_| if we didn't originally know the video size.
+  if (old_size.IsEmpty())
+    CreateWatchTimeReporter();
 
   if (overlay_enabled_ && surface_manager_)
     surface_manager_->NaturalSizeChanged(rotated_size);
 
-  gfx::Size old_size = pipeline_metadata_.natural_size;
-  pipeline_metadata_.natural_size = rotated_size;
-  if (old_size.IsEmpty()) {
-    // WatchTimeReporter doesn't report metrics for empty videos. Re-create
-    // |watch_time_reporter_| if we didn't originally know the video size.
-    CreateWatchTimeReporter();
-  }
   client_->SizeChanged();
 
   if (observer_)
@@ -2348,4 +2352,29 @@ void WebMediaPlayerImpl::RecordUnderflowDuration(base::TimeDelta duration) {
     UMA_HISTOGRAM_TIMES("Media.UnderflowDuration.MSE", duration);
 }
 
+void WebMediaPlayerImpl::RecordVideoNaturalSize(const gfx::Size& natural_size) {
+  // Always report video natural size to MediaLog.
+  media_log_->AddEvent(media_log_->CreateVideoSizeSetEvent(
+      natural_size.width(), natural_size.height()));
+
+  // TODO(xhwang): Also report "average" video height for this playback session.

[DaleCurtis, 2017/04/14 22:03:38]

Want to go ahead and just add this now?

[xhwang, 2017/04/14 22:18:06]

I want to merge this back to M59 to collect data early, so I want to keep this
CL simple. The average video height might need to get MediaInternalsUMAHandler
involved since we will only have the final "average" value after playback
finished.

[xhwang, 2017/04/14 23:07:41]

Done.

+  // See http://crbug.com/709354
+
+  if (initial_video_height_recorded_)

[tguilbert, 2017/04/14 22:00:49]

Drive-by: For HLS, our original video size is always 0, because we do not know
the video size until we start playing. I am not sure if the initial size would
be reported correctly in this case.

[xhwang, 2017/04/14 22:04:53]

Good point. 

Will OnVideoNaturalSizeChange() be called with 0 size in the HLS case? If so, we
can define the "initial size" to be the "first non-zero size", and adjust the
code to accommodate this case. 

+    return;
+
+  initial_video_height_recorded_ = true;
+  int height = natural_size.height();
+
+  if (load_type_ == kLoadTypeURL)
+    UMA_HISTOGRAM_VIDEO_HEIGHT("Media.VideoHeight.Initial.SRC", height);
+  else if (load_type_ == kLoadTypeMediaSource)
+    UMA_HISTOGRAM_VIDEO_HEIGHT("Media.VideoHeight.Initial.MSE", height);
+
+  if (is_encrypted_)
+    UMA_HISTOGRAM_VIDEO_HEIGHT("Media.VideoHeight.Initial.EME", height);
+
+  UMA_HISTOGRAM_VIDEO_HEIGHT("Media.VideoHeight.Initial.All", height);
+}
+
 }  // namespace media