Closed Bug 1365205 Opened 3 years ago Closed 3 years ago

Ski Safari WebGL game crashes in android.media.MediaCodec$CodecException: Error 0xfffffff3 at android.media.MediaCodec.releaseOutputBuffer(Native Method)

Categories

(Firefox for Android :: Audio/Video, defect, P1)

ARM
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 56
Tracking Status
firefox54 --- unaffected
firefox55 --- fixed
firefox56 --- fixed

People

(Reporter: jujjyl, Assigned: jhlin)

References

Details

(Keywords: crash, topcrash)

Crash Data

Attachments

(2 files)

This bug was filed from the Socorro interface and is 
report bp-e2ac97e5-bfc5-41b6-9ba0-52f970170515.
=============================================================

Playing Ski Safari WebGL game on Facebook on a Huawei P10 Plus, the browser occassionally crashes with this callstack.

https://www.facebook.com/skisafarigame/
Assignee: nobody → jolin
This is currently the top crash on Android Nightly in the last 7 days.
Keywords: topcrash
:jolin - Do we need to reproduce this, or do you need other information to help move this along? Thanks.
Flags: needinfo?(jolin)
Hi Marcia, could you reproduce this and get the logcat dump? The Java stack trace in the report is from decoder process instead of browser process, and the logcat shows nothing from browser process. My guess is that browser crashed during decoder calling back to it and brought the decoder down with it, but need log from browser for investigation. Thanks.
Flags: needinfo?(jolin)
ni on Ioana to see if we can get someone to reproduce this crash. Currently the top crash on beta as well as nightly.
Flags: needinfo?(ioana.chiorean)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a7a9d1bf542&selectedJob=113570504

Maybe we can reproduce it at try server, dom/media/test/test_buffered.html
Priority: -- → P1
Logcat shows there was a flush right before the crash. It looks like a race condition that buffer callback emitted before flush executed after the flush is completed and all buffers are invalid.

07-16 22:57:27.972 26448 27116 I OMXClient: MuxOMX ctor
07-16 22:57:27.982 26448 27116 I ACodec  : [OMX.google.aac.decoder] Now Loaded
07-16 22:57:27.984 26448 27116 I MediaCodec: MediaCodec will operate in async mode
07-16 22:57:27.992 26448 27116 I ACodec  : [OMX.google.aac.decoder] Now Loaded->Idle
07-16 22:57:28.000 26448 27116 I ACodec  : [OMX.google.aac.decoder] Now Idle->Executing
07-16 22:57:28.002 26448 27116 I ACodec  : [OMX.google.aac.decoder] Now Executing
07-16 22:57:28.009 26448 27116 I ACodec  : [OMX.google.aac.decoder] Now handling output port settings change
07-16 22:57:28.021 26448 27116 I ACodec  : [OMX.google.aac.decoder] Now Executing
07-16 22:57:28.034 26448 27116 I ACodec  : [OMX.google.aac.decoder] signalFlush
07-16 22:57:28.034 26448 27116 I ACodec  : [OMX.google.aac.decoder] ExecutingState flushing now (codec owns 0/4 input, 3/4 output).
07-16 22:57:28.034 26448 27116 I ACodec  : [OMX.google.aac.decoder] Now Flushing
07-16 22:57:28.035 26448 27116 I ACodec  : [OMX.google.aac.decoder] FlushingState onOMXEvent(0,1,0)
07-16 22:57:28.035 26448 27116 I ACodec  : [OMX.google.aac.decoder] FlushingState onOMXEvent(0,1,1)
07-16 22:57:28.035 26448 27116 I ACodec  : [OMX.google.aac.decoder] Now Executing
07-16 22:57:28.036 26448 26448 E MediaCodec: getBufferAndFormat - invalid operation (the index 2 is not owned by client)
07-16 22:57:28.037 26448 26448 W System.err: java.lang.IllegalStateException
07-16 22:57:28.052 26448 26448 W System.err: 	at android.media.MediaCodec.getBuffer(Native Method)
Flags: needinfo?(ioana.chiorean)
Blocks: 1365828
Comment on attachment 8886909 [details]
Bug 1365205 - part 1: check for buffer validness before processing it.

https://reviewboard.mozilla.org/r/157638/#review162864

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/Codec.java:136
(Diff revision 1)
>  
>          }
>  
> +        private boolean isValidBuffer(final int index) {
> +            try {
> +                mCodec.getInputBuffer(index);

Is there no better way to check for validity and should we not check for a null-return?

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/Codec.java:268
(Diff revision 1)
>              }
>          }
>  
> +        private boolean isValidBuffer(final int index) {
> +            try {
> +                mCodec.getOutputBuffer(index);

See previous comment.
Attachment #8886909 - Flags: review?(esawin) → review+
Comment on attachment 8886910 [details]
Bug 1365205 - part 2: execute callback directly when already on the right looper/thread.

https://reviewboard.mozilla.org/r/157640/#review162866
Attachment #8886910 - Flags: review?(esawin) → review+
Comment on attachment 8886909 [details]
Bug 1365205 - part 1: check for buffer validness before processing it.

https://reviewboard.mozilla.org/r/157638/#review162864

> Is there no better way to check for validity and should we not check for a null-return?

Thanks a lot for the review. I'll add nullness check in next version.
Pushed by jolin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/73b2bebfcc18
part 1: check for buffer validness before processing it. r=esawin
https://hg.mozilla.org/integration/autoland/rev/9286e4085ee0
part 2: execute callback directly when already on the right looper/thread. r=esawin
https://hg.mozilla.org/mozilla-central/rev/73b2bebfcc18
https://hg.mozilla.org/mozilla-central/rev/9286e4085ee0
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Please request Beta approval on this when you get a chance.
Flags: needinfo?(jolin)
Comment on attachment 8886909 [details]
Bug 1365205 - part 1: check for buffer validness before processing it.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1361679
[User impact if declined]: On Android L or later devices, sometimes browser crashes when watching video
[Is this code covered by automated tests?]:no
[Has the fix been verified in Nightly?]:yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:none
[Is the change risky?]:low risk
[Why is the change risky/not risky?]:it detects invalid buffers before processing them
[String changes made/needed]:none
Flags: needinfo?(jolin)
Attachment #8886909 - Flags: approval-mozilla-beta?
Comment on attachment 8886909 [details]
Bug 1365205 - part 1: check for buffer validness before processing it.

fix fennec crash, regression in 55, should be in 55.0b12
Attachment #8886909 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Blocks: 1375389
You need to log in before you can comment on or make changes to this bug.