New issue
Advanced search Search tips

Issue 689781 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 2
Type: Bug
Flaky-Test: http/tests/media/media-source/mediasource-duration.html



Sign in to add a comment

Flaky layout test: mediasource-duration.html (external/wpt version is still flaky)

Project Member Reported by shimazu@chromium.org, Feb 8 2017

Issue description

Project Member

Comment 1 by bugdroid1@chromium.org, Feb 8 2017

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

commit ea12101af3165ed67b9bebd37e8a0ddcc43b3e04
Author: shimazu <shimazu@chromium.org>
Date: Wed Feb 08 03:49:04 2017

Mark mediasource-duration.html as flaky

BUG=689781
TBR=wolenetz@chromium.org

Review-Url: https://codereview.chromium.org/2683933002
Cr-Commit-Position: refs/heads/master@{#448899}

[modify] https://crrev.com/ea12101af3165ed67b9bebd37e8a0ddcc43b3e04/third_party/WebKit/LayoutTests/TestExpectations

Cc: -wolenetz@chromium.org
Owner: wolenetz@chromium.org
Status: Assigned (was: Available)
virtual/mojo-loading/http/tests/media/media-source/mediasource-duration.html is flaky with frequency 3/25. I'll mark that one as flaky as well.


Project Member

Comment 6 by bugdroid1@chromium.org, Feb 13 2017

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

commit d83110a5ee45420581f6558715a4dc0c67263a96
Author: msramek <msramek@chromium.org>
Date: Mon Feb 13 15:45:15 2017

Disabled the flaky virtual/mojo-loading/http/tests/media/media-source/mediasource-duration.html

TBR=wolenetz@chromium.org
NOTRY=True
BUG=689781

Review-Url: https://codereview.chromium.org/2693953002
Cr-Commit-Position: refs/heads/master@{#449965}

[modify] https://crrev.com/d83110a5ee45420581f6558715a4dc0c67263a96/third_party/WebKit/LayoutTests/TestExpectations

Quick inspection shows some current flakiness still, last two subtests within this file appear flaky:

WebKit Linux Trusty:
FAIL Test duration reduction below highest buffered presentation time is disallowed assert_equals: mediaElement duration matches fullDuration expected 6.042 but got NaN

WebKit Linux Trusty (dbg):
FAIL Test setting same duration multiple times does not fire duplicate durationchange assert_equals: mediaElement duration matches newDuration expected 6.042 but got 12.084

Linux Tests:
FAIL Test duration reduction below highest buffered presentation time is disallowed assert_equals: mediaElement duration matches fullDuration expected 6.042 but got NaN



Project Member

Comment 8 by bugdroid1@chromium.org, Aug 4 2017

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

commit 48abf3c391c49c611fde0f8f59ca9adb29dcb2ad
Author: Philip Jägenstedt <foolip@chromium.org>
Date: Fri Aug 04 11:44:37 2017

Mark mediasource-duration.html (wpt version) as flaky

Enabled in https://chromium-review.googlesource.com/599728 but now
failing on WebKit Linux Trusty (dbg) in the waterfall.

Put it with the same test in other places.

Bug: 689781
Change-Id: I37a8e13ae607a18fffe3b60aa4f8812e9aadd448
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/601911
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491994}
[modify] https://crrev.com/48abf3c391c49c611fde0f8f59ca9adb29dcb2ad/third_party/WebKit/LayoutTests/TestExpectations

Cc: chcunningham@chromium.org
With a debug build including proprietary codecs on linux, I have a local repro of flaky (1 fail out of 100 runs, ~30-80/100 when logged/tee'd directly to terminal):

OUT: FAIL Test setting same duration multiple times does not fire duplicate durationchange assert_equals: mediaElement duration matches newDuration expected 6.042 but got 12.084

Also, a few other flaky fails:

OUT: FAIL Test appendBuffer completes previous seek to reduced duration assert_equals: Event types match. expected "seeking" but got "timeupdate"\n

OUT: FAIL Test seek starts on duration reduction below currentTime assert_equals: Event types match. expected "seeking" but got "timeupdate"\n

OUT: FAIL Test endOfStream completes previous seek to reduced duration assert_equals: Event types match. expected "seeking" but got "timeupdate"\n


It seems slowing down the execution with output to a terminal (greater fails) or file (only 1/100 failed) using logging enabled me to obtain any failures. e.g. --driver-logging --additional-drt-flag="--v=2" --additional-drt-flag="--vmodule=MediaSource*=3,HTMLMediaSource*=3,SourceBuffer*=3,HTMLMediaElement*=3,chunk*=2,source*=2,media*=2,webmed*=2,websou*=2,ffmpeg*=3,decoder_stream*=2,*=0" --additional-drt-flag="--blink-platform-log-channels=Media"' 

But I was unable to repro this locally:
FAIL Test duration reduction below highest buffered presentation time is disallowed assert_equals: mediaElement duration matches fullDuration expected 6.042 but got NaN

cc+chcunningham@ as FYI since some of these flaky failures might have been regressed by or not improved by https://chromium.googlesource.com/chromium/src/+/b92d50681a244d9cf988ea32fcdb6ea7cd096be2
The following flake:

OUT: FAIL Test setting same duration multiple times does not fire duplicate durationchange assert_equals: mediaElement duration matches newDuration expected 6.042 but got 12.084

is because of implementation bug (not test bug).
Spec compliant duration reduction (from 12.084 to 6.042) should occur synchronously with endOfStream() operation. However, our implementation does this reduction of duration on the ChunkDemuxer side, but doesn't report that new duration synchronously back up to blink MediaSource for it to let HTMLME know of new duration. Instead, the updated duration trampolines on the render thread through the DemuxerHost over to WMPI. That races the rest of the test.

Similar trampolining race of duration change can occur for asynch SourceBuffer appendBuffer()ing (setting/increasing duration) versus JS polling of 'updating' state and then doing something synchronously that changes duration (instead of awaiting 'updateend' or similar event which signals updating state change). Luckily, apps tend not to busy-wait, but my upcoming work to move appendBuffer'ing off the main render thread may be an opportune time to ensure that any resulting duration change is stable in pipeline/WMPI/HTMLME *before* signalling completion of the append.
A fix for one of the flaky parts:

FAIL Test setting same duration multiple times does not fire duplicate durationchange assert_equals: mediaElement duration matches newDuration expected 6.042 but got 12.084

is in review at https://chromium-review.googlesource.com/c/606683
Status: Started (was: Assigned)
A fix for two other flaky parts is in review at https://chromium-review.googlesource.com/c/606746

With #11 and #12 applied, the only remaining flakiness I can observe remaining is:
TIMEOUT Test setting same duration multiple times does not fire duplicate durationchange Test timed out

Note also this bug shouldn't really be reused for the flakiness in the similarly named external/wpt version of this test, but since it is already, this bug may remain open for a while after the downstream test's flakiness is fixed or worked-around.
A fix for the timeout flakiness noted in #13 is in review at https://chromium-review.googlesource.com/c/607407

With #11, #12 and https://chromium-review.googlesource.com/c/607407 applied locally, I can no longer repro any flakiness of http/tests/media/media-source/mediasource-duration.html
Project Member

Comment 15 by bugdroid1@chromium.org, Aug 10 2017

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

commit 433dd373be4c680c0dc41b8303275b9fb3c08626
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Thu Aug 10 05:28:31 2017

MSE: Synchronously update element duration on endOfStream()

This change synchronously notifies the MediaElement of potentially
updated duration resulting from MediaSource.endOfStream().  This should
fix the flakiness resulting from thread hopping race of duration change
that occurs in http/tests/media/media-source/mediasource-duration.html:

FAIL Test setting same duration multiple times does not fire duplicate
durationchange assert_equals: mediaElement duration matches newDuration
expected 6.042 but got 12.084

Note, there are other sources of flakiness in that same layout test that
are not fixed by this change.

BUG=689781

Change-Id: Iad9b7375bca0b15a96eb6c2cdd17ba6f26d09281
Reviewed-on: https://chromium-review.googlesource.com/606683
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493271}
[modify] https://crrev.com/433dd373be4c680c0dc41b8303275b9fb3c08626/third_party/WebKit/Source/modules/mediasource/MediaSource.cpp

Project Member

Comment 16 by bugdroid1@chromium.org, Aug 10 2017

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

commit 008f8daf59e8d74ea3bf5a27e7d5d3ae9132c4bd
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Thu Aug 10 17:32:12 2017

MSE: Fix 2 causes of mediasource-duration.html test flakiness

This change makes the test less prone to flakiness for a couple cases:

1) One test case didn't await 'loadedmetadata' from mediaElement (or any
   other mediaElement event indicating it had reached a readyState >=
   HAVE_METADATA) before asserting that the mediaElement had a valid
   duration. Bug 641121 tracks fixing the implementation to ensure
   HAVE_METADATA is reached before 'updateend' is signalled for an
   appendBuffer that triggered the initialization segment received
   algorithm for the last SourceBuffer that previously hadn't received
   an init segment yet. Example flaky failure previously:
      Test duration reduction below highest buffered presentation time
      is disallowed assert_equals: mediaElement duration matches
      fullDuration expected 6.042 but got NaN

2) Multiple test cases used a common utility which (correctly) expected
   a 'timeupdate' event between 'seeking' and 'seeked'. The test
   expectation manager would thus expect 'timeupdate' to occur *only*
   between those two other events; 'timeupdate' could actually also
   occur prior to 'seeking'. This change introduces an extra
   'waitForExpectedEvents' between that utility's 'seeking' and
   'timeupdate' expectations to let the utility ignore any 'timeupdate'
   which might occur prior to 'seeking'.
   Example flaky failure previously:
      FAIL Test seek starts on duration reduction below currentTime
      assert_equals: Event types match. expected "seeking" but got
      "timeupdate"

BUG=689781,641121

Change-Id: Ie2a0f2171afda4cf1196f3eff1944f118dd6b2be
Reviewed-on: https://chromium-review.googlesource.com/606746
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493447}
[modify] https://crrev.com/008f8daf59e8d74ea3bf5a27e7d5d3ae9132c4bd/third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-duration.html

Project Member

Comment 17 by bugdroid1@chromium.org, Aug 10 2017

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

commit 34e69d173cbdd2deca59604a10ed93282f1773c5
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Thu Aug 10 20:51:59 2017

MSE: Fix mediasource-duration.html TIMEOUT flakiness

For the 'Test setting same duration multiple times does not fire
duplicate durationchange' test case, this change:

a) just awaits 'loadedmetadata' instead of awaiting 'playing', to reduce the
   time spent waiting and possible timeout,
b) removes the 2.5 second timeout, and
c) includes some minor cleanup.

BUG=689781

Change-Id: Idde1f8231e8a652ede0a7c4aba771c944b7558b1
Reviewed-on: https://chromium-review.googlesource.com/607407
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493535}
[modify] https://crrev.com/34e69d173cbdd2deca59604a10ed93282f1773c5/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/34e69d173cbdd2deca59604a10ed93282f1773c5/third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-duration.html

Status: Fixed (was: Started)
Status: Assigned (was: Fixed)
Summary: Flaky layout test: mediasource-duration.html (external/wpt version is still flaky) (was: Flaky layout test: http/tests/media/media-source/mediasource-duration.html)
The downstream http/tests/media/media-source/mediasource-duration.html is showing transition to non-flakiness on the dashboard.

As expected though, the external/wpt/ version of similar test is still flaky/failing. 

Per #8/#13, retitling this bug and re-activating it to track fixing the external/wpt/ version....
Labels: FlakyLayoutMediaTest

Comment 21 by zcorpan@gmail.com, May 16 2018

wolenetz, can you please review https://github.com/w3c/web-platform-tests/pull/10989 ?
Project Member

Comment 22 by bugdroid1@chromium.org, Nov 20

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

commit 478a17a5364bb91e3a3e0e54127f163ae4426605
Author: Maxim Kolosovskiy <kolos@chromium.org>
Date: Tue Nov 20 13:20:11 2018

Disable flaky http/tests/media/media-source/mediasource-duration.html on Win

TBR=wolenetz@chromium.org

Bug: 689781
Change-Id: I432995526e5aa0a2e235bf2a8a496ed265cbff74
Reviewed-on: https://chromium-review.googlesource.com/c/1343086
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609679}
[modify] https://crrev.com/478a17a5364bb91e3a3e0e54127f163ae4426605/third_party/WebKit/LayoutTests/TestExpectations

Cc: dalecur...@chromium.org
 Issue 910905  has been merged into this issue.

Sign in to add a comment