1519617 - MP4 parser should be tolerant of tracks that use the reserved track_id 0

Status: RESOLVED FIXED

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

Description

[Villa]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:66.0) Gecko/20100101 Firefox/66.0

Steps to reproduce:

try to view html5-hls video for example on the website:

https://www.phoenix.de/sendungen/gespraeche/augstein-und-blome/wut-auf-afd-und-gruene-verroht-die-republik-a-627419.html (geo-block may appear it's for germany)

Actual results:

playing the video starts one second and than shows endless spinning - videoplayback not possible

Expected results:

video playback

Comment 1

[Gingerbread Man]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:66.0) Gecko/20100101 Firefox/66.0
20190112094119

Playback was nearly instantaneous for me, until I updated to the current Nightly. Now, after pressing the Play button, there's a long delay, then playback starts but it's only audio.

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e4cbf560928e09c1d7003e5375a5601eabb79c61&tochange=5686d29392e8aecb57c0f75414bedebf54631447

Comment 2

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Thanks for the report and the NI. This looks like us running into a new check I introduced when looking up mp4 metadata: it appears that our lookup of the sample entries is failing because it appears the fragments in the mp4 are referencing sample entries that we don't have a record of. Investigating now.

Comment 4

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Think I've located the cause of this: some sites are serving mp4s that are muxed using the track id 0. This is forbidden by the spec:

track_ID is an integer that uniquely identifies this track over the entire life-time of this presentation. Track IDs are never re-used and cannot be zero.

but it appears to occur in the wild such as on the stream for this bug. Other browsers seem to be tolerant of this, and we have (perhaps inadvertently) been tolerant up until my recent changes.

We've previously reserved the 0 track ID in some mp4 handling code to trigger special handling where a track parser would read metadata from multiple tracks (see here). This had previously (mostly?) worked with streams using the 0 track_id, but I doubt this was intentional.

However, if we're going to tolerate tracks using the reserved track id (zero), we can't use it as a special number in our code -- we should replace it with a bool member or similar. This would also have the advantage of a more descriptive identifier, rather than overloading mTrackId to keep the track id, unless it's 0, in which case read all tracks.

Comment 5

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=80ff072d8d795b17d30fc94f311121b63cc71947

Comment 6

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: Bug 1519617 - Update MoofParser to handle tracks using track_id 0. r?jya,padenot

Using track_id 0 is forbidden by the mp4 spec, however, some sites still serve
media using this track_id. We've been using the 0 track ID to trigger special
handling in the MoofParser where we will parse multiple tracks, and this led us
to be tolerant of tracks using this reserved id (though we likely had some bugs
due to this).

Since sites are using this track_id, and as other browsers (and Firefox until I
broke this) tolerate such media, we should too. In order to do so correctly, we
should no longer us track_id=0 as a special case in the MoofParser, and instead
have an explicit flag, which is what this patch does.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dad40f23f4c1
Update MoofParser to handle tracks using track_id 0. r=jya

Comment 10

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/dad40f23f4c1

Comment 11

[Martin Giger [:freaktechnik]]

This is fixed for me with the latest nightly build (20190114215201), both at the reporter's original URL and behind a paywall where I experienced this.

Comment 12

[Paul Adenot (:padenot)]

This breaks soundcloud, mozregression tells me (and I verified locally).

I don't know why it would, but I'm going to have it backed out.

Comment 13

[Julien Cristau [:jcristau] (back April 22)]

https://hg.mozilla.org/mozilla-central/rev/a559b84032a8

Comment 14

[Cosmin Sabou [:CosminS]]

Backed out from central as requested by Paul: https://hg.mozilla.org/mozilla-central/rev/a559b84032a8d570a64f0bb67ce917c9d696d4fc

Comment 15

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Thanks, Paul. This looks like it's due to the the Moof class not being changed and still treating track_id 0 as a magic number. Fixed patch incoming, and another scenario I will encode into a test for bug 1519919.

Comment 16

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=98005b7ff16a2fac9309e2ac0988375a7b1d7ef3

Comment 17

[Villa]

build 20190115103851 Linux x86_64; rv:66.0
lets sound run in initially linked video - after some seconds loading - but theres still no video visible

soundcloud.com is working for me.

Comment 18

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

(In reply to Villa from comment #17)

build 20190115103851 Linux x86_64; rv:66.0
lets sound run in initially linked video - after some seconds loading - but theres still no video visible

soundcloud.com is working for me.

There were some issues in the first landing of this, so it's been removed and it's being reworked to fix the bug Paul encountered.

Comment 19

[Jim Jeffery not reading bug-mail 1/2/11]

Treeherder build from comment 15 fixed the playback issues for me on foxnews and msnbc, also soundcloud plays.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c800571b02d
Update MoofParser to handle tracks using track_id 0. r=jya

Comment 21

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/4c800571b02d

Comment 22

[Villa]

Linux x86_64; rv:66.0 - 20190116093310
sound plays after long waiting, about 20 seconds, in initially linked video - but theres only no video, just black.
ZDF-Player 2.7.1 stable (html5-hls)

Comment 23

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

(In reply to Villa from comment #22)

Linux x86_64; rv:66.0 - 20190116093310
sound plays after long waiting, about 20 seconds, in initially linked video - but theres only no video, just black.
ZDF-Player 2.7.1 stable (html5-hls)

This has just been merged into mozilla central, but the build containing it has not yet gone out to the nightly channel. It should be in the next build.

Comment 24

[Villa]

nightly build 20190116170105 in wildlife - instand sound + video
thanks a lot! great work