1341228 - [mse][youtube] unable to pass test case 92(AppendMultipleInitOpusAudio)

Status: RESOLVED FIXED

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

Description

[Vincent Chang[:vchang][changyihsin]]

STR,
1. Using the firefox nightly(trunk) built by myself.
2. Turn on media.mediasource.webm.enabled to true
3. click test case 92. 

It's failed in below line when calling SourceBuffer.buffered.end(0) API. 
var end = src.buffered.end(0);


[1]http://yt-dash-mse-test.commondatastorage.googleapis.com/unit-tests/2017.html?&test_type=conformance-test&timestamp=1487670424161

Comment 1

[Jean-Yves Avenard [:jya]]

Please, don't change priority, this is up to the triage team.

Comment 2

[Jean-Yves Avenard [:jya]]

https://w3c.github.io/media-source/webm-byte-stream-format.html#webm-media-segments

"The user agent MUST accept and ignore Cues or Chapters elements that follow a Cluster element."

But here the Cues are placed *prior* the cluster element.

https://w3c.github.io/media-source/webm-byte-stream-format.html#webm-init-segments
states:
"The user agent MUST accept and ignore any elements other than an EBML Header or a Cluster that occur before, in between, or after the Segment Information and Tracks elements."

so I guess we shouldn't error on cues.

The WebM container parser really, was only designed to accept what YouTube typically feeds...

Comment 3

[Alastor Wu [:alwu]]

Attachment: cues.png

I checked this bug today, but I still not did not really understand how to fix this. I did see the last element Id in the init segment is Cues, because we didn't handle that in WebMBufferedParser, so it seems being skipped and not affecting our parsing for the init segment. Then, the error happened in nestegg, see here, because the id is ID_Cues(0x1c53bb6b), not ID_EBML. But by looking closer to the ne_peek_element, it seems reading the last element, which I don't understand why. Because ID_EBML should be the first element, not the last.

Kinetik, would you mind to give me some suggestion for this issue? I would like to help on this issue.
Thank you so much.

Comment 4

[Matthew Gregan [:kinetik]]

Original test URL is dead, but https://ytlr-cert.appspot.com/latest/main.html?&test_type=msecodec-test is a live URL - test is now 11. AppendMultipleInitOpusAudio in the MSE Codec Tests category. Still fails in current Nightly.

Looking at car_opus_low.webm:

+ EBML head      at      0  size      36
+ Segment        at     36  size 1205138
| SeekHead       at     48  size      62
| EBML void      at    110  size      48
| Segment info   at    158  size      37
| Tracks         at    195  size      77   ← parser USED to commit init-end at byte 272 (= 195 + 77)
| Cues           at    272  size     319   ← orphaned by old code, included by fix
| Cluster        at    591  size   60079   ← spec-correct end of init

During the first appendBuffer, WebMBufferedParser was setting mInitEndOffset to the offset where Tracks finished (272). In this case, Cues comes after Tracks and the correct offset for the end of the init segment should be 591. We'd call nestegg_init with mInitData spanning [0, 272), leaving 319 bytes in mInputBuffer to be treated as a media segment.

During the second appendBuffer, mInputBuffer now holds the Cues followed by the valid init segment being appended. This causes a parser reinit, where nestegg_init fails because it begins parsing a buffer beginning with Cues instead of the expected EBML.

The fix is to teach WebMBufferedParser's SKIP_DATA step to skip anything that appears after we've accepted an init segment but before we've found the first cluster.

Comment 5

[Matthew Gregan [:kinetik]]

Attachment: Bug 1341228 - Extend WebM init segment to first Cluster, including Cues

Per the MSE WebM byte-stream spec the initialization segment is a
Segment element containing Segment Information and Tracks elements
"up to (but not including) any Cluster elements". Other top-level
Segment children (Cues, SeekHead, Tags, Chapters, Void) that appear
between Tracks and the first Cluster are part of the initialization
segment.

WebMBufferedParser was committing mInitEndOffset to the byte after
Tracks ended, leaving any subsequent non-Cluster top-level element
classified as media data. When a SourceBuffer received an
initialization segment containing Cues (the layout used by
YouTube TV's AppendMultipleInitOpusAudio conformance test, among
others), the bytes following Tracks remained in the SourceBuffer's
input buffer as orphan data. A second appendBuffer of the same init
segment then misaligned the parse: the buffer began with the orphan
Cues bytes, the next ParseStartAndEndTimestamps mis-identified the
init range, the bytes handed to the WebM demuxer's nestegg_init no
longer started with an EBML header, and TrackBuffersManager rejected
the append with NS_ERROR_DOM_MEDIA_METADATA_ERR.

Extend mInitEndOffset over each top-level non-Cluster element as it
finishes being skipped while we're past Tracks (mInitEndOffset >= 0)
but haven't seen a Cluster yet (mClusterOffset < 0). When the first
Cluster does appear, mInitEndOffset is at the byte before it -- the
spec-correct boundary. For init-segment-only buffers (the MSE case)
the offset advances to the end of the appended data.

Adds a mochitest that mirrors the YouTube TV conformance flow
(append init, append cluster, abort, append init again, verify
buffered.end(0) is unchanged) using a 591-byte init segment carved
from car_opus_low.webm and its first 60KB cluster. The test fails
without the parser change.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/54fc0d4dbb10
https://hg.mozilla.org/integration/autoland/rev/704a5a937cc2
Extend WebM init segment to first Cluster, including Cues r=media-playback-reviewers,aosmond

Comment 7

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/704a5a937cc2