Videos with MPEG2 audio not supported

RESOLVED FIXED in Firefox 23

Status

Firefox OS
General
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: diego, Assigned: diego)

Tracking

unspecified
B2G C4 (2jan on)
ARM
Gonk (Firefox OS)
Bug Flags:
in-moztrap +

Firefox Tracking Flags

(blocking-b2g:leo+, firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)

Details

(Whiteboard: [SR 01088871] [TD-8903], URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 721043 [details]
MPEG4 video with MPEG2 audio

OMX hardware decoders do not support this.

I spoke to cjones and he suggested actively rejecting this format before even attempting to decode if possible.

Sample attached
(Assignee)

Comment 1

5 years ago
Created attachment 733384 [details] [diff] [review]
Abort playback if OMX audio init fails

Otherwise the OMX decoder is never released during thumbnail generation. And, for example, a video with an unsupported audio format would block all further OMX playback.
Attachment #733384 - Flags: review?(cpearce)
Attachment #733384 - Flags: review?(chris.double)
Comment on attachment 733384 [details] [diff] [review]
Abort playback if OMX audio init fails

Review of attachment 733384 [details] [diff] [review]:
-----------------------------------------------------------------

Reader changes are sensible, I'm not sure about the OmxDecoder change, doublec would know about that.
Attachment #733384 - Flags: review?(cpearce) → review+

Comment 3

5 years ago
Comment on attachment 733384 [details] [diff] [review]
Abort playback if OMX audio init fails

Review of attachment 733384 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/omx/MediaOmxReader.cpp
@@ +51,5 @@
>    *aTags = nullptr;
>  
>    if (!mOmxDecoder.get()) {
>      mOmxDecoder = new OmxDecoder(mDecoder->GetResource(), mDecoder);
> +    if(!mOmxDecoder->Init()) {

Needs a space between "if" and "(" per style rules.
Attachment #733384 - Flags: review?(chris.double) → review+
(Assignee)

Updated

5 years ago
Assignee: nobody → dwilson
(Assignee)

Comment 4

5 years ago
Created attachment 733587 [details] [diff] [review]
Abort playback if OMX audio init fails v2

Addressed format nit
Carry forward r+
Attachment #733384 - Attachment is obsolete: true
Attachment #733587 - Flags: review+
(Assignee)

Updated

5 years ago
blocking-b2g: --- → leo?
Keywords: checkin-needed
(leo+ as this a downstream request, with a r+ patch in hand that simply improves error handling)
blocking-b2g: leo? → leo+
Whiteboard: [SR 01088871]
status-b2g18: --- → affected
Attachment #733587 - Attachment is patch: true
https://hg.mozilla.org/mozilla-central/rev/ca8b7c325ede
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → B2G C4 (2jan on)
(Assignee)

Comment 8

5 years ago
Can I get help landing on gecko b2g18 ?
(Assignee)

Updated

5 years ago
Flags: needinfo?(ryanvm)
Absolutely. Bugs filed under the Gaia component typically don't get on my radar, so adding a checkin-needed is a good way to get my attention (or needinfo works :)...). Otherwise, I do periodically go through the Gaia tef+ bugs too to look for bugs like these, but it's not an every day thing for me.
Flags: needinfo?(ryanvm)
Keywords: checkin-needed
Component: Gaia::Video → General
https://hg.mozilla.org/releases/mozilla-b2g18/rev/d2d8ae22d88a
status-b2g18: affected → fixed
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
status-firefox21: --- → wontfix
status-firefox22: --- → wontfix
status-firefox23: --- → fixed
Keywords: checkin-needed

Updated

5 years ago
Whiteboard: [SR 01088871] → [SR 01088871] [TD-8903]

Updated

5 years ago
Flags: in-moztrap?

Updated

5 years ago
Flags: in-moztrap? → in-moztrap+
Blocks: 1112506
You need to log in before you can comment on or make changes to this bug.