Last Comment Bug 797225 - Add Stagefright software decoder fallback for hardware decoders that report unknown video color formats
: Add Stagefright software decoder fallback for hardware decoders that report u...
Status: RESOLVED FIXED
: qawanted
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: 17 Branch
: ARM Android
: P2 normal (vote)
: mozilla18
Assigned To: Chris Peterson [:cpeterson]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-02 17:29 PDT by Chris Peterson [:cpeterson]
Modified: 2014-01-10 10:39 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
fixed
fixed


Attachments
part-1-add-OmxPlugin-namespace.patch (2.38 KB, patch)
2012-10-02 17:31 PDT, Chris Peterson [:cpeterson]
cajbir.bugzilla: review+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review
part-2-add-software-fallback.patch (4.92 KB, patch)
2012-10-02 17:33 PDT, Chris Peterson [:cpeterson]
cajbir.bugzilla: review+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review
part-3-disable-software-fallback.patch (2.00 KB, patch)
2012-10-02 17:33 PDT, Chris Peterson [:cpeterson]
cajbir.bugzilla: review-
Details | Diff | Splinter Review

Description Chris Peterson [:cpeterson] 2012-10-02 17:29:18 PDT
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] 2012-10-02 17:31:39 PDT
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.
Comment 2 Chris Peterson [:cpeterson] 2012-10-02 17:33:02 PDT
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.
Comment 3 Chris Peterson [:cpeterson] 2012-10-02 17:33:37 PDT
Created attachment 667269 [details] [diff] [review]
part-3-disable-software-fallback.patch

Part 3: Disable software decoder fallback for Nightly, Aurora, and unofficial builds.
Comment 4 Chris Peterson [:cpeterson] 2012-10-03 14:41:41 PDT
Green try builds:
https://tbpl.mozilla.org/?tree=Try&rev=9bf021a07262
Comment 5 cajbir (:cajbir) 2012-10-07 19:15:06 PDT
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] 2012-10-07 22:06:17 PDT
(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 9 Chris Peterson [:cpeterson] 2012-10-08 15:36:34 PDT
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] 2012-10-08 15:37:44 PDT
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] 2012-10-10 15:54:19 PDT
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] 2012-10-10 15:54:26 PDT
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 14 Tracy Walker [:tracy] 2014-01-10 10:39:38 PST
mass remove verifyme requests greater than 4 months old

Note You need to log in before you can comment on or make changes to this bug.