Closed Bug 866080 Opened 7 years ago Closed 6 years ago

OMX Video Color Conversion fails on Nexus 4/5, SGS 3/4

Categories

(Core :: Audio/Video, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla29
Tracking Status
firefox28 --- fixed
firefox29 --- verified
fennec + ---

People

(Reporter: aaronmt, Assigned: eflores)

References

()

Details

(Keywords: regression, reproducible)

Attachments

(1 file, 4 obsolete files)

I/OMXClient( 5738): Using client-side OMX mux.
I/OMXCodec( 5738): [OMX.qcom.video.decoder.avc] AVC profile = 100 (High), level = 30
E/OMX-VDEC-1080P(  162): 
E/OMX-VDEC-1080P(  162):  No color conversion required
E/OMX-VDEC-1080P(  162): 
E/OMX-VDEC-1080P(  162):  No color conversion required
E/OMX-VDEC-1080P(  162): 
E/OMX-VDEC-1080P(  162):  No color conversion required
E/OMX-VDEC-1080P(  162): 
E/OMX-VDEC-1080P(  162):  No color conversion required
E/OMX-VDEC-1080P(  162): get_config: unknown param 117440527
I/OMXCodec( 5738): [OMX.qcom.video.decoder.avc] video dimensions are 852 x 480
I/OmxPlugin( 5738): Unknown video color format: 0x7fa30c03
I/OmxPlugin( 5738): Falling back to software decoder
E/OMX-VDEC-1080P(  162): 
E/OMX-VDEC-1080P(  162):  Error in ioctl read next msg
E/        (  162): 
E/        (  162):  Destroy C2D instance
E/        (  162): 
E/        (  162):  Destroy C2D instance
E/OMXMaster( 5738): A component of name 'OMX.qcom.audio.decoder.aac' already exists, ignoring this one.
I/OMXCodec( 5738): [OMX.google.h264.decoder] AVC profile = 100 (High), level = 30
I/OMXCodec( 5738): [OMX.google.h264.decoder] video dimensions are 320 x 240
I/OMXCodec( 5738): [OMX.google.h264.decoder] Crop rect is 320 x 240 @ (0, 0)
I/OmxPlugin( 5738): rotation not available, assuming 0
I/OmxPlugin( 5738): width: 320 height: 240 component: OMX.google.h264.decoder format: 0x13 stride: 320 sliceHeight: 240 rotation: 0 crop: 0,0-319,239
I/SoftAAC2( 5738): Reconfiguring decoder: 44100 Hz, 2 channels
I/OmxPlugin( 5738): channelCount: 2 sampleRate: 44100
E/SoftAVC ( 5738): Decoder failed: -2
E/OMXCodec( 5738): [OMX.google.h264.decoder] ERROR(0x80001001, -1007)
I/OmxPlugin( 5738): mVideoSource ERROR 0x80000000
I/OmxPlugin( 5738): mVideoSource ERROR 0x80000000

Simply loading http://people.mozilla.com/~atrain/mobile/tests/big-buck-high.mp4 fails to display any video. Audio continues to work as the stream plays.

--
Nightly (04/26)
LG Nexus 4 (Android 4.2.2)
Duplicate of this bug: 866073
Duplicate of this bug: 866057
What device is this happening on?
(In reply to Chris Double (:doublec) from comment #3)
> What device is this happening on?

Oh, disregard, missed the  bit at the end of the comment. Email has trained me to ignore anything following a "-- " as a signature :)
Assignee: nobody → chris.double
tracking-fennec: ? → +
Status: NEW → ASSIGNED
Re-tested on the same device (LG Nexus 4, Android 4.2.2) Nightly (05/06), same problem. Interestingly, http://cd.pn/b/ works.

Also tested on the Asus Nexus 7 (Android 4.2.2), and wasn't able to reproduce on both the original URL and http://cd.pn/b/
Still reproducible. Samsung Galaxy SIV (Android 4.4, Nightly 11/18) with this attachment: https://bugzilla.mozilla.org/attachment.cgi?id=832851
Assignee: chris.double → edwin
Duplicate of this bug: 937840
Summary: OMX-VDEC: Error in ioctl read next msg - H.264 video fails to display → OMX Video Color Conversion fails on Nexus 4/5, SGS4
Any news on this issue? As we're starting to see some broken sites / videos out there it would be nice to sort this out.
Hi Aaron,

Could you try this build? It's most of the way there on my device, modulo one bug I'm hoping is specific to my model. If it fails, I would appreciate a logcat.

http://people.mozilla.org/~eflores/apk/fennec-28.0a1.en-US.android-arm.apk
Flags: needinfo?(aaron.train)
For whatever reason in the build above, I'm not able to successfully load any video content on my media test-page (http://people.mozilla.org/~atrain/mobile/tests/media.html)

E/GeckoConsole( 6700): [JavaScript Warning: "HTTP "Content-Type" of "video/mp4" is not supported. Load of media resource http://people.mozilla.org/~atrain/mobile/tests/test.mp4 failed." {file: "http://people.mozilla.org/~atrain/mobile/tests/media.html" line: 0}]

E/GeckoConsole( 6700): [JavaScript Warning: "All candidate resources failed to load. Media load paused." {file: "http://people.mozilla.org/~atrain/mobile/tests/media.html" line: 0}]
Flags: needinfo?(aaron.train)
(In reply to Aaron Train [:aaronmt] from comment #11)
> For whatever reason in the build above, I'm not able to successfully load
> any video content on my media test-page
> (http://people.mozilla.org/~atrain/mobile/tests/media.html)

Sorry, these phone models are blacklisted due to this bug. Could you set stagefright.force-enabled = true, restart Fennec and try again?

Thanks!
Flags: needinfo?(aaron.train)
No difference. Flipped the setting, and restarted the browser and am unable to load content

I/MediaPluginHost( 9965): Android Version is: 19
I/MediaPluginHost( 9965): Android Release Version is: 4.4
I/MediaPluginHost( 9965): Android Device is: Nexus 4
I/MediaPluginHost( 9965): Android Manufacturer is: LGE
I/MediaPluginHost( 9965): Android Hardware is: mako
I/MediaPluginHost( 9965): Loading OMX Plugin: libomxplugin.so
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: unhandled flags #8 not handled
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005b50
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005b98
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005be0
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005b54
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005b9c
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005be4
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005b58
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005ba0
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005be8
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005b5c
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005ba4
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005bec
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005b60
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005ba8
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005bf0
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005b64
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005bac
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005bf4
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005b68
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005bb0
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005bf8
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005b6c
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005bb4
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Warning: relocation to NULL @0x00005bfc
E/GeckoLinker( 9965): /data/app/org.mozilla.fennec_me-1.apk!/assets/libomxplugin.so: Error: relocation to NULL @0x00005f90 for symbol "_ZN7android10VectorImpl19reservedVectorImpl1Ev"
E/GeckoConsole( 9965): [JavaScript Warning: "HTTP "Content-Type" of "video/mp4" is not supported. Load of media resource http://people.mozilla.org/~atrain/mobile/tests/test.mp4 failed." {file: "http://people.mozilla.org/~atrain/mobile/tests/media.html" line: 0}]
E/GeckoConsole( 9965): [JavaScript Warning: "All candidate resources failed to load. Media load paused." {file: "http://people.mozilla.org/~atrain/mobile/tests/media.html" line: 0}]
Flags: needinfo?(aaron.train)
Duplicate of this bug: 946900
Summary: OMX Video Color Conversion fails on Nexus 4/5, SGS4 → OMX Video Color Conversion fails on Nexus 4/5, SGS 3/4
Attached patch 866080.patch (obsolete) — Splinter Review
Force the OMX decoder to output frames in a colour format we can support.

This patch has some pretty stupid stuff in it. Since libstagefright doesn't provide a way to set the output colour format, we need to talk to OMX directly. But to make OMX think the call is coming from libstagefright, we need to steal the |node_id| from stagefright's |allocateNode| call to OMX.

So, we need to wrap the OMX object, catch the |node_id| in our |allocateNode|, and use that in a message to OMX to set its colour format.
Attachment #8344929 - Flags: review?(chris.double)
Attached patch 866080-imports.patch (obsolete) — Splinter Review
Android imports needed for patch.
Attachment #8344932 - Flags: review?(chris.double)
Attached patch omxplugin-kitkat.patch (obsolete) — Splinter Review
Imports needed for KitKat support.
Comment on attachment 8344929 [details] [diff] [review]
866080.patch

I'd like feedback on the approach from Sotaro before reviewing. Just in case there's some stagefright API we're not familiar with that would make this easier.
Attachment #8344929 - Flags: feedback?(sotaro.ikeda.g)
Attachment #8344938 - Flags: review?(chris.double)
Attached patch 866080.patch (obsolete) — Splinter Review
Minor fixes.
Attachment #8344929 - Attachment is obsolete: true
Attachment #8344929 - Flags: review?(chris.double)
Attachment #8344929 - Flags: feedback?(sotaro.ikeda.g)
Attachment #8345042 - Flags: review?(chris.double)
Attachment #8345042 - Flags: feedback?(sotaro.ikeda.g)
(In reply to Chris Double (:doublec) from comment #18)
> Comment on attachment 8344929 [details] [diff] [review]
> 866080.patch
> 
> I'd like feedback on the approach from Sotaro before reviewing. Just in case
> there's some stagefright API we're not familiar with that would make this
> easier.

The way seem to work, but very risky. If it is going to be committed to m-c, it might better to confirm some chipsets other than qcom and TI.(like spreadtum). Some chip set modifies almost all internal of OMXCodec.

Android do not provide a way to set out put color format from client side. Since HC/ICS, android provides a color conversion for VideoEditor. It converts the color format to I420. Can't we use it?
http://androidxref.com/4.4_r1/xref/frameworks/native/include/media/editor/II420ColorConverter.h
http://androidxref.com/4.4_r1/xref/frameworks/av/libvideoeditor/lvpp/I420ColorConverter.cpp
It might be better if we could disable the OmxWrapper just by a preference.
Attachment #8345042 - Flags: feedback?(sotaro.ikeda.g)
Comment on attachment 8345042 [details] [diff] [review]
866080.patch

I'll investigate Sotaro's approach. Unsetting r? on this patch for now.
Attachment #8345042 - Flags: review?(chris.double)
Comment on attachment 8344938 [details] [diff] [review]
omxplugin-kitkat.patch

...and this...
Attachment #8344938 - Flags: review?(chris.double)
Comment on attachment 8344932 [details] [diff] [review]
866080-imports.patch

...and this.
Attachment #8344932 - Flags: review?(chris.double)
This patch adds support for I420ColorConverter. Works on my device.

There's a known issue with videos with odd widths, where the U/V planes we get from the color converter have fractional stride. That is, every 2*n+1'th row has stride floor(width/2), but every 2*n'th row has stride ceil(width/2).
Attachment #8344932 - Attachment is obsolete: true
Attachment #8344938 - Attachment is obsolete: true
Attachment #8345042 - Attachment is obsolete: true
Attachment #8346873 - Flags: review?(sotaro.ikeda.g)
Comment on attachment 8346873 [details] [diff] [review]
866080-i420.patch

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

Looks good! Forward review to doublec as a correct reviewer.
Attachment #8346873 - Flags: review?(sotaro.ikeda.g)
Attachment #8346873 - Flags: review?(chris.double)
Attachment #8346873 - Flags: review+
Comment on attachment 8346873 [details] [diff] [review]
866080-i420.patch

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

Have you tested this on other ICS devices to make sure they ship that shared library?

::: media/omx-plugin/OmxPlugin.cpp
@@ +40,5 @@
> +#if !defined(MOZ_ANDROID_V2_X_X) && !defined(MOZ_ANDROID_HC)
> +#define MOZ_ANDROID_V4_OR_ABOVE
> +#endif
> +
> +#ifdef MOZ_ANDROID_V4_OR_ABOVE

Use "#if defined" syntax to be consistent with the rest of the file. Ditto with other places this is done.

@@ +737,5 @@
> +                                                       crop,
> +                                                       buffer);
> +
> +  // result is 0 on success, -1 otherwise.
> +  if (!result) {

Make this "result == OK" so casual readers don't miss this is the success condition. OK == 0 in the Android headers.

@@ +742,5 @@
> +    aFrame->mTimeUs = aTimeUs;
> +    aFrame->mSize = mVideoWidth * mVideoHeight * 3 / 2;
> +  }
> +
> +  return !result;

Make this "return result == OK" too.
Attachment #8346873 - Flags: review?(chris.double) → review+
(In reply to Chris Double (:doublec) from comment #27)
> Have you tested this on other ICS devices to make sure they ship that shared
> library?

Works on all of the ICS devices I have handy. Also green on ICS pandaboards on try:
https://tbpl.mozilla.org/?tree=Try&rev=71aea67c275f
https://hg.mozilla.org/mozilla-central/rev/65adb5a76773
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Status: RESOLVED → VERIFIED
FYI, from android JB, OMXCodec provide a way to set out put color format. It can be set by settinging kKeyColorFormat as OMXCodec::Create()'s argument. I recognize it yesterday.

http://androidxref.com/4.4_r1/xref/frameworks/av/media/libstagefright/StagefrightMetadataRetriever.cpp#151
Depends on: 952315
Depends on: 963621
Comment on attachment 8346873 [details] [diff] [review]
866080-i420.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Android library changes.
User impact if declined: No video playback on some Android 4.x phones.
Testing completed (on m-c, etc.): Has been on m-c for over a month.
Risk to taking this patch (and alternatives if risky): Low.
String or IDL/UUID changes made by this patch: None.
Attachment #8346873 - Flags: approval-mozilla-aurora?
Attachment #8346873 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Duplicate of this bug: 970847
Depends on: 970076
Depends on: 984230
You need to log in before you can comment on or make changes to this bug.