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)
Comment on attachment 8447830 [details] [diff] [review] Workaround for ColorConverter, version 1 I think this is more Edwin's domain than mine.
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.
Hi, could you provide a try link for this patch thanks!
I don't think I have access to the tryserver
(In reply to dncook from comment #5) > I don't think I have access to the tryserver edwin could you help here ?
What's the status of this? Was the patch suppose to land?
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?
Yes! We should land this if it's still an issue.
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.
Upgrade to JB fixes the issue.