New issue
Advanced search Search tips

Issue 902968 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 906994
issue 902870
issue 906475
issue 907320



Sign in to add a comment

Enable MojoVideoDecoder (via an adapter) by default on CrOS

Project Member Reported by posciak@chromium.org, Nov 7

Issue description

MojoVideoDecoder is being enabled by default (issue 522298), to replace GpuVideoDecoder. Currently for CrOS, it uses an adapter to convert to legacy VDA API in GPU process. Work on migrating CrOS VDAs to VideoDecoder is currently in progress.

Switching to MVD requires adapting our tests and histograms, which use GVD-based histograms for verification. We should update and verify our tests, and enable MVD by enabling kMojoVideoDecoder feature in media/base/media_switches.cc by default for CrOS.
 
In GpuVideoDecoder, we add 2 histogram:
- Media.GpuVideoDecoderInitializeStatus
- Media.GpuVideoDecoderError


To switch to MojoVideoDecoder, we should to:

1. Add the same histogram and another equivalent one at MojoVideoDecoder. i.e. - Media.MojoVideoDecoderInitializeStatus
- Media.MojoVideoDecoderError
- Media.GpuVideoDecoderInitializeStatus
- Media.GpuVideoDecoderError

2. Enable the MojoVideoDecoder

3. Change to verify histogram "Media.MojoVideoDecoder*" at autotest

4. Remove "Media.GpuVideoDecoder*" from MojoVideoDecoder
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 14

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

commit 6aac2af34906d6ed98d74773711cddba39ca9825
Author: Chih-Yu Huang <akahuang@google.com>
Date: Wed Nov 14 03:39:32 2018

MojoVideoDecoder: Add histogram to track initialization and error.

The autotest video_ChromeHWDecodeUsed verifies Chrome used the HW
decoding when playing a video stream by checking the histogram value,
which is reported from GpuVideoDecoder.
In order to keep the test pass during switching from GpuVideoDecoder
to MojoVideoDecoder, we have to report the same histogram value
temporarily from MojoVideoDecoder.

With this CL, we can enable MojoVideoDecoder at ChromeOS platform.

BUG=chromium:902968
TEST=Pass video_ChromeHWDecodeUsed autotest on Eve
TEST=Play video at Youtube on Eve and confirm the successfull option
     of Media.GpuVideoDecoderInitializeStatus are increased.

Change-Id: I26fc53704843e9b153bbee304e8f60ab3a7b0417
Reviewed-on: https://chromium-review.googlesource.com/c/1328634
Commit-Queue: Chih-Yu Huang <akahuang@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607890}
[modify] https://crrev.com/6aac2af34906d6ed98d74773711cddba39ca9825/media/base/media_switches.cc
[modify] https://crrev.com/6aac2af34906d6ed98d74773711cddba39ca9825/media/mojo/clients/mojo_video_decoder.cc

Updated:

Offline discussed with Hiro, currently we mainly used the histogram to verify HW acceleration is used and no error occurs at autotest.
Though we can analysis the histogram from data reported from users, but we should be able to detect it at the lab.

Therefore I will add trace instead of new histogram at MojoVD for testing.
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 16

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

commit 6f002a06a222317492af20934a4c6936cb056bde
Author: Scott Violet <sky@chromium.org>
Date: Fri Nov 16 18:59:57 2018

Revert "MojoVideoDecoder: Add histogram to track initialization and error."

This reverts commit 6aac2af34906d6ed98d74773711cddba39ca9825.

Reason for revert: Caused breakage of mash test. See  https://crbug.com/902870 .

Original change's description:
> MojoVideoDecoder: Add histogram to track initialization and error.
> 
> The autotest video_ChromeHWDecodeUsed verifies Chrome used the HW
> decoding when playing a video stream by checking the histogram value,
> which is reported from GpuVideoDecoder.
> In order to keep the test pass during switching from GpuVideoDecoder
> to MojoVideoDecoder, we have to report the same histogram value
> temporarily from MojoVideoDecoder.
> 
> With this CL, we can enable MojoVideoDecoder at ChromeOS platform.
> 
> BUG=chromium:902968
> TEST=Pass video_ChromeHWDecodeUsed autotest on Eve
> TEST=Play video at Youtube on Eve and confirm the successfull option
>      of Media.GpuVideoDecoderInitializeStatus are increased.
> 
> Change-Id: I26fc53704843e9b153bbee304e8f60ab3a7b0417
> Reviewed-on: https://chromium-review.googlesource.com/c/1328634
> Commit-Queue: Chih-Yu Huang <akahuang@chromium.org>
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#607890}

TBR=posciak@chromium.org,sandersd@chromium.org,akahuang@chromium.org,hiroh@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: chromium:902968
Change-Id: I3b74c752b6cc34dcbcba203eeb39707f4921d405
Reviewed-on: https://chromium-review.googlesource.com/c/1340378
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608874}
[modify] https://crrev.com/6f002a06a222317492af20934a4c6936cb056bde/media/base/media_switches.cc
[modify] https://crrev.com/6f002a06a222317492af20934a4c6936cb056bde/media/mojo/clients/mojo_video_decoder.cc

Blockedon: 906475
Blockedon: 902870
Blockedon: 906994
Because the performance issue (crbug.com/906994), we probably won't enable MojoVD at R72.
Blockedon: 907320
Project Member

Comment 10 by bugdroid1@chromium.org, Jan 15

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

commit c913665f44585ac8c621b8bdc2142061a5046e0b
Author: Chih-Yu Huang <akahuang@google.com>
Date: Tue Jan 15 05:41:36 2019

MojoVideoDecoder: Add histogram to track initialization and error.

The autotest video_ChromeHWDecodeUsed verifies Chrome used the HW
decoding when playing a video stream by checking the histogram value,
which is reported from GpuVideoDecoder.
In order to keep the test pass during switching from GpuVideoDecoder
to MojoVideoDecoder, we have to report the same histogram value
temporarily from MojoVideoDecoder.

BUG=chromium:902968
TEST=Enable MojoVideoDecoder and play video at Youtube on Eve.
     Confirm the successfull option of
     Media.GpuVideoDecoderInitializeStatus are increased.
TEST=tast -verbose run -build=false $(DUT) video.PlayDecodeAccelUsed*
TEST=test_that -b eve $(DUT) video_ChromeHWDecodeUsed.h264

Change-Id: I172115f170e5ed6706d1866747c7a8cf90a8320f
Reviewed-on: https://chromium-review.googlesource.com/c/1405164
Commit-Queue: Chih-Yu Huang <akahuang@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622749}
[modify] https://crrev.com/c913665f44585ac8c621b8bdc2142061a5046e0b/media/mojo/clients/mojo_video_decoder.cc

Sign in to add a comment