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

RESOLVED FIXED in Firefox 66

Status

()

defect
P1
normal
RESOLVED FIXED
6 months ago
5 months ago

People

(Reporter: Villa, Assigned: bryce)

Tracking

({regression})

unspecified
mozilla66
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox64 unaffected, firefox65 unaffected, firefox66 fixed)

Details

()

Attachments

(1 attachment)

Reporter

Description

6 months ago

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

6 months ago
regression-window

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

Blocks: 1487416
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → Audio/Video: Playback
Ever confirmed: true
Flags: needinfo?(bvandyk)
Keywords: regression
OS: Unspecified → All
Product: Firefox → Core
Hardware: Unspecified → All
Assignee

Comment 2

5 months ago

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.

Assignee: nobody → bvandyk
Status: NEW → ASSIGNED
Flags: needinfo?(bvandyk)
Priority: -- → P1
Assignee

Updated

5 months ago
Duplicate of this bug: 1519683
Assignee

Comment 4

5 months ago

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.

Summary: html5-hls video-player not working ZDF-Player 2.7.1-stable → MP4 parser should be tolerant of tracks that use the reserved track_id 0
Assignee

Comment 6

5 months ago
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.
Assignee

Updated

5 months ago
Duplicate of this bug: 1519683

Comment 8

5 months ago
Pushed by bvandyk@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dad40f23f4c1
Update MoofParser to handle tracks using track_id 0. r=jya
Assignee

Updated

5 months ago
See Also: → 1519919

Updated

5 months ago
Duplicate of this bug: 1519620

Comment 10

5 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

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.

Status: RESOLVED → VERIFIED

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.

Flags: needinfo?(bvandyk)
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla66 → ---
Version: 66 Branch → unspecified
Assignee

Comment 15

5 months ago

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.

Flags: needinfo?(bvandyk)
Attachment #9036244 - Attachment description: Bug 1519617 - Update MoofParser to handle tracks using track_id 0. r?alwu,jya → Bug 1519617 - Update MoofParser to handle tracks using track_id 0. r?jya,padenot
Reporter

Comment 17

5 months ago

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.

Assignee

Comment 18

5 months ago

(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.

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

Comment 20

5 months ago
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4c800571b02d
Update MoofParser to handle tracks using track_id 0. r=jya

Comment 21

5 months ago
bugherder
Status: REOPENED → RESOLVED
Closed: 5 months ago5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Reporter

Comment 22

5 months ago

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)

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee

Comment 23

5 months ago

(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.

Reporter

Comment 24

5 months ago

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

Status: REOPENED → RESOLVED
Closed: 5 months ago5 months ago
Resolution: --- → FIXED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.