Force stagefright on Galaxy S2 to return video frames

RESOLVED FIXED in mozilla22

Status

()

Core
Audio/Video
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: eflores, Assigned: eflores)

Tracking

unspecified
mozilla22
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
Created attachment 700846 [details] [diff] [review]
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.
Attachment #700846 - Flags: review?(chris.double)
Created attachment 700848 [details] [diff] [review]
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

5 years ago
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

5 years ago
(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?

Updated

5 years ago
Attachment #700846 - Flags: review?(chris.double) → review-
(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.
(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.
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

5 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WORKSFORME

Comment 9

5 years ago
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?
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
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 on attachment 700846 [details] [diff] [review]
Possible fix

Looks like this is what fixed it for me.
Attachment #700846 - Flags: review- → review?(chris.double)

Comment 12

5 years ago
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.
Attachment #700846 - Flags: review?(chris.double) → review+
Attachment #700848 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9c76968a95e
https://hg.mozilla.org/mozilla-central/rev/e9c76968a95e
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.