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

Issue 768621 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

video_ChromeVidResChangeHWDecode is failing on M63 with an error "Video either stopped or ended"

Project Member Reported by avkodipelli@chromium.org, Sep 26 2017

Issue description

Issue is still happening on M63 and M64 latest builds.
Cc: owenlin@chromium.org hiroh@chromium.org
Owner: kcwu@chromium.org
Status: Assigned (was: Untriaged)
Owner: hiroh@chromium.org

Comment 5 by hiroh@chromium.org, Oct 23 2017

This is caused by the following Chromium change. crrev.com/c/651237
I confirmed the test was passed on Chrome ToT by reverting this change.
Seeing the commit message, this change is an optimization change. It pauses a video if a video tab is background.
video_ChromeVidResChangeHWDecode opens histogram page, after starting playing a test video.
Then, the video tab goes background and it get paused by this change.
Is it possible to see histogram value without opening the histogram tab, or opening it background?

Comment 6 by hiroh@chromium.org, Oct 23 2017

tabs.New() doesn't have such specific function. 
https://chromium.googlesource.com/chromium/src/+/dde688fb441bd1c2be34c50b8b88c2791bcc4765/tools/telemetry/telemetry/internal/browser/tab_list.py#8
How about opening another Chrome "Window" and check the histogram value on that?
We definitely confirm it doesn't affect playback performance.

Comment 7 by hiroh@chromium.org, Oct 23 2017

histogram_verifier.verify() are only used in three tests. Only video_ChromeVidResChange opens a tab (call the function) while a video is being played.
So we need to open a new chrome window in video_ChromeVidResChange.
My idea is add a new argument which identify if a chrome window is newly opened.
The argument is only true in video_ChromeVidResChange.
I think it'd be preferable not to depend on such a solution. We could have a similar feature to pause when another window comes into focus/is opened/covers the current one in the future. Also, opening a window may affect performance tests.

It would be best to have a way to query the histogram from the tab, or use some other means (perhaps a js call on the video element, if such capability exists).

Comment 9 by hiroh@chromium.org, Oct 25 2017

I would fix this test failure by opening a histogram tab after a video is ended.
The way without opening a histogram tab is still better though.
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/6ab4f013dccdaf88bdcb8a5c3a25eb53b0ec327e

commit 6ab4f013dccdaf88bdcb8a5c3a25eb53b0ec327e
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Thu Oct 26 02:30:16 2017

Open chrome://hitograms after a video is ended in ChromeVidResChangeHWDecode test

ChromeVidResChnageHWDecode opens chrome://histograms to check
Media.GpuVideoDecoderInitializeStatus value while a test video is played.
Because of that, the video played tab becomes background.
Since Chrome pauses a video in background tab, the test is failed because it
detects a test video is unexpectedly stopped.
To fix this failure, the test opens chrome://histograms after a test video is ended.

BUG= chromium:768621 
TEST=video_ChromeVidResChangeHWDecode on caroline

Change-Id: Id62970a91d1ff416820812c1d45ddd34c1762746
Reviewed-on: https://chromium-review.googlesource.com/734881
Commit-Ready: Hirokazu Honda <hiroh@chromium.org>
Tested-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Kuang-che Wu <kcwu@chromium.org>

[modify] https://crrev.com/6ab4f013dccdaf88bdcb8a5c3a25eb53b0ec327e/client/site_tests/video_ChromeVidResChangeHWDecode/video_ChromeVidResChangeHWDecode.py

Comment 11 by hiroh@chromium.org, Oct 27 2017

Status: Verified (was: Assigned)
The test start to be passed on all the boards now.

Comment 12 by kcwu@chromium.org, Oct 27 2017

Status: Assigned (was: Verified)
This need to be merged to R63.

Comment 13 by hiroh@chromium.org, Oct 27 2017

Labels: Merge-Request-63
Project Member

Comment 15 by sheriffbot@chromium.org, Oct 27 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Owner: gkihumba@chromium.org
Hi gkihumba@,
CL in comment#14 is requested to merge to R63. The change is only in autotest code video_ChromeVidResChangeHWDecode.py

All boards are failing video_ChromeVidResChangeHWDecode without the change in R63, R64 is green now.
https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/testDetails?milestone=63&testName=video_ChromeVidResChangeHWDecode

Thanks
Labels: -Merge-Review-63 Merge-Approved-63
Labels: -Hotlist-Merge-Review

Comment 19 by kcwu@chromium.org, Nov 6 2017

Owner: hiroh@chromium.org
Project Member

Comment 20 by sheriffbot@chromium.org, Nov 6 2017

Cc: josa...@chromium.org
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: -josa...@chromium.org gkihumba@chromium.org
Hirokazu, any update on the merge here? 
Project Member

Comment 22 by bugdroid1@chromium.org, Nov 6 2017

Labels: merge-merged-release-R63-10032.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/1e8e284bdf800e571d962595bce374454cee5d96

commit 1e8e284bdf800e571d962595bce374454cee5d96
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Mon Nov 06 23:05:09 2017

Open chrome://hitograms after a video is ended in ChromeVidResChangeHWDecode test

ChromeVidResChnageHWDecode opens chrome://histograms to check
Media.GpuVideoDecoderInitializeStatus value while a test video is played.
Because of that, the video played tab becomes background.
Since Chrome pauses a video in background tab, the test is failed because it
detects a test video is unexpectedly stopped.
To fix this failure, the test opens chrome://histograms after a test video is ended.

BUG= chromium:768621 
TEST=video_ChromeVidResChangeHWDecode on caroline

Change-Id: Id62970a91d1ff416820812c1d45ddd34c1762746
Reviewed-on: https://chromium-review.googlesource.com/734881
Commit-Ready: Hirokazu Honda <hiroh@chromium.org>
Tested-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Kuang-che Wu <kcwu@chromium.org>
(cherry picked from commit 6ab4f013dccdaf88bdcb8a5c3a25eb53b0ec327e)
Reviewed-on: https://chromium-review.googlesource.com/740590
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Trybot-Ready: Hirokazu Honda <hiroh@chromium.org>

[modify] https://crrev.com/1e8e284bdf800e571d962595bce374454cee5d96/client/site_tests/video_ChromeVidResChangeHWDecode/video_ChromeVidResChangeHWDecode.py

I am sorry for the delay response. I merged this CL now and mark verified after the test was passed on R63 branch.
Status: Verified (was: Assigned)
Project Member

Comment 25 by sheriffbot@chromium.org, Nov 10 2017

Cc: josa...@chromium.org
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 26 by hiroh@chromium.org, Dec 19 2017

Labels: -Arch-All -M-63 -Merge-Approved-63 -merge-merged-release-R63-10032.B

Sign in to add a comment