New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 793151 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Add is hardware decoded to WatchTime UKM metrics

Project Member Reported by dalecur...@chromium.org, Dec 8 2017

Issue description

This requires creating an enumeration of decoder names since we can't store string content in UKM fields. This gets us one step closer to not needing the Media.PipelineStatus enumerations.
 
Labels: -Type-Bug Type-Task
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 11 2017

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

commit 62fe0b34e77390dc7c8d11a8f360ba466149c7a1
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Mon Dec 11 19:35:11 2017

Stop posting video stats for every Render(); optimize storage.

There's really no reason for us to be posting this for every single
Render() call. They can be updated nearly as quickly during each
FrameReady() call. There's also no reason for us to create a new
PipelineStatistics structure every time either. Instead replace
a multitude of member variables with a PipelineStatistics member.

This is an optimization of PipelineStatistics usage so that we
can add a Decoder enum value later.

BUG= 793151 
TEST=existing tests pass.

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I5375acf74a625e1095e6631e177b4a04a4a0e54d
Reviewed-on: https://chromium-review.googlesource.com/816019
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523171}
[modify] https://crrev.com/62fe0b34e77390dc7c8d11a8f360ba466149c7a1/media/filters/decoder_stream_traits.cc
[modify] https://crrev.com/62fe0b34e77390dc7c8d11a8f360ba466149c7a1/media/filters/decoder_stream_traits.h
[modify] https://crrev.com/62fe0b34e77390dc7c8d11a8f360ba466149c7a1/media/renderers/video_renderer_impl.cc
[modify] https://crrev.com/62fe0b34e77390dc7c8d11a8f360ba466149c7a1/media/renderers/video_renderer_impl.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 11 2018

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

commit c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Thu Jan 11 20:01:05 2018

Add decoder name to PipelineStatistics; report to UKM.

There's value in tracking the decoder name along with the rest of
our WatchTime data, particularly LastPipelineStatus, for seeing
how a given decoder is performing in the field.

This adds the decoder name to the PipelineStatistics structure
and reports it up through the watch time reporter to the watch
time recorder. These end up reported as two new keys:

AudioDecoderName
VideoDecoderName

Since the decoder may change during playback due to errors or
configuration changes to unsupported content, we need to report
when fallback occurs. This is done by recreating the reporter
when a new decoder name is provided.

Decoder names are reported to UKM as zero valued enumerations;
the lists are as follows:

enum class AudioDecoderName : int {
  kUnknown = 0,  // Decoder name string is not recognized or n/a.
  kFFmpeg = 1,   // FFmpegAudioDecoder
  kMojo = 2,     // MojoAudioDecoder
};

enum class VideoDecoderName : int {
  kUnknown = 0,  // Decoder name string is not recognized or n/a.
  kGpu = 1,      // GpuVideoDecoder
  kFFmpeg = 2,   // FFmpegVideoDecoder
  kVpx = 3,      // VpxVideoDecoder
  kAom = 4,      // AomVideoDecoder
  kMojo = 5,     // MojoVideoDecoder
};

BUG= 793151 
TEST=updated unitests, manual verification

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: If13dae960130a535d3046f2a4b5d3ceb03f09339
Reviewed-on: https://chromium-review.googlesource.com/818532
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528719}
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/base/mock_filters.h
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/base/pipeline.h
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/base/pipeline_impl.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/base/pipeline_impl.h
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/base/pipeline_status.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/base/pipeline_status.h
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/blink/watch_time_reporter.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/blink/watch_time_reporter.h
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/blink/watch_time_reporter_unittest.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/filters/decoder_stream.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/filters/decoder_stream_traits.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/filters/pipeline_controller_unittest.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/mojo/clients/mojo_renderer.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/mojo/clients/mojo_renderer.h
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/mojo/interfaces/watch_time_recorder.mojom
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/mojo/services/media_resource_shim.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/mojo/services/watch_time_recorder.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/mojo/services/watch_time_recorder.h
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/mojo/services/watch_time_recorder_unittest.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/remoting/proto_utils.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/remoting/proto_utils_unittest.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/remoting/rpc.proto
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/renderers/video_renderer_impl_unittest.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/test/pipeline_integration_test_base.cc
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/media/test/pipeline_integration_test_base.h
[modify] https://crrev.com/c7d2a7d2291d41bf3b2bdb5b67bc2e733c9af6c7/tools/metrics/ukm/ukm.xml

Cc: hbengali@chromium.org
Labels: M-65
Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 13 2018

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

commit 8c8da87d1dfc4b12073a16c21693ef4863f3c52c
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Sat Jan 13 01:52:22 2018

Add decrypting a/v decoder names to UKM reporting.

http://crrev.com/528719 forgot to include the CDM wrapper decoders:
DecryptingAudioDecoder and DecryptingVideoDecoder. These will now
have enum values of 3 and 6 respectively.

BUG= 793151 
TEST=no more unknown decoder warning for Netflix.

Change-Id: I8d8ff1bf11dfe1ddb5fc0b58f60fba1a6cee0c28
Reviewed-on: https://chromium-review.googlesource.com/865823
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529141}
[modify] https://crrev.com/8c8da87d1dfc4b12073a16c21693ef4863f3c52c/media/mojo/services/watch_time_recorder.cc
[modify] https://crrev.com/8c8da87d1dfc4b12073a16c21693ef4863f3c52c/media/mojo/services/watch_time_recorder_unittest.cc

Sign in to add a comment