New issue
Advanced search Search tips

Issue 735238 link

Starred by 2 users
Status: Fixed
Owner:
dalecur...@chromium.org
Closed: Sep 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Feature



Sign in to add a comment

Add UKM metrics for watch time by codecs

Project Member Reported by hbengali@chromium.org, Jun 20 2017 
We currently only count instantiations of media files with each codec, but we want to start measuring watch time by codec so we can surface this on the internal Chrome media dashboard.

The proposal is to add Media.WatchTime.VideoCodec.[CodecName] histograms for the following:

VP9
VP8
H264
Theora
(AV1, if it's possible to setup instrumentation before actual decoding support lands)

And Media.WatchTime.AudioCodec.[CodecName] histograms for the following:

AAC
MP3
PCM
Vorbis
FLAC
Opus

Comment 1 by dalecur...@chromium.org, Jun 20 2017 

I don't think we should do this in UMA. Instead we should do this only in UKM which allows for free form data, see

https://chromium-review.googlesource.com/c/538119/

Continuing to blow up the histogram permutations like this is not sustainable.

Comment 2 by dalecur...@chromium.org, Jun 20 2017

Labels: -Restrict-View-Google
Owner: dalecur...@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by hbengali@chromium.org, Jun 21 2017

Summary: Add UKM metrics for watch time by codecs (was: Add UMA metrics for watch time by codecs)
SGTM. Updating the bug title to reflect that we'll use UKM
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 28 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d

commit 1b5b36fd2dcb4fa11d6453da1189be00e9cc892d
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Fri Jul 28 01:39:56 2017

Refactor watch time reporting into a mojo service.

It's not sustainable to continue to stuff metrics into the MediaInternals
reporter. Since these values aren't even displayed in the internals page
they should go into their own mojo service. Which has the benefit of
being a much simpler implementation to boot (~250 lines less).

We now have a mojo WatchTimeRecorder interface that the WatchTimeReporter
can send watch time updates too. It can also finalize individual keys with
this new interface. As with the old interface if the process dies the mojo
channel will close and we'll finalize all held values at destruction.

BUG=716643,  735238 
TEST=new/existing unittests all pass

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I0565422fbf70eba676dc22e08ea22cec88f09778
Reviewed-on: https://chromium-review.googlesource.com/546903
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490187}
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/content/browser/media/media_internals.cc
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/content/browser/media/media_internals.h
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/content/browser/media/media_internals_unittest.cc
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/content/renderer/media/media_factory.cc
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/content/renderer/media/media_factory.h
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/content/renderer/media/render_media_log.cc
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/base/ipc/media_param_traits_macros.h
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/base/media_log.cc
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/base/media_log_event.h
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/base/watch_time_keys.cc
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/base/watch_time_keys.h
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/blink/BUILD.gn
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/blink/DEPS
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/blink/watch_time_reporter.cc
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/blink/watch_time_reporter.h
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/blink/watch_time_reporter_unittest.cc
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/blink/webmediaplayer_impl_unittest.cc
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/blink/webmediaplayer_params.cc
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/blink/webmediaplayer_params.h
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/mojo/BUILD.gn
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/mojo/DEPS
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/mojo/interfaces/BUILD.gn
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/mojo/interfaces/media_types.mojom
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/mojo/interfaces/media_types.typemap
[add] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/mojo/interfaces/watch_time_recorder.mojom
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/mojo/services/BUILD.gn
[add] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/mojo/services/watch_time_recorder.cc
[add] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/mojo/services/watch_time_recorder.h
[add] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/media/mojo/services/watch_time_recorder_unittest.cc
[modify] https://crrev.com/1b5b36fd2dcb4fa11d6453da1189be00e9cc892d/services/metrics/public/cpp/ukm_recorder.h
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 28 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/114b70bcea84830d1248cd72fc1689b7e82416a8

commit 114b70bcea84830d1248cd72fc1689b7e82416a8
Author: meade_UTC10 <meade@chromium.org>
Date: Fri Jul 28 02:04:14 2017

Revert "Refactor watch time reporting into a mojo service."

This reverts commit 1b5b36fd2dcb4fa11d6453da1189be00e9cc892d.

Reason for revert: Broke compilation and caused the tree to be closed

Original change's description:
> Refactor watch time reporting into a mojo service.
> 
> It's not sustainable to continue to stuff metrics into the MediaInternals
> reporter. Since these values aren't even displayed in the internals page
> they should go into their own mojo service. Which has the benefit of
> being a much simpler implementation to boot (~250 lines less).
> 
> We now have a mojo WatchTimeRecorder interface that the WatchTimeReporter
> can send watch time updates too. It can also finalize individual keys with
> this new interface. As with the old interface if the process dies the mojo
> channel will close and we'll finalize all held values at destruction.
> 
> BUG=716643,  735238 
> TEST=new/existing unittests all pass
> 
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
> Change-Id: I0565422fbf70eba676dc22e08ea22cec88f09778
> Reviewed-on: https://chromium-review.googlesource.com/546903
> Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
> Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
> Reviewed-by: Nasko Oskov <nasko@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
> Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#490187}

TBR=dalecurtis@chromium.org,nasko@chromium.org,xhwang@chromium.org,rkaplow@chromium.org,yzshen@chromium.org,mlamouri@chromium.org,chcunningham@chromium.org

Change-Id: Ie678350b476efc0ad1dede4672d6bd4bd4bd9efb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 716643,  735238 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Reviewed-on: https://chromium-review.googlesource.com/590334
Reviewed-by: meade_UTC10 <meade@chromium.org>
Commit-Queue: meade_UTC10 <meade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490204}
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/content/browser/media/media_internals.cc
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/content/browser/media/media_internals.h
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/content/browser/media/media_internals_unittest.cc
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/content/renderer/media/media_factory.cc
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/content/renderer/media/media_factory.h
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/content/renderer/media/render_media_log.cc
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/base/ipc/media_param_traits_macros.h
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/base/media_log.cc
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/base/media_log_event.h
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/base/watch_time_keys.cc
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/base/watch_time_keys.h
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/blink/BUILD.gn
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/blink/DEPS
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/blink/watch_time_reporter.cc
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/blink/watch_time_reporter.h
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/blink/watch_time_reporter_unittest.cc
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/blink/webmediaplayer_impl_unittest.cc
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/blink/webmediaplayer_params.cc
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/blink/webmediaplayer_params.h
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/mojo/BUILD.gn
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/mojo/DEPS
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/mojo/interfaces/BUILD.gn
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/mojo/interfaces/media_types.mojom
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/mojo/interfaces/media_types.typemap
[delete] https://crrev.com/2e3125b7dff512325a9a704b8d9f36483422d59c/media/mojo/interfaces/watch_time_recorder.mojom
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/media/mojo/services/BUILD.gn
[delete] https://crrev.com/2e3125b7dff512325a9a704b8d9f36483422d59c/media/mojo/services/watch_time_recorder.cc
[delete] https://crrev.com/2e3125b7dff512325a9a704b8d9f36483422d59c/media/mojo/services/watch_time_recorder.h
[delete] https://crrev.com/2e3125b7dff512325a9a704b8d9f36483422d59c/media/mojo/services/watch_time_recorder_unittest.cc
[modify] https://crrev.com/114b70bcea84830d1248cd72fc1689b7e82416a8/services/metrics/public/cpp/ukm_recorder.h
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 2 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1adbe6aa7c5c005de36840c3f78834ef49798a66

commit 1adbe6aa7c5c005de36840c3f78834ef49798a66
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Wed Aug 02 02:09:13 2017

Reland "Refactor watch time reporting into a mojo service."

This is a reland of 1b5b36fd2dcb4fa11d6453da1189be00e9cc892d
Original change's description:
> Refactor watch time reporting into a mojo service.
> 
> It's not sustainable to continue to stuff metrics into the MediaInternals
> reporter. Since these values aren't even displayed in the internals page
> they should go into their own mojo service. Which has the benefit of
> being a much simpler implementation to boot (~250 lines less).
> 
> We now have a mojo WatchTimeRecorder interface that the WatchTimeReporter
> can send watch time updates too. It can also finalize individual keys with
> this new interface. As with the old interface if the process dies the mojo
> channel will close and we'll finalize all held values at destruction.
> 
> BUG=716643,  735238 
> TEST=new/existing unittests all pass
> 
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
> Change-Id: I0565422fbf70eba676dc22e08ea22cec88f09778
> Reviewed-on: https://chromium-review.googlesource.com/546903
> Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
> Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
> Reviewed-by: Nasko Oskov <nasko@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
> Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#490187}

TBR=xhwang,nasko,rkaplow,yzshen,mlamouri

Bug: 716643,  735238 
Change-Id: Ife0d1287dc4d8655ccbc98cdcbead48659eb685a
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Reviewed-on: https://chromium-review.googlesource.com/593855
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491219}
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/content/browser/media/media_internals.cc
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/content/browser/media/media_internals.h
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/content/browser/media/media_internals_unittest.cc
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/content/renderer/media/media_factory.cc
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/content/renderer/media/media_factory.h
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/content/renderer/media/render_media_log.cc
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/base/ipc/media_param_traits_macros.h
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/base/media_log.cc
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/base/media_log_event.h
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/base/watch_time_keys.cc
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/base/watch_time_keys.h
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/blink/BUILD.gn
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/blink/DEPS
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/blink/watch_time_reporter.cc
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/blink/watch_time_reporter.h
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/blink/watch_time_reporter_unittest.cc
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/blink/webmediaplayer_impl_unittest.cc
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/blink/webmediaplayer_params.cc
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/blink/webmediaplayer_params.h
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/mojo/BUILD.gn
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/mojo/DEPS
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/mojo/interfaces/BUILD.gn
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/mojo/interfaces/media_types.mojom
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/mojo/interfaces/media_types.typemap
[add] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/mojo/interfaces/watch_time_recorder.mojom
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/mojo/services/BUILD.gn
[add] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/mojo/services/watch_time_recorder.cc
[add] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/mojo/services/watch_time_recorder.h
[add] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/media/mojo/services/watch_time_recorder_unittest.cc
[modify] https://crrev.com/1adbe6aa7c5c005de36840c3f78834ef49798a66/services/metrics/public/cpp/ukm_recorder.h

Comment 7 by dalecur...@chromium.org, Sep 20 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment