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

Status: RESOLVED FIXED

(Core :: Audio/Video, defect, P2)

Description

[Chris Peterson [:cpeterson]]

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.

Comment 1

[Chris Peterson [:cpeterson]]

Attachment: 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.

Comment 2

[Chris Peterson [:cpeterson]]

Attachment: part-2-add-software-fallback.patch

Part 2: Use software decoder for color formats we don't know how to convert.

Comment 3

[Chris Peterson [:cpeterson]]

Attachment: part-3-disable-software-fallback.patch

Part 3: Disable software decoder fallback for Nightly, Aurora, and unofficial builds.

Comment 4

[Chris Peterson [:cpeterson]]

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

Comment 5

[cajbir (:cajbir)]

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.

Comment 6

[Chris Peterson [:cpeterson]]

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

Comment 7

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5c1ee495765f
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea218a65affb

Comment 8

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/mozilla-central/rev/5c1ee495765f
https://hg.mozilla.org/mozilla-central/rev/ea218a65affb

Comment 9

[Chris Peterson [:cpeterson]]

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

Comment 10

[Chris Peterson [:cpeterson]]

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.

Comment 11

[Alex Keybl [:akeybl]]

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.

Comment 12

[Alex Keybl [:akeybl]]

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.

Comment 13

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/releases/mozilla-beta/rev/11daec6acf38
https://hg.mozilla.org/releases/mozilla-beta/rev/9269833b9947

Comment 14

[Tracy Walker [:tracy]]

mass remove verifyme requests greater than 4 months old