1304820 - Opus in non-fragmented MP4 is not working

Status: RESOLVED FIXED

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

Description

[Vittorio Giovara]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160111180959

Steps to reproduce:

Download https://people.mozilla.org/~rgiles/2016/sample.mp4

Try to open it in Firefox.


Actual results:

The usual "Video cannot be played" appears.


Expected results:

The audio should have started playing.

Comment 1

[Vittorio Giovara]

Note, I don't know if opus is working with MSE or fragmented MP4.

Related bugs https://bugzilla.mozilla.org/show_bug.cgi?id=1240413 https://bugzilla.mozilla.org/show_bug.cgi?id=1267887

Comment 2

[Ralph Giles (:rillian)]

So this is a 'regression' in that the non-fragmented file used to play, even though we were targetting MSE in the initial implementation.

Matthew, is non-fragmented Opus blocked for a good reason, or was this accidental?

Comment 3

[Matthew Gregan [:kinetik]]

It's a regression from bug 1240412.  That added a call into MP4MetadataRust::ReadTrackIndex but only implemented the fragmented case, so now instead of using Stagefright's answer we MOZ_ASSERT(false, "Not yet implemented") and then return false.

Comment 4

[Ralph Giles (:rillian)]

Right, thanks for clarifying.

Comment 5

[Alfredo Yang (:alfredo)]

Attachment: Bug 1304820 - use stagefright instead of rust parser when it fails to parse the file.

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

Comment 6

[Alfredo Yang (:alfredo)]

Attachment: Bug 1304820 - use stagefright instead of rust parser when rust parser fails to parse a file.

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

Comment 7

[Matthew Gregan [:kinetik]]

Comment on attachment 8794068 [details]
Bug 1304820 - use stagefright instead of rust parser when rust parser fails to parse a file.

https://reviewboard.mozilla.org/r/80692/#review79342

Looks fine, but maybe downgrade the NYI assert in the Rust ReadTrackIndex to an NS_WARNING.

Comment 8

[Alfredo Yang (:alfredo)]

Comment on attachment 8794068 [details]
Bug 1304820 - use stagefright instead of rust parser when rust parser fails to parse a file.

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9e67b79f3eea
use stagefright instead of rust parser when rust parser fails to parse a file. r=kinetik

Comment 10

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/9e67b79f3eea

Comment 11

[Fahima Zulfath]

Attachment: testcase.png

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]:
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:
[Why is the change risky/not risky?]:
[String changes made/needed]:

Comment 12

[Fahima Zulfath]

[testday-20170217]The bug is not resolved it seems. Can't play video in Firefox Beta 51.
OPERATING SYSTEM : Windows 10

Comment 13

[Julien Cristau [:jcristau]]

(In reply to fahimazulfath.a from comment #12)
> [testday-20170217]The bug is not resolved it seems. Can't play video in
> Firefox Beta 51.
> OPERATING SYSTEM : Windows 10

Firefox beta is 52, not 51.  What exact version did you test (look at the version and build id fields in about:support)?

Comment 14

[Vuyisile Ndlovu [:terrameijar]]

I can verify that this has been fixed in Nightly 54.0a1. I tested on Ubuntu using Firefox Beta 52.0b3 and Nightly 54.0a1.

https://people-mozilla.org/~rgiles/2016/sample.mp4 plays a song and does not display the "Video can't be played because the file is corrupt." message