Stagefright: Galaxy Nexus hardware decoder video is blank. Need OMX_TI_COLOR_FormatYUV420PackedSemiPlanar color conversion.

VERIFIED FIXED in Firefox 17

Status

()

P1
normal
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: aaronmt, Assigned: cpeterson)

Tracking

({regression})

Trunk
mozilla18
ARM
Android
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox16 unaffected, firefox17 verified, firefox18 verified, fennec17+)

Details

(URL)

Attachments

(7 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
adb shell am start -a android.intent.aciton.VIEW -n org.mozilla.fennec/.App -d http://people.mozilla.com/~atrain/mobile/tests/test.mp4

I/OMXClient( 2574): Using client-side OMX mux.
I/OMXCodec( 2574): [OMX.TI.DUCATI1.VIDEO.DECODER] AVC profile = 66 (Baseline), level = 30
I/OMXCodec( 2574): [OMX.TI.DUCATI1.VIDEO.DECODER] video dimensions are 320 x 178
I/OMXCodec( 2574): [OMX.TI.DUCATI1.VIDEO.DECODER] Crop rect is 320 x 178 @ (0, 0)

This worked on mozilla-inbound yesterday (08/21), this is a new regression.

--
Samsung Galaxy Nexus (Android 4.1.1)
20120821054201
http://hg.mozilla.org/integration/mozilla-inbound/rev/eb4392d9bc48
Keywords: regressionwindow-wanted
(Assignee)

Comment 1

6 years ago
This bug is likely fallout from bug 782508.
(Assignee)

Comment 2

6 years ago
This bug is definitely fallout from bug 782508. If I force Stagefright to use the software decoder, then the video plays. If I force Stagefright to use the hardware decoder, then video fails to play.

Curiously, the hardware and software decoders disagree on the video's dimensions. The hardware decoder also logs errors about "Unknown video color format: 7f000100". (I am testing a Galaxy Nexus running Jelly Bean.)


* With kHardwareCodecsOnly:

I/OmxPlugin(30096): XXX CreateDecoder: aMimeChars="video/mp4"
I/OmxPlugin(30096): Init: MediaExtractor found videoTrackIndex=0, videoMime="video/avc"
I/OmxPlugin(30096): Init: MediaExtractor found audioTrackIndex=1, audioMime="audio/mp4a-latm"
I/OmxPlugin(30096): Init: video duration=185819152 us
I/OmxPlugin(30096): Init: audio duration=185829297 us
I/OmxPlugin(30096): stride not available, assuming width
I/OmxPlugin(30096): slice height not available, assuming height
I/OmxPlugin(30096): rotation not available, assuming 0
I/OmxPlugin(30096): width: 320 height: 178 component: OMX.TI.DUCATI1.VIDEO.DECODER format: 2130706688 stride: 320 sliceHeight: 178 rotation: 0
I/OmxPlugin(30096): channelCount: 2 sampleRate: 44100
I/OmxPlugin(30096): stride not available, assuming width
I/OmxPlugin(30096): slice height not available, assuming height
I/OmxPlugin(30096): rotation not available, assuming 0
I/OmxPlugin(30096): width: 384 height: 288 component: OMX.TI.DUCATI1.VIDEO.DECODER format: 2130706688 stride: 384 sliceHeight: 288 rotation: 0
I/OmxPlugin(30096): Unknown video color format: 7f000100
I/OmxPlugin(30096): Unknown video color format: 7f000100
I/OmxPlugin(30096): Unknown video color format: 7f000100


* With kSoftwareCodecsOnly:

I/OmxPlugin(29782): XXX CreateDecoder: aMimeChars="video/mp4"
I/OmxPlugin(29782): Init: MediaExtractor found videoTrackIndex=0, videoMime="video/avc"
I/OmxPlugin(29782): Init: MediaExtractor found audioTrackIndex=1, audioMime="audio/mp4a-latm"
I/OmxPlugin(29782): Init: video duration=185819152 us
I/OmxPlugin(29782): Init: audio duration=185829297 us
I/OmxPlugin(29782): stride not available, assuming width
I/OmxPlugin(29782): slice height not available, assuming height
I/OmxPlugin(29782): rotation not available, assuming 0
I/OmxPlugin(29782): width: 320 height: 240 component: OMX.google.h264.decoder format: 19 stride: 320 sliceHeight: 240 rotation: 0
I/OmxPlugin(29782): channelCount: 2 sampleRate: 44100
I/OmxPlugin(29782): stride not available, assuming width
I/OmxPlugin(29782): slice height not available, assuming height
I/OmxPlugin(29782): rotation not available, assuming 0
I/OmxPlugin(29782): width: 320 height: 192 component: OMX.google.h264.decoder format: 19 stride: 320 sliceHeight: 192 rotation: 0
Assignee: nobody → cpeterson
Status: NEW → ASSIGNED
(Assignee)

Comment 3

6 years ago
mediainfo says test.mp4 is 320x178, encoded as H264 Baseline Profile Level 3.0.

Comment 4

6 years ago
We don't support color format OMX_TI_COLOR_FormatYUV420PackedSemiPlanar which is 0x7f000100. This will be why no video is displaying. If we add support for that in OmxCodec::ToVideo it should play. I don't have a device to test unfortunately. cpeterson, do you want to implement the color format and test or would you like me to implement it and you test?
(Assignee)

Comment 5

6 years ago
Created attachment 654494 [details] [diff] [review]
part-1-log-omx-errors.patch

Part 1: Add some OmxPlugin error logging.
Attachment #654494 - Flags: review?(chris.double)
(Assignee)

Comment 6

6 years ago
Created attachment 654495 [details] [diff] [review]
part-2-check-color-format.patch

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

This change is ugly because OMXCodec must initialize the videoSource and parse the video's AVCDecoderConfigurationRecord before we can check the color format. In the normal case, we recognize the color format and continue. If we do not know how to convert the color format, throw away the original videoSource and initialize a new one using Stagefright's software decoder.

Do you know of an easier way to determine the video's color format? Or should we not bother jumping through so many hoops? Currently, if we don't recognize a color format, the screen will be blank but audio will play correctly.
Attachment #654495 - Flags: review?(chris.double)
(Assignee)

Comment 7

6 years ago
Created attachment 654496 [details] [diff] [review]
WIP-part-3-convert-YUV420PackedSemiPlanar.patch

Part 3: Add color conversion for OMX_TI_COLOR_FormatYUV420PackedSemiPlanar.

WIP! This patch does not work because I have not figured out how to extract the U and V planes without crashing. I just wanted to share my work with you, in case you know how to fix it. <:)
Attachment #654496 - Flags: feedback?(chris.double)
(Assignee)

Updated

6 years ago
Priority: -- → P1
tracking-fennec: ? → 16+
(Assignee)

Comment 8

6 years ago
(In reply to Chris Peterson (:cpeterson) from comment #6)
> Do you know of an easier way to determine the video's color format? Or
> should we not bother jumping through so many hoops? Currently, if we don't
> recognize a color format, the screen will be blank but audio will play
> correctly.

doublec: on further thought, silently falling back to the software decoder when we don't recognize the hardware decoder's color format is good for users, but will make our testing more difficult. QA will have a harder time identifying devices that need a new color conversion function to be written (because they will see a slow software-decoded video instead of blank screen).
tracking-fennec: 16+ → ?
Keywords: regressionwindow-wanted
tracking-fennec: ? → 16+

Updated

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

Comment 9

6 years ago
(In reply to Chris Peterson (:cpeterson) from comment #7)
> Created attachment 654496 [details] [diff] [review]
> WIP-part-3-convert-YUV420PackedSemiPlanar.patch
> 
> Part 3: Add color conversion for OMX_TI_COLOR_FormatYUV420PackedSemiPlanar.
> 
> WIP! This patch does not work because I have not figured out how to extract
> the U and V planes without crashing. I just wanted to share my work with
> you, in case you know how to fix it. <:)

Have a look in the Android source at the file ./frameworks/base/media/libstagefright/colorconversion/SoftwareRenderer.cpp in the 'render' function. There's code there that does alignment adjustments on the stride, etc that may be needed.

Updated

6 years ago
Attachment #654495 - Flags: review?(chris.double) → review+
(Assignee)

Updated

6 years ago
Attachment #654496 - Flags: feedback?(chris.double)
No longer blocks: 759945

Updated

6 years ago
Whiteboard: [leave open] → [leave open], [swdecoder]
(Assignee)

Updated

6 years ago
Summary: No video playback with OMX.TI.DUCATI1.VIDEO.DECODER via MP4 in Android → Stagefright: Galaxy Nexus hardware decoder video is blank. Need OMX_TI_COLOR_FormatYUV420PackedSemiPlanar color conversion.
(Assignee)

Updated

6 years ago
Duplicate of this bug: 790950
(Assignee)

Updated

6 years ago
status-firefox18: --- → affected
Whiteboard: [leave open], [swdecoder] → [leave open],
(Assignee)

Comment 13

6 years ago
Created attachment 661893 [details] [diff] [review]
part-3-local-MetaData-pointer.patch

Part 3: Simplify SetVideoFormat() by using local MetaData pointer.
Attachment #654496 - Attachment is obsolete: true
Attachment #661893 - Flags: review?(chris.double)
(Assignee)

Comment 14

6 years ago
Created attachment 661895 [details] [diff] [review]
part-4-rename-conversion-functions.patch

Part 4: Rename color conversion functions to use the standard OMX_COLOR enum names.
Attachment #661895 - Flags: review?(chris.double)
(Assignee)

Comment 15

6 years ago
Created attachment 661896 [details] [diff] [review]
part-5-fix-stride-and-slice-height.patch

Part 5: Stagefright's kKeyWidth and kKeyHeight are actually the stride and slice height.
Attachment #661896 - Flags: review?(chris.double)
(Assignee)

Comment 16

6 years ago
Created attachment 661898 [details] [diff] [review]
part-6-convert-YUV420PackedSemiPlanar.patch

Part 6: Add color conversion function for OMX_TI_COLOR_FormatYUV420PackedSemiPlanar.
Attachment #661898 - Flags: review?(chris.double)
(Assignee)

Comment 17

6 years ago
Created attachment 661899 [details] [diff] [review]
part-7-sanity-check-metadata.patch

Part 7: Add sanity checking of video format metadata.

Do you think these error checks are too strict? We may be able to assume reasonable defaults for some unexpected values (e.g. substituting 0,0 for a negative crop rect positions), but we're probably just asking for trouble by trying to play a video with bad metadata.
Attachment #661899 - Flags: review?(chris.double)
(Assignee)

Updated

6 years ago
Duplicate of this bug: 791500

Updated

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

Updated

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

Updated

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

Updated

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

Updated

6 years ago
Attachment #661899 - Flags: review?(chris.double) → review+
(Assignee)

Comment 20

6 years ago
Triage, we probably don't need to track this for fennec=16+ because we won't ship Stagefright video until 17.
tracking-fennec: 16+ → ?
Going to set tracking 17+ on the assumption that this is on track for 17ish.
tracking-fennec: ? → 17+
(Assignee)

Comment 23

6 years ago
Comment on attachment 661893 [details] [diff] [review]
part-3-local-MetaData-pointer.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 782508
User impact if declined: Galaxy Nexus users will see a blank screen when using H264 hardware decoder (the default as of bug 782508).
Testing completed (on m-c, etc.): This change has been on m-c for 3 days without any major fallout (other than a fixed build break)
Risk to taking this patch (and alternatives if risky): Medium risk because this patch changes H264 code paths used on all Android and B2G devices.
String or UUID changes made by this patch: N/A
Attachment #661893 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 24

6 years ago
Comment on attachment 661895 [details] [diff] [review]
part-4-rename-conversion-functions.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 782508
User impact if declined: Galaxy Nexus users will see a blank screen when using H264 hardware decoder (the default as of bug 782508).
Testing completed (on m-c, etc.): This change has been on m-c for 3 days without any major fallout (other than a fixed build break)
Risk to taking this patch (and alternatives if risky): Medium risk because this patch changes H264 code paths used on all Android and B2G devices.
String or UUID changes made by this patch: N/A
Attachment #661895 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 25

6 years ago
Comment on attachment 661896 [details] [diff] [review]
part-5-fix-stride-and-slice-height.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 782508
User impact if declined: Galaxy Nexus users will see a blank screen when using H264 hardware decoder (the default as of bug 782508).
Testing completed (on m-c, etc.): This change has been on m-c for 3 days without any major fallout (other than a fixed build break)
Risk to taking this patch (and alternatives if risky): Medium risk because this patch changes H264 code paths used on all Android and B2G devices.
String or UUID changes made by this patch: N/A
Attachment #661896 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 26

6 years ago
Comment on attachment 661898 [details] [diff] [review]
part-6-convert-YUV420PackedSemiPlanar.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 782508
User impact if declined: Galaxy Nexus users will see a blank screen when using H264 hardware decoder (the default as of bug 782508).
Testing completed (on m-c, etc.): This change has been on m-c for 3 days without any major fallout (other than a fixed build break)
Risk to taking this patch (and alternatives if risky): Medium risk because this patch changes H264 code paths used on all Android and B2G devices.
String or UUID changes made by this patch: N/A
Attachment #661898 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 27

6 years ago
Comment on attachment 661899 [details] [diff] [review]
part-7-sanity-check-metadata.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 782508
User impact if declined: Galaxy Nexus users will see a blank screen when using H264 hardware decoder (the default as of bug 782508).
Testing completed (on m-c, etc.): This change has been on m-c for 3 days without any major fallout (other than a fixed build break)
Risk to taking this patch (and alternatives if risky): Medium risk because this patch changes H264 code paths used on all Android and B2G devices.
String or UUID changes made by this patch: N/A
Attachment #661899 - Flags: approval-mozilla-aurora?
Sounds like we should get some broader testing to ensure this isn't going to surprise us with unexpected behaviour on non-Nexus devices.  Adding qawanted, please test this with several top devices to see if other models exhibit any fallout or strange behaviours.
Keywords: qawanted, verifyme
QA Contact: jsmith
Setting Jason Smith to the QA contact so we can also test this on B2G devices.
On the B2G side, I tried out Aaron's test case with the video app. The test case worked with no issues (mp4 video played correctly, no garbleness, etc) on the 9/28 build. There *has* been a regression though with mp3 files recently though (they don't play on the FF OS device right now), although I'm not sure its related to this patch or not (https://github.com/mozilla-b2g/gaia/issues/5466). 

We probably need to figure out if this patch causes the mp3 regression or not.
(Reporter)

Comment 31

6 years ago
Tested via Nexus 7 and can confirm the fix attached does now work on the aforementioned test-case.

On the Galaxy Note, Asus Transformer TF201, and Samsung Galaxy SII the video is now completely green.

Please post a fix for these devices.
status-firefox18: fixed → affected
Keywords: qawanted, verifyme

Updated

6 years ago
QA Contact: jsmith → nobody

Comment 32

6 years ago
(In reply to Aaron Train [:aaronmt] from comment #31)> 
> On the Galaxy Note, Asus Transformer TF201, and Samsung Galaxy SII the video
> is now completely green.
> 
> Please post a fix for these devices.

This is a different bug. Please raise one for that issue. Sounds a lot like bug 786103 though.
Cpeterson - can you confirm if your patch is causing the issues Jason mentions in comment 30?  We'll need to know this before we can approve for Aurora landing. If it's a separate bug, let's get that filed.
(Assignee)

Comment 34

6 years ago
(In reply to Aaron Train [:aaronmt] from comment #31)
> Tested via Nexus 7 and can confirm the fix attached does now work on the
> aforementioned test-case.
> 
> On the Galaxy Note, Asus Transformer TF201, and Samsung Galaxy SII the video
> is now completely green.
> 
> Please post a fix for these devices.

aaronmt, this fix is specifically for the Galaxy Nexus. The Nexus 7, Galaxy Note, and Galaxy S II have different chipsets. If those devices have green video, please open a new bug report for each device.
(Assignee)

Comment 35

6 years ago
(In reply to Lukas Blakk [:lsblakk] from comment #33)
> Cpeterson - can you confirm if your patch is causing the issues Jason
> mentions in comment 30?  We'll need to know this before we can approve for
> Aurora landing. If it's a separate bug, let's get that filed.

lsblakk, I don't have a B2G device to test, but my patch landed in build 09-21 (comment 22) and the B2G mp3 bug was first reported on 9/28 (bug 795623).
(Assignee)

Comment 36

6 years ago
(In reply to Aaron Train [:aaronmt] from comment #31)
> On the Galaxy Note, Asus Transformer TF201, and Samsung Galaxy SII the video
> is now completely green.

aaronmt, did these devices work before my fix in the Nightly 9/21 build?

lsblakk, I'd like to uplift this fix to Aurora 17. AFAICT, my fix did not cause the B2G mp3 bug.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
status-firefox18: affected → fixed
Resolution: --- → FIXED
Whiteboard: [leave open],
(Reporter)

Comment 37

6 years ago
Missed the last couple comments here, Chris, I'll file a bug and take a look regarding your comments tomorrow.

As for this bug, this is verified on the GN.
Status: RESOLVED → VERIFIED
status-firefox18: fixed → verified
Comment on attachment 661893 [details] [diff] [review]
part-3-local-MetaData-pointer.patch

Thanks for clarifying, Aaron - and for filing bug 797364 to track the green video issue on the other devices. Approving this one for Aurora.
Attachment #661893 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #661895 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #661896 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #661898 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #661899 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Reporter)

Updated

6 years ago
status-firefox17: fixed → verified
You need to log in before you can comment on or make changes to this bug.