1293613 - Correct MSE web-platform-test

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Jean-Yves Avenard [:jya]]

All the failures remaining are due to bugs in the tests themselves.

This bug aim at fixing those.

Most of them are related to invalid calculations in the buffered range.
Both Blink and Edge are incorrectly using dts to perform operations on the sourcebuffer, when they should be using pts.

We'll fix the tests. So hopefully other parties will fix their implementations quicker.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1293613: don't assume playback won't be finished by the time the seeked event is fired.

The tests incorrectly assume that all videos start at 0. However this is often not the case, in particular for the mp4 files.
The buffered range is the intersection of the audio track and the video track. As such, if the video track starts at a later time than 0, the buffered range of the source buffer can't be starting at 0.

Rather than using different videos, we properly use the correct values; this is done to ensure that buffered range are calculated correctly, regardless of the video content.

timestamps verify with mkvinfo utility for webm and ffprobe for mp4.

See issue https://github.com/w3c/web-platform-tests/issues/1939

Review commit: https://reviewboard.mozilla.org/r/70590/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70590/

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1293613: Update manifest with new test scripts.

Review commit: https://reviewboard.mozilla.org/r/70592/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70592/

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1293613: Correct mediasource-duration.html

- The WebM file manifest correctly indicates the duration as reported in the metadata. However, the last frame has an end time 0f 6.05s which would adjust the duration. As such it is incorrect to assume that after a full appendBuffer the duration will remain the same as set in the metadata.
(source: https://w3c.github.io/media-source/index.html#sourcebuffer-coded-frame-processing "5. If the media segment contains data beyond the current duration, then run the duration change algorithm with new duration set to the maximum of the current duration and the group end timestamp")

- When setting the duration, the actual final duration may be different to what was set as it will be aligned to the highest end time
(source: https://w3c.github.io/media-source/index.html#duration-change-algorithm ("4.1 Update new duration to the highest end time reported by the buffered attribute across all SourceBuffer objects in sourceBuffers.")

Review commit: https://reviewboard.mozilla.org/r/70594/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70594/

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1293613: An invalid or revoked MediaSource URI can't be trace to a MediaSource object.

https://w3c.github.io/media-source/index.html#mediasource-attach: "If the resource fetch algorithm absolute URL matches the MediaSource object URL, ignore any preload attribute of the media element, skip any optional steps for when preload equals none"
A dummy URL or a revoked one no longer matches the MediaSource object URL, as such, the preload attribute isn't to be ignored.

Review commit: https://reviewboard.mozilla.org/r/70596/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70596/

Comment 5

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1293613: Don't assume all segments audio and video track start at the same time point.

Audio track and video tracks have under most cases a different start and end time. Additionally, the mp4 file is poorly muxed and contains out of order frames. As such we keep a table of time and duration for each media segments.

The test itself was right, however it had an invalid time table to start with as it incorrectly used the dts (decode timestamp) instead of the pts (presentation timestamp).

Review commit: https://reviewboard.mozilla.org/r/70598/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70598/

Comment 6

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1293613: Don't assume that all videos start at 0.

See issue https://github.com/w3c/web-platform-tests/issues/1939

Review commit: https://reviewboard.mozilla.org/r/70600/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70600/

Comment 7

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1293613: Make liveseekable test run.

- No html document existed, preventing the test to run.
- Remove test related to the updating attribute being cleared: see https://github.com/w3c/media-source/issues/118

Review commit: https://reviewboard.mozilla.org/r/70602/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70602/

Comment 8

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1293613: Update test using new manifest details.

Do not assume that the element is still seeking when the seeking event is actually fired (the seeking operation is asynchronous)

Review commit: https://reviewboard.mozilla.org/r/70604/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70604/

Comment 9

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1293613: Don't assume that once playing event has fired, the earlier appendBuffer has completed.

The two operations are asynchronous and independent.

Review commit: https://reviewboard.mozilla.org/r/70606/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70606/

Comment 10

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1293613: amend expected MSE web-platform-tests expected results.

They should all run properly now.

Review commit: https://reviewboard.mozilla.org/r/70608/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70608/

Comment 11

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8779628 [details]
Bug 1293613: don't assume playback won't be finished by the time the seeked event is fired.

https://reviewboard.mozilla.org/r/70590/#review67958

r+ with nit:

Typo in commit description: "timestamps verify" -> "Timestamps verified".

Comment 12

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8779629 [details]
Bug 1293613: Update manifest with new test scripts.

https://reviewboard.mozilla.org/r/70592/#review67960

Comment 13

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8779630 [details]
Bug 1293613: Correct mediasource-duration.html

https://reviewboard.mozilla.org/r/70594/#review67962

r+ with nit:

Typo in commit description: "end time 0f" -> "end time of"

Comment 14

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8779631 [details]
Bug 1293613: An invalid or revoked MediaSource URI can't be trace to a MediaSource object.

https://reviewboard.mozilla.org/r/70596/#review67968

r+ with usual typo in commit description: "trace" -> "traced".

Comment 15

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8779632 [details]
Bug 1293613: Don't assume all segments audio and video track start at the same time point.

https://reviewboard.mozilla.org/r/70598/#review67976

r+ with the usual commit description typos:
"all segments audio and video track" -> "all segments' audio and video tracks"
"Audio track and video tracks have" -> "Audio and video tracks have"
"a different start and end time" -> "different start and end times"

Comment 16

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8779633 [details]
Bug 1293613: Don't assume that all videos start at 0.

https://reviewboard.mozilla.org/r/70600/#review67978

Comment 17

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8779634 [details]
Bug 1293613: Make liveseekable test run.

https://reviewboard.mozilla.org/r/70602/#review67980

Comment 18

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8779635 [details]
Bug 1293613: Update test using new manifest details.

https://reviewboard.mozilla.org/r/70604/#review67982

::: testing/web-platform/tests/media-source/mediasource-seek-during-pending-seek.html:105
(Diff revision 1)
>                });
>  
>                test.waitForExpectedEvents(function()
>                {
> -                  assert_true(mediaElement.seeking, 'mediaElement is still seeking');
> -
> +                  // Note that we can't assume that the element is still seeking
> +                  // when the seeking event is fired as the operation is asyncronous.

"asyncronous" -> "asynchronous"

Comment 19

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8779635 [details]
Bug 1293613: Update test using new manifest details.

https://reviewboard.mozilla.org/r/70604/#review67984

Comment 20

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8779636 [details]
Bug 1293613: Don't assume that once playing event has fired, the earlier appendBuffer has completed.

https://reviewboard.mozilla.org/r/70606/#review67986

Comment 21

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8779637 [details]
Bug 1293613: amend expected MSE web-platform-tests expected results.

https://reviewboard.mozilla.org/r/70608/#review67988

Comment 22

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779628 [details]
Bug 1293613: don't assume playback won't be finished by the time the seeked event is fired.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70590/diff/1-2/

Comment 23

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779629 [details]
Bug 1293613: Update manifest with new test scripts.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70592/diff/1-2/

Comment 24

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779630 [details]
Bug 1293613: Correct mediasource-duration.html

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70594/diff/1-2/

Comment 25

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779631 [details]
Bug 1293613: An invalid or revoked MediaSource URI can't be trace to a MediaSource object.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70596/diff/1-2/

Comment 26

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779632 [details]
Bug 1293613: Don't assume all segments audio and video track start at the same time point.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70598/diff/1-2/

Comment 27

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779633 [details]
Bug 1293613: Don't assume that all videos start at 0.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70600/diff/1-2/

Comment 28

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779634 [details]
Bug 1293613: Make liveseekable test run.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70602/diff/1-2/

Comment 29

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779635 [details]
Bug 1293613: Update test using new manifest details.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70604/diff/1-2/

Comment 30

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779636 [details]
Bug 1293613: Don't assume that once playing event has fired, the earlier appendBuffer has completed.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70606/diff/1-2/

Comment 31

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779637 [details]
Bug 1293613: amend expected MSE web-platform-tests expected results.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70608/diff/1-2/

Comment 32

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1293613: Never attempt to read past end of manifest table.

Should the events being waited for take a while to fire, we could have attempted to append more segments than the table contain causing an exception.

Review commit: https://reviewboard.mozilla.org/r/70836/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70836/

Comment 33

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779628 [details]
Bug 1293613: don't assume playback won't be finished by the time the seeked event is fired.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70590/diff/2-3/

Comment 34

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779629 [details]
Bug 1293613: Update manifest with new test scripts.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70592/diff/2-3/

Comment 35

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779630 [details]
Bug 1293613: Correct mediasource-duration.html

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70594/diff/2-3/

Comment 36

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779631 [details]
Bug 1293613: An invalid or revoked MediaSource URI can't be trace to a MediaSource object.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70596/diff/2-3/

Comment 37

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779632 [details]
Bug 1293613: Don't assume all segments audio and video track start at the same time point.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70598/diff/2-3/

Comment 38

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779633 [details]
Bug 1293613: Don't assume that all videos start at 0.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70600/diff/2-3/

Comment 39

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779634 [details]
Bug 1293613: Make liveseekable test run.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70602/diff/2-3/

Comment 40

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779635 [details]
Bug 1293613: Update test using new manifest details.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70604/diff/2-3/

Comment 41

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779636 [details]
Bug 1293613: Don't assume that once playing event has fired, the earlier appendBuffer has completed.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70606/diff/2-3/

Comment 42

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779637 [details]
Bug 1293613: amend expected MSE web-platform-tests expected results.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70608/diff/2-3/

Comment 43

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8779968 [details]
Bug 1293613: Never attempt to read past end of manifest table.

https://reviewboard.mozilla.org/r/70836/#review68224

Comment 44

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/96e477498ea4
Don't assume that all videos start at 0. r=gerald
https://hg.mozilla.org/integration/autoland/rev/2cae41cf5ea5
Update manifest with new test scripts. r=gerald
https://hg.mozilla.org/integration/autoland/rev/0db1ed3d603b
Correct mediasource-duration.html r=gerald
https://hg.mozilla.org/integration/autoland/rev/1ea1ecf7d4b0
An invalid or revoked MediaSource URI can't be trace to a MediaSource object. r=gerald
https://hg.mozilla.org/integration/autoland/rev/bf05ed42e18a
Don't assume all segments audio and video track start at the same time point. r=gerald
https://hg.mozilla.org/integration/autoland/rev/c4a4c5ead421
Don't assume that all videos start at 0. r=gerald
https://hg.mozilla.org/integration/autoland/rev/d5e63c7b5409
Make liveseekable test run. r=gerald
https://hg.mozilla.org/integration/autoland/rev/07b2fa5b4c94
Update test using new manifest details. r=gerald
https://hg.mozilla.org/integration/autoland/rev/bde91881654f
Don't assume that once playing event has fired, the earlier appendBuffer has completed. r=gerald
https://hg.mozilla.org/integration/autoland/rev/9f514918f335
Never attempt to read past end of manifest table. r=gerald
https://hg.mozilla.org/integration/autoland/rev/7462a01bd906
amend expected MSE web-platform-tests expected results. r=gerald

Comment 45

[Jean-Yves Avenard [:jya]]

Comment on attachment 8779628 [details]
Bug 1293613: don't assume playback won't be finished by the time the seeked event is fired.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70590/diff/3-4/

Comment 46

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/31e906fb345e
don't assume playback won't be finished by the time the seeked event is fired. r=gerald

Comment 47

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/96e477498ea4
https://hg.mozilla.org/mozilla-central/rev/2cae41cf5ea5
https://hg.mozilla.org/mozilla-central/rev/0db1ed3d603b
https://hg.mozilla.org/mozilla-central/rev/1ea1ecf7d4b0
https://hg.mozilla.org/mozilla-central/rev/bf05ed42e18a
https://hg.mozilla.org/mozilla-central/rev/c4a4c5ead421
https://hg.mozilla.org/mozilla-central/rev/d5e63c7b5409
https://hg.mozilla.org/mozilla-central/rev/07b2fa5b4c94
https://hg.mozilla.org/mozilla-central/rev/bde91881654f
https://hg.mozilla.org/mozilla-central/rev/9f514918f335
https://hg.mozilla.org/mozilla-central/rev/7462a01bd906
https://hg.mozilla.org/mozilla-central/rev/31e906fb345e