Index: media/blink/webmediaplayer_impl.cc |
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc |
index 7c10f9fe3981b98b65561a2e49d7f3a4fe8f9ca8..1ed4ce37b47af8c4ddd89dcfd777cb41a902458b 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) \ |
+ 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); |
DaleCurtis
2017/04/14 18:30:19
Put below l.1415 to avoid duplicate size events?
xhwang
2017/04/14 20:30:35
OnVideoNaturalSizeChange() should only be called o
|
+ |
+ 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,33 @@ 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 18:30:19
Why not just record every size change? Why the foc
xhwang
2017/04/14 20:30:35
As discussed offline, this is useful to measure th
|
+ // See http://crbug.com/709354 |
+ |
+ if (initial_video_height_recorded_) |
+ return; |
+ |
+ initial_video_height_recorded_ = true; |
+ |
+ int height = natural_size.height(); |
+ // For vertical videos, use it's width instead of height for UMA reporting. |
DaleCurtis
2017/04/14 18:48:57
Hmmm, why? Seems like this might muddy the metrics
xhwang
2017/04/14 20:30:35
I was debating on this myself. Unless we report th
|
+ if (height > natural_size.width()) |
+ height = natural_size.width(); |
+ |
+ 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); |
DaleCurtis
2017/04/14 18:48:57
All
xhwang
2017/04/14 20:30:35
Done. Thanks for catching this!
|
+} |
+ |
} // namespace media |