New issue
Advanced search Search tips

Issue 763528 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Re-enable H/W MP4 MSE config-change layout tests on Android

Project Member Reported by wolenetz@chromium.org, Sep 8 2017

Issue description

It looks like https://chromium-review.googlesource.com/c/chromium/src/+/481042 undid chcunningham@'s additions of these tests to SmokeTests:

virtual/media-gpu-accelerated/http/tests/media/media-source/mediasource-play.html
virtual/media-gpu-accelerated/http/tests/media/media-source/mediasource-config-change-mp4-av-audio-bitrate.html
virtual/media-gpu-accelerated/http/tests/media/media-source/mediasource-config-change-mp4-av-framesize.html
virtual/media-gpu-accelerated/http/tests/media/media-source/mediasource-config-change-mp4-v-framerate.html

This is surprising, since running those specific tests was the primary reason for the media-gpu-accelerated virtual test suite.

Was this intentional or accidental? If the latter, they need to be added back IMHO. If the former, I need to be enlightened :)
 
Owner: wolenetz@chromium.org
Status: Started (was: Untriaged)
With possible naivete as to why these tests were previously removed, I'm going to prep a CL to re-add them...
Though not MSE, virtual/media-gpu-accelerated/media/video-autoplay.html appears to also have been accidentally removed in https://chromium-review.googlesource.com/c/chromium/src/+/481042.

Blocking: 718641
Blocking: -718641
Ideally this would be fixed to help automate mp4 version of layout testing in support of  bug 718641 , but since the android_blink_rel bot isn't liking some of the re-added mp4 config change tests, I'm not going to block  bug 718641  further on getting this bug fixed. For  bug 718641 's verifying pts/dts-related fixes to various layout tests, I can run the related layout tests locally on an mp4-configured build instead (like we always have had to do so far.. was just hoping to end-run that).
As you expected: The removal of those tests from the list was accidental, not intentional.
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 11 2017

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

commit 3e5e86cfeea0cf627b5655b16e57f117fe46c341
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Mon Sep 11 23:35:47 2017

Re-enable media-gpu-accelerated SmokeTests

It looks like https://chromium-review.googlesource.com/c/chromium/src/+/481042
accidentally undid chcunningham@'s additions of all of these tests to
SmokeTests.

This is surprising, since running those specific tests was the primary
reason for the media-gpu-accelerated virtual test suite.

BUG= 763528 , 599975 , 555703 

Change-Id: Ia02f8c144dccd8709b6cda854089a166c667a8b9
Reviewed-on: https://chromium-review.googlesource.com/658356
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501098}
[modify] https://crrev.com/3e5e86cfeea0cf627b5655b16e57f117fe46c341/third_party/WebKit/LayoutTests/SmokeTests
[add] https://crrev.com/3e5e86cfeea0cf627b5655b16e57f117fe46c341/third_party/WebKit/LayoutTests/platform/android/http/tests/media/media-source/mediasource-config-change-mp4-av-audio-bitrate-expected.txt
[add] https://crrev.com/3e5e86cfeea0cf627b5655b16e57f117fe46c341/third_party/WebKit/LayoutTests/platform/android/http/tests/media/media-source/mediasource-config-change-mp4-av-framesize-expected.txt
[add] https://crrev.com/3e5e86cfeea0cf627b5655b16e57f117fe46c341/third_party/WebKit/LayoutTests/platform/android/http/tests/media/media-source/mediasource-config-change-mp4-v-framerate-expected.txt

Status: Fixed (was: Started)
#7 should fix this.
Flakiness DB shows green for these MSE SmokeTests re-enabled in #7:
* 3 MP4 config-change SmokeTests (and about 3-4 seconds execution time per each of those 3): https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_tests&tests=media-gpu-accelerated%2Fhttp%2Ftests%2Fmedia%2Fmedia-source%2Fmediasource-config-change
* The mediasource-play SmokeTest (about 2 seconds exec time) 

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 13 2017

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

commit 6315c77889427fffb84dea412dddc39e6e69b017
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Wed Sep 13 23:10:12 2017

SmokeTests: Allow comments and require Tools OWNERS to review changes

To help prevent accidental SmokeTests coverage regressions like that
fixed in  bug 763528 , this change allows '#' prefixed comments in
SmokeTests, and also makes changes to SmokeTests require Tools OWNERS
review.

BUG= 764556 , 763528 

Change-Id: I612fb2b67fbe9f0a99107c1079b79a4b61338944
Reviewed-on: https://chromium-review.googlesource.com/665350
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501783}
[modify] https://crrev.com/6315c77889427fffb84dea412dddc39e6e69b017/third_party/WebKit/LayoutTests/OWNERS
[modify] https://crrev.com/6315c77889427fffb84dea412dddc39e6e69b017/third_party/WebKit/LayoutTests/SmokeTests
[modify] https://crrev.com/6315c77889427fffb84dea412dddc39e6e69b017/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py

Sign in to add a comment