Crash when playing video on Samsung Galaxy S II due to problem in libstagefright

RESOLVED WORKSFORME

Status

()

Core
Audio/Video: Playback
RESOLVED WORKSFORME
3 years ago
2 years ago

People

(Reporter: dncook, Assigned: dncook)

Tracking

(Blocks: 1 bug, {crash})

Trunk
ARM
Android
crash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Created attachment 8447830 [details] [diff] [review]
Workaround for ColorConverter, version 1

This is a follow-up from from 845729. The cause for the crash is an assertion in ColorConverter::convert() from certain builds of libstagefright.so. ColorConverter::isValid() will return true when the source format is QOMX_COLOR_FormatYUV420PackedSemiPlanar64x32Tile2m8ka, (0x7fa30c03) so the code from bug 803394 chooses the hardware codec using that format. When ColorConverter::convert() is called with the same format, it raises an asssertion.

There are four obvious workarounds I can think of.
1. Replace the color conversion routines with new ones, either written from scratch or from ffmpeg or VLC.
2. Skip ColorConverter on problem devices, and either fall back to libI420colorconvert.so or choose a different codec.
3. Blocklist all hardware stagefright codecs on problem devices
4. Change the priority of color conversion routines, check libI420colorconvert before ColorConverter. It looks like libI420colorconvert.so can handle color format 0x7fa30c03, which is good.

I have attached a patch that combines workarounds 2 and 4. Does this sound like a good approach? I did my testing on the T-Mobile US version of the Samsung Galaxy S II, (SGH-T989) running Android 4.0.4. The model strings in the blocklist are from crash reports matching the signatures on bug 845729. The models are variations on the Samsung Galaxy S II, Samsung Galaxy Note, Samsung Galaxy Tab. (and Samsung Galaxy S Blaze, Samsung Exhilirate)
(Assignee)

Updated

3 years ago
Attachment #8447830 - Flags: review?(cajbir.bugzilla)

Comment 1

3 years ago
Comment on attachment 8447830 [details] [diff] [review]
Workaround for ColorConverter, version 1

I think this is more Edwin's domain than mine.
Attachment #8447830 - Flags: review?(cajbir.bugzilla) → review?(edwin)
(Assignee)

Comment 2

3 years ago
Review ping?
Comment on attachment 8447830 [details] [diff] [review]
Workaround for ColorConverter, version 1

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

Sorry for the wait. We had some major-ish colour conversion changes in the pipeline so I was putting this off, but that has stalled for now. LGTM.
Attachment #8447830 - Flags: review?(edwin) → review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
Hi, could you provide a try link for this patch thanks!
Assignee: nobody → divergentdave
Flags: needinfo?(divergentdave)
Keywords: checkin-needed
(Assignee)

Comment 5

3 years ago
I don't think I have access to the tryserver
Flags: needinfo?(divergentdave)
(In reply to dncook from comment #5)
> I don't think I have access to the tryserver

edwin could you help here ?
Flags: needinfo?(edwin)
https://tbpl.mozilla.org/?tree=Try&rev=c61ecda594d6
Flags: needinfo?(edwin)

Comment 8

2 years ago
What's the status of this? Was the patch suppose to land?

Updated

2 years ago
Blocks: 845729
(Assignee)

Comment 9

2 years ago
I'm not sure where this sits, but if needed, I could refresh the patch to deal with bitrot and test it again. What are the next steps?
Edwin, can dncook's Samsung fix land now? Or do you want to finish the other color conversion changes first?
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(edwin)
Keywords: crash
Yes! We should land this if it's still an issue.
Flags: needinfo?(edwin) → needinfo?(divergentdave)
(Assignee)

Comment 12

2 years ago
I recently upgraded my phone from ICS to JB, which fixed the underlying issue, but just before that, the latest Firefox was still triggering the same crash.
Flags: needinfo?(divergentdave)
Component: Audio/Video → Audio/Video: Playback
Upgrade to JB fixes the issue.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.