Firefox developer edition appears to have a problem with mp4.

VERIFIED FIXED in Firefox 40



4 years ago
4 years ago


(Reporter: martijn, Assigned: jya)



40 Branch
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox39 unaffected, firefox40+ verified, firefox41+ verified)



(3 attachments)



4 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150602004005

Steps to reproduce:

I've tried to play an mp4 video in Firefox Developer Edition (40.0a2) that works in both the regular (consumer) edition of Firefox (38.0.5) and WMP. I've uploaded the file to this location for you to reproduce:

Actual results:

It throws an error instead of playing the video.

The console reads:
Media resource could not be decoded.

The video player itself says:
Video can't be played because the file is corrupt.

Expected results:

I had expected the video to work in the developer edition like it does in regular Firefox.

Comment 1

4 years ago
Regression range:

Probably bug 1156689.
Ever confirmed: true
Flags: needinfo?(jyavenard)
Keywords: regression


4 years ago
Blocks: 1156689

Comment 3

4 years ago
Posted video example.mp4

Comment 4

4 years ago
Via local build:
Last Good: d5282779b0ca
First Bad: 69d8168282fc

Triggered by: 69d8168282fc	Jean-Yves Avenard — Bug 1156689: Part8. Use new MoofParser::HasMetadata in MP4Metadata. r=kentuckyfriedtakahe

Comment 5

4 years ago
Properly read box's 64 bits size... How did that ever worked ?
Attachment #8615076 - Flags: review?(ajones)


4 years ago
Assignee: nobody → jyavenard

Comment 6

4 years ago
looking at the code, it also doesn't properly calculate sizes that are set to 0, which according to ISO/IEC 14496-12:2012: means to the end of the file.

This wouldn't work with this domain however, as the HTTP server doesn't properly report the size of the file
Flags: needinfo?(jyavenard)

Comment 7

4 years ago
Handle box's size marked as 0 as per ISO/IEC 14496-12:2012 as per subclause 4.2
Attachment #8615084 - Flags: review?(ajones)
Tracked for 40 and 41, to insure the video will play as it does in earlier versions.
Comment on attachment 8615076 [details] [diff] [review]
Properly read 64bits header's size

Review of attachment 8615076 [details] [diff] [review]:

::: media/libstagefright/binding/Box.cpp
@@ +70,5 @@
>      MediaByteRange bigLengthRange(headerRange.mEnd,
>                                    headerRange.mEnd + sizeof(bigLength));
>      if ((mParent && !mParent->mRange.Contains(bigLengthRange)) ||
>          !byteRange->Contains(bigLengthRange) ||
> +        !mContext->mSource->CachedReadAt(aOffset+8, bigLength,

Nit: more spaces in aOffset + 8
Attachment #8615076 - Flags: review?(ajones) → review+
Comment on attachment 8615084 [details] [diff] [review]
Part2. Properly hande box size marked as 0

Review of attachment 8615084 [details] [diff] [review]:

Please add a test file and a quick mochitest.
Attachment #8615084 - Flags: review?(ajones) → review+
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41

Comment 13

4 years ago
Comment on attachment 8615076 [details] [diff] [review]
Properly read 64bits header's size

Approval Request Comment
[Feature/regressing bug #]: bug 1045909
[User impact if declined]: Videos can't be played (and regression in Aurora as this code is now more used)
[Describe test coverage new/current, TreeHerder]: In m-c, manual testing.
[Risks and why]: Low. This is implementing as per specification
[String/UUID change made/needed]: None
Attachment #8615076 - Flags: approval-mozilla-beta?
Attachment #8615076 - Flags: approval-mozilla-aurora?
Jean, from comment 1 and comment 4 it sounds like 39 was unaffected by this regression. So we may only need to uplift this to aurora.
Flags: needinfo?(jyavenard)
Comment on attachment 8615076 [details] [diff] [review]
Properly read 64bits header's size

Approved for uplift to aurora.
Attachment #8615076 - Flags: approval-mozilla-beta?
Attachment #8615076 - Flags: approval-mozilla-beta-
Attachment #8615076 - Flags: approval-mozilla-aurora?
Attachment #8615076 - Flags: approval-mozilla-aurora+

Comment 16

4 years ago
(In reply to Liz Henry (:lizzard) from comment #14)
> Jean, from comment 1 and comment 4 it sounds like 39 was unaffected by this
> regression. So we may only need to uplift this to aurora.

The code is still used even in beta but only under some circumstances. It just happens to *always* be used in aurora.

I still think it should be uplifted to beta, as we won't be able to play some videos without it.
Flags: needinfo?(jyavenard)
Flags: qe-verify+
Verified fixed on Firefox 40 Beta 4 (buildID: 20150713153304) and latest Aurora 41.0a2 (buildID: 20150719004007).
You need to log in before you can comment on or make changes to this bug.