Add Stagefright software decoder fallback for hardware decoders that report unknown video color formats

RESOLVED FIXED in Firefox 17

Status

()

Core
Audio/Video
P2
normal
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: cpeterson, Assigned: cpeterson)

Tracking

({qawanted})

17 Branch
mozilla18
ARM
Android
qawanted
Points:
---

Firefox Tracking Flags

(firefox16 unaffected, firefox17 fixed, firefox18 fixed)

Details

Attachments

(3 attachments)

(Assignee)

Description

5 years ago
Try to use the device's default decoder (most likely a hardware decoder). If we don't have a ToVideoFrame_*() color conversion function implemented for the video's color format, then fall back to Stagefright's software decoder. This will allow users to watch (non-hardware-optimized) video instead of a black rectangle. 

Disable the fallback on Nightly, Aurora, and unofficial builds so we can identify devices that need color conversion functions to be implemented.
(Assignee)

Comment 1

5 years ago
Created attachment 667265 [details] [diff] [review]
part-1-add-OmxPlugin-namespace.patch

Part 1: Add an OmxPlugin namespace so we can add local definitions of OMX_COLOR enums that are only defined by some toolchains.

Various Android and B2G toolchains do not seem to consistently define that same set of OMX_COLOR vendor extensions:

 * My Android toolchain defines OMX_QCOM_COLOR_FormatYVU420SemiPlanar, but not OMX_QCOM_COLOR_FormatYVU420PackedSemiPlanar32m4ka.
 * Diego Wilson's B2G toolchain defines OMX_QCOM_COLOR_FormatYVU420PackedSemiPlanar32m4ka (bug 793229).
 * cjones' B2G toolchain seems to define neither OMX_QCOM_COLOR_FormatYVU420PackedSemiPlanar32m4ka nor OMX_QCOM_COLOR_FormatYVU420SemiPlanar (bug 792988).

By declaring local (re)definitions in an OmxPlugin namespace, we should be able to compile with any of these toolchains. Since these OMX_COLOR definitions are enum values, we can't test for them with #ifdefs at compile-time.
Attachment #667265 - Flags: review?(chris.double)
(Assignee)

Comment 2

5 years ago
Created attachment 667268 [details] [diff] [review]
part-2-add-software-fallback.patch

Part 2: Use software decoder for color formats we don't know how to convert.
Attachment #667268 - Flags: review?(chris.double)
(Assignee)

Comment 3

5 years ago
Created attachment 667269 [details] [diff] [review]
part-3-disable-software-fallback.patch

Part 3: Disable software decoder fallback for Nightly, Aurora, and unofficial builds.
Attachment #667269 - Flags: review?(chris.double)
(Assignee)

Comment 4

5 years ago
Green try builds:
https://tbpl.mozilla.org/?tree=Try&rev=9bf021a07262

Updated

5 years ago
Attachment #667265 - Flags: review?(chris.double) → review+

Updated

5 years ago
Attachment #667268 - Flags: review?(chris.double) → review+

Comment 5

5 years ago
Comment on attachment 667269 [details] [diff] [review]
part-3-disable-software-fallback.patch

This seems like a bad idea. It would mean the software fallback code gets no widespread testing until past Aurora and makes it hard to track down bugs that occur in the field in that code.
Attachment #667269 - Flags: review?(chris.double) → review-
(Assignee)

Comment 6

5 years ago
(In reply to Chris Double (:doublec) from comment #5)
> Comment on attachment 667269 [details] [diff] [review]
> part-3-disable-software-fallback.patch
> 
> This seems like a bad idea. It would mean the software fallback code gets no
> widespread testing until past Aurora and makes it hard to track down bugs
> that occur in the field in that code.

That's a good point. My thought was that we would want to disable the software fallback in Nightly and Aurora so testers can easily identify devices that need a new color conversion function. With the software fallback, testers with those devices will just see slower video performance. But we (and interested beta testers) can use the "media.stagefright.omxcodec.flags" pref to force hardware decoding for testing device compatibility.
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c1ee495765f
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea218a65affb
status-firefox18: affected → fixed
Target Milestone: --- → mozilla18
https://hg.mozilla.org/mozilla-central/rev/5c1ee495765f
https://hg.mozilla.org/mozilla-central/rev/ea218a65affb
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 9

5 years ago
Comment on attachment 667268 [details] [diff] [review]
part-2-add-software-fallback.patch

I'd like to uplift this fix to Beta 17.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This bug was caused by an unimplemented feature, not a regression.
User impact if declined: Some Android devices will display HTML5 video as a black rectangle because Firefox still needs to implement support for their H.264 hardware decoder.
Testing completed (on m-c, etc.): Tested locally, waiting for m-c
Risk to taking this patch (and alternatives if risky): Medium risk because this patch changes our Android video initialization code, but the alternative is blank video for some devices.
String or UUID changes made by this patch: N/A
Attachment #667268 - Flags: approval-mozilla-beta?
(Assignee)

Comment 10

5 years ago
Comment on attachment 667265 [details] [diff] [review]
part-1-add-OmxPlugin-namespace.patch

[Approval Request Comment]

I'd like to uplift this fix to Beta 17. This patch is small refactoring that the real fix (patch 2) depends on.
Attachment #667265 - Flags: approval-mozilla-beta?

Updated

5 years ago
Keywords: qawanted, verifyme
Comment on attachment 667265 [details] [diff] [review]
part-1-add-OmxPlugin-namespace.patch

If major issues come up in QA's testing, we're confident we'll have time to backout.
Attachment #667265 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 667268 [details] [diff] [review]
part-2-add-software-fallback.patch

If major issues come up in QA's testing, we're confident we'll have time to backout.
Attachment #667268 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/11daec6acf38
https://hg.mozilla.org/releases/mozilla-beta/rev/9269833b9947
status-firefox17: affected → fixed
Summary: Add Stagefright software decoder fallback for Beta and Release channels → Add Stagefright software decoder fallback for hardware decoders that report unknown video color formats
mass remove verifyme requests greater than 4 months old
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.