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

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Aaron Train [:aaronmt]]

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)

Comment 3

[cajbir (:cajbir)]

What device is this happening on?

Comment 4

[cajbir (:cajbir)]

(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 :)

Comment 5

[Aaron Train [:aaronmt]]

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/

Comment 6

[Aaron Train [:aaronmt]]

Still reproducible. Samsung Galaxy SIV (Android 4.4, Nightly 11/18) with this attachment: https://bugzilla.mozilla.org/attachment.cgi?id=832851

Comment 8

[cajbir (:cajbir)]

Seems related to bug 934412. See bug 934412 comment 3 and bug 934412 comment 7.

Comment 9

[Hallvord R. M. Steen [:hallvors]]

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.

Comment 10

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

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

Comment 11

[Aaron Train [:aaronmt]]

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}]

Comment 12

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

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

Comment 13

[Aaron Train [:aaronmt]]

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}]

Comment 15

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: 866080.patch

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.

Comment 16

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: 866080-imports.patch

Android imports needed for patch.

Comment 17

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: omxplugin-kitkat.patch

Imports needed for KitKat support.

Comment 18

[cajbir (:cajbir)]

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.

Comment 19

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: 866080.patch

Minor fixes.

Comment 20

[Sotaro Ikeda [:sotaro]]

(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

Comment 21

[Sotaro Ikeda [:sotaro]]

It might be better if we could disable the OmxWrapper just by a preference.

Comment 22

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Comment on attachment 8345042 [details] [diff] [review]
866080.patch

I'll investigate Sotaro's approach. Unsetting r? on this patch for now.

Comment 23

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Comment on attachment 8344938 [details] [diff] [review]
omxplugin-kitkat.patch

...and this...

Comment 24

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Comment on attachment 8344932 [details] [diff] [review]
866080-imports.patch

...and this.

Comment 25

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: 866080-i420.patch

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

Comment 26

[Sotaro Ikeda [:sotaro]]

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.

Comment 27

[cajbir (:cajbir)]

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.

Comment 28

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

(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

Comment 29

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/65adb5a76773

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/65adb5a76773

Comment 31

[Sotaro Ikeda [:sotaro]]

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

Comment 32

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

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.

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/354c393518fb