829408 - Force stagefright on Galaxy S2 to return video frames

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Comment 1

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

Attachment: Possible fix

This patch might be all that's needed to fix most Gingerbread SGS2s. It seems that Samsung at some point updated their libstagefright to add the missing index for enabling `Thumbnail Mode' in the OMX driver.

I think we can try this for now, and if there's still a significant number of devices exhibiting this bug, I'm attaching a (much more hacky) WIP patch that should work for those devices where libstagefright hasn't been patched.

Comment 2

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

Attachment: WIP hacky bad fix

WIP patch that should fix the GB SGS2 bug properly, but introduces more cruft than I'd like to for one device.

Haven't been able to repro, so this patch is untested.

Comment 3

[cajbir (:cajbir)]

Comment on attachment 700846 [details] [diff] [review]
Possible fix

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

Does this change work on the SGS 2 device we have? Or any device you've tested?

::: media/omx-plugin/OmxPlugin.cpp
@@ +307,2 @@
>      // Let Stagefright choose hardware or software decoder.
> +    sp<MediaSource> videoSource = OMXCodec::Create(omx, aVideoTrack->getFormat(),

Why the change from aOmx to omx? Where is 'omx' from?

@@ +352,5 @@
>  #endif
>    }
>  
> +  MOZ_ASSERT(flags != DEFAULT_STAGEFRIGHT_FLAGS);
> +  return OMXCodec::Create(omx, aVideoTrack->getFormat(), false, aVideoTrack,

Same question about 'omx' here.

Comment 4

[cajbir (:cajbir)]

(In reply to Edwin Flores [:eflores] [:edwin] from comment #2)
> Haven't been able to repro, so this patch is untested.

What do you mean by this? Does 'repro' mean 'reproduce?' The SGS 2 we have doesn't show the green video frames?

Comment 5

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

(In reply to Chris Double (:doublec) from comment #4)
> (In reply to Edwin Flores [:eflores] [:edwin] from comment #2)
> > Haven't been able to repro, so this patch is untested.
> 
> What do you mean by this? Does 'repro' mean 'reproduce?' The SGS 2 we have
> doesn't show the green video frames?

Nope. Got to trying to test it but my getExtensionIndex() never gets hit, presumably because of a hack they've added on the libstagefright side.

I have a GB image somewhere which, IIRC, does show the bug. I'll try that.

Comment 6

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

(In reply to Chris Double (:doublec) from comment #3)
> Comment on attachment 700846 [details] [diff] [review]
> Possible fix
> 
> Review of attachment 700846 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Does this change work on the SGS 2 device we have? Or any device you've
> tested?

Works on the office SGS2.

> ::: media/omx-plugin/OmxPlugin.cpp
> @@ +307,2 @@
> >      // Let Stagefright choose hardware or software decoder.
> > +    sp<MediaSource> videoSource = OMXCodec::Create(omx, aVideoTrack->getFormat(),
> 
> Why the change from aOmx to omx? Where is 'omx' from?

Blargh. An artefact from splitting the patch into two.

Comment 7

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

Can't get the green video frames with the other image either. I don't remember whether we tried this in Fennec on GB before or just on B2G...

Comment 8

[cajbir (:cajbir)]

I don't know why but on our SGS 2 testing device we're no longer see the green frames so can't test or reproduce. Closing WORKSFORME. We'll reopen if the issue appears again so we can reproduce and test.

Comment 9

[cajbir (:cajbir)]

The hardware decoder on my SGS 2 still seems to be returning a color format that is not understood. Here's the logcat:

I/OMXCodec( 4054): OMXCodec::Create matchComponentName ((null)), flags (0)
D/OMXCodec( 4054): componentName=OMX.SEC.avcdec, quirks=73728, flags=0
I/OMXCodec( 4054): eColorFormat (19)
I/OmxPlugin( 4054): Unknown video color format: 0x2c
I/OmxPlugin( 4054): Falling back to software decoder

0x2c is the green frame thing isn't it?

Comment 10

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

Well that's no good.

Is my build still on there? If so, can you try it with that and see if the same happens?

Comment 11

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

Comment on attachment 700846 [details] [diff] [review]
Possible fix

Looks like this is what fixed it for me.

Comment 12

[cajbir (:cajbir)]

Comment on attachment 700846 [details] [diff] [review]
Possible fix

Patch is good and seems to work. You'll need to fix up the omx vs aOmx as mentioned in comment 3.

Comment 13

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/e9c76968a95e

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e9c76968a95e