1207214 - Vine site crash at GonkVideoDecoderManager

Status: RESOLVED DUPLICATE of bug 1210045

(Core :: Audio/Video: Playback, defect, P1)

Description

[Sotaro Ikeda [:sotaro]]

+++ This bug was initially created as a clone of Bug #1207198 +++

After applying a fix of bug 1207198, crash in difference place happened on flame-kk with the following STR

STR
[1] Start browser app
[2] Move to https://vine.co/tags/firefoxos
[3] Scroll the site up and down until crash happens.

Comment 1

[Sotaro Ikeda [:sotaro]]

[Blocking Requested - why for this release]: Crash happens easily on the link. And vine is popular service.

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: stack of the crash

Comment 3

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #2)
> Created attachment 8664289 [details]
> stack of the crash

Crash happened because GonkVideoDecoderManager::Input() is called when mDecoder(MediaCodecProxy) is nullptr.

Comment 4

[Bobby Chien]

ni Blake. could you help on this? Thanks.

Comment 5

[Blake Wu [:bwu][:blakewu]]

(In reply to Bobby Chien [:bchien] from comment #4)
> ni Blake. could you help on this? Thanks.
Sure. I will check it.

Comment 6

[Blake Wu [:bwu][:blakewu]]

I can see the problem on my Z3C, although the back trace is different. Maybe there is two rootcauses one is comment 3 and the other is this one which is similar to bug 1204622 comment 15. 

#0  strlen () at bionic/libc/arch-arm/cortex-a15/bionic/strlen.S:75
#1  0xb6e74692 in strlen (s=0xe5e5e5e5 <Address 0xe5e5e5e5 out of bounds>) at bionic/libc/include/string.h:217
#2  __vfprintf (fp=fp@entry=0xae5ff784, fmt0=0xb5bbae55 "Configure video mime type: %s, width:%d, height:%d", fmt0@entry=0xaca8d400 "\005", ap=...) at bionic/libc/stdio/vfprintf.c:625
#3  0xb6e75990 in vsnprintf (str=<optimized out>, str@entry=0xae5ff7d4 "\003", n=n@entry=1024, fmt=fmt@entry=0xaca8d400 "\005", ap=..., ap@entry=...) at bionic/libc/stdio/vsnprintf.c:61
#4  0xb6ea940e in vsnprintf (ap=..., format=0xaca8d400 "\005", size=1024, dest=0xae5ff7d4 "\003") at bionic/libc/include/stdio.h:461
#5  __android_log_print (prio=3, tag=0xb5bbaabf "GonkVideoDecoderManager", fmt=0xb5bbae55 "Configure video mime type: %s, width:%d, height:%d") at system/core/liblog/logd_write.c:220
#6  0xb4d7b05a in mozilla::GonkVideoDecoderManager::codecReserved (this=0xaca8d400) at /Volumes/firefoxos/z3c/B2G/gecko/dom/media/platforms/gonk/GonkVideoDecoderManager.cpp:486
#7  0xb439cb48 in apply<nsMemoryReporterManager, nsresult (nsMemoryReporterManager::*)()> (
    m=(nsresult (nsMemoryReporterManager::*)(nsMemoryReporterManager * const)) 0xb4d7b011 <mozilla::GonkVideoDecoderManager::codecReserved()>, o=<optimized out>, this=<optimized out>)
    at ../../dist/include/nsThreadUtils.h:661
#8  nsRunnableMethodImpl<nsresult (nsMemoryReporterManager::*)(), true>::Run (this=<optimized out>) at ../../dist/include/nsThreadUtils.h:868
#9  0xb43bf3e0 in mozilla::TaskQueue::Runner::Run (this=0xacf7ba90) at /Volumes/firefoxos/z3c/B2G/gecko/xpcom/threads/TaskQueue.cpp:170
#10 0xb43bcf02 in nsThreadPool::Run (this=0xacf4de20) at /Volumes/firefoxos/z3c/B2G/gecko/xpcom/threads/nsThreadPool.cpp:225
#11 0xb43bcaf6 in ProcessNextEvent (aResult=0xae5ffd1f, aMayWait=<optimized out>, this=0xacbe1550) at /Volumes/firefoxos/z3c/B2G/gecko/xpcom/threads/nsThread.cpp:950
#12 nsThread::ProcessNextEvent (this=0xacbe1550, aMayWait=<optimized out>, aResult=0xae5ffd1f) at /Volumes/firefoxos/z3c/B2G/gecko/xpcom/threads/nsThread.cpp:838
#13 0xb43ce29a in NS_ProcessNextEvent (aThread=<optimized out>, aMayWait=aMayWait@entry=false) at /Volumes/firefoxos/z3c/B2G/gecko/xpcom/glue/nsThreadUtils.cpp:277
#14 0xb4520414 in mozilla::ipc::MessagePumpForNonMainThreads::Run (this=0xac8ab310, aDelegate=0xaec233c0) at /Volumes/firefoxos/z3c/B2G/gecko/ipc/glue/MessagePump.cpp:326
#15 0xb4511330 in MessageLoop::RunInternal (this=this@entry=0xaec233c0) at /Volumes/firefoxos/z3c/B2G/gecko/ipc/chromium/src/base/message_loop.cc:234
#16 0xb45113e4 in RunHandler (this=0xaec233c0) at /Volumes/firefoxos/z3c/B2G/gecko/ipc/chromium/src/base/message_loop.cc:227
#17 MessageLoop::Run (this=this@entry=0xaec233c0) at /Volumes/firefoxos/z3c/B2G/gecko/ipc/chromium/src/base/message_loop.cc:201
#18 0xb43bd34a in nsThread::ThreadFunc (aArg=0xacbe1550) at /Volumes/firefoxos/z3c/B2G/gecko/xpcom/threads/nsThread.cpp:379
#19 0xb6de891a in _pt_root (arg=0xac948980) at ../../../../../gecko/nsprpub/pr/src/pthreads/ptthread.c:212
#20 0xb6e5a22c in __thread_entry (func=0xb6de8881 <_pt_root>, arg=0xac948980, tls=0xae5ffdd0) at bionic/libc/bionic/pthread_create.cpp:105
#21 0xb6e5a3c4 in pthread_create (thread_out=0xa9393b24, attr=<optimized out>, start_routine=0xb6de8881 <_pt_root>, arg=0x78) at bionic/libc/bionic/pthread_create.cpp:224
#22 0x00000000 in ?? ()

Comment 7

[Blake Wu [:bwu][:blakewu]]

After several repros, I always hit the crash with the same bt described in comment 6, so set the bug 1204622 in the "Depends on" list. Will check this bug again after bug 1204622 is fixed.

Comment 8

[Blake Wu [:bwu][:blakewu]]

With the latest codes, I still can see the crash, although it doesn't happen very often. The back trace of current crash (shown below) is different from 6 as well. It takes me around ~5 mins to repro. 

#0  0xb468df2e in ChainTo (this=<optimized out>, aChainedPromise=..., aCallSite=<optimized out>) at ../../dist/include/mozilla/MozPromise.h:605
#1  mozilla::MozPromise<nsRefPtr<mozilla::MetadataHolder>, mozilla::ReadMetadataFailureReason, true>::ChainTo (this=<optimized out>, aChainedPromise=..., aCallSite=0xb55248fa "<Proxy Promise>")
    at ../../dist/include/mozilla/MozPromise.h:602
#2  0xb468e020 in mozilla::detail::ProxyRunnable<mozilla::MozPromise<nsRefPtr<mozilla::MetadataHolder>, mozilla::ReadMetadataFailureReason, true>, mozilla::MediaDecoderReader>::Run (this=0xa41dbc60)
    at ../../dist/include/mozilla/MozPromise.h:919
#3  0xb3cf2d2c in mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run (this=0xacd27fa0) at ../../dist/include/mozilla/TaskDispatcher.h:180
#4  0xb3cf8a44 in mozilla::TaskQueue::Runner::Run (this=0xacf4ab90) at /Volumes/firefoxos/z3c/B2G/gecko/xpcom/threads/TaskQueue.cpp:170
#5  0xb3cf655a in nsThreadPool::Run (this=0xacd7a700) at /Volumes/firefoxos/z3c/B2G/gecko/xpcom/threads/nsThreadPool.cpp:227
#6  0xb3cf6148 in ProcessNextEvent (aResult=0xaaa86d1f, aMayWait=<optimized out>, this=0xacd007e0) at /Volumes/firefoxos/z3c/B2G/gecko/xpcom/threads/nsThread.cpp:960
#7  nsThread::ProcessNextEvent (this=0xacd007e0, aMayWait=<optimized out>, aResult=0xaaa86d1f) at /Volumes/firefoxos/z3c/B2G/gecko/xpcom/threads/nsThread.cpp:845
#8  0xb3d0785a in NS_ProcessNextEvent (aThread=<optimized out>, aMayWait=aMayWait@entry=false) at /Volumes/firefoxos/z3c/B2G/gecko/xpcom/glue/nsThreadUtils.cpp:277
#9  0xb3e5afac in mozilla::ipc::MessagePumpForNonMainThreads::Run (this=0xa59dbca0, aDelegate=0xa47463a0) at /Volumes/firefoxos/z3c/B2G/gecko/ipc/glue/MessagePump.cpp:326
#10 0xb3e48930 in MessageLoop::RunInternal (this=this@entry=0xa47463a0) at /Volumes/firefoxos/z3c/B2G/gecko/ipc/chromium/src/base/message_loop.cc:234
#11 0xb3e489e4 in RunHandler (this=0xa47463a0) at /Volumes/firefoxos/z3c/B2G/gecko/ipc/chromium/src/base/message_loop.cc:227
#12 MessageLoop::Run (this=this@entry=0xa47463a0) at /Volumes/firefoxos/z3c/B2G/gecko/ipc/chromium/src/base/message_loop.cc:201
#13 0xb3cf699e in nsThread::ThreadFunc (aArg=0xacd007e0) at /Volumes/firefoxos/z3c/B2G/gecko/xpcom/threads/nsThread.cpp:382
#14 0xb676a91a in _pt_root (arg=0xaf31ee00) at ../../../../../gecko/nsprpub/pr/src/pthreads/ptthread.c:212
#15 0xb6e3122c in __thread_entry (func=0xb676a881 <_pt_root>, arg=0xaf31ee00, tls=0xaaa86dd0) at bionic/libc/bionic/pthread_create.cpp:105

Comment 9

[JW Wang [:jwwang] [:jw_wang]]

Per discussion with Blake, I will take a look.

Comment 10

[JW Wang [:jwwang] [:jw_wang]]

I can repro it on flame-kk.
F/MOZ_Assert( 9140): Assertion failure: mWaitOutput.IsEmpty(), at /media/jwwang/DATA/codebase/mozilla-central-b2g/dom/media/platforms/gonk/GonkMediaDataDecoder.cpp:208

I will leave it to Alfredo before I can fix the promise crash in comment 8.

Comment 11

[Alfredo Yang (:alfredo)]

(In reply to JW Wang [:jwwang] from comment #10)
> I can repro it on flame-kk.
> F/MOZ_Assert( 9140): Assertion failure: mWaitOutput.IsEmpty(), at
> /media/jwwang/DATA/codebase/mozilla-central-b2g/dom/media/platforms/gonk/
> GonkMediaDataDecoder.cpp:208
> 
> I will leave it to Alfredo before I can fix the promise crash in comment 8.

I think it doesn't need after bug 1198664.

Comment 12

[Alfredo Yang (:alfredo)]

Attachment: release_codec_at_reader_thread

Comment 13

[Alfredo Yang (:alfredo)]

Since bug 1198664 is landed, I think it is not required to assert here. What do you think?

https://dxr.mozilla.org/mozilla-central/rev/4f4615ffec6a6a7ec40ff61ffda90a46c53f8d31/dom/media/platforms/gonk/GonkMediaDataDecoder.cpp#208

Comment 14

[Alfredo Yang (:alfredo)]

Although attached patch can solve it, but I think the better way is to solve bug 1214997. That will simplify the the callback complexity in Gonk when allocate/release hardware codec.

Comment 15

[John Lin [:jhlin][:jolin]]

(In reply to Alfredo Yang (:alfredo) from comment #13)
> Since bug 1198664 is landed, I think it is not required to assert here. What
> do you think?
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> 4f4615ffec6a6a7ec40ff61ffda90a46c53f8d31/dom/media/platforms/gonk/
> GonkMediaDataDecoder.cpp#208

 Looks like I missed a condition in the assertion.
 Sometimes the decoder attaches EOS flag to the final output buffer instead of emits EOS by itself. Since an item was put in mWaitOutput even for EOS input sample [1], it can be still left in the array when that happens. Will upload a later for this.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/media/platforms/gonk/GonkMediaDataDecoder.cpp#87

Comment 16

[John Lin [:jhlin][:jolin]]

Attachment: Assert output buffer with EOS flag.

Assert that decoder attaches EOS flag to final output buffer.

Comment 17

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8674677 [details] [diff] [review]
Assert output buffer with EOS flag.

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

Not familiar with GonkMediaDataDecoder. Delegate to Sotaro.

Comment 18

[Alfredo Yang (:alfredo)]

Bug 1210045 also solves this problem.

Comment 19

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8674677 [details] [diff] [review]
Assert output buffer with EOS flag.

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

The patch seems not related to the crash in this bug. But the patch seems to address EOS problem. As in ACodec::BaseState::onOMXFillBufferDone(), ACodec permit valid buffer + EOS flag. It might better to be handled as a different bug.

http://androidxref.com/5.1.1_r6/xref/frameworks/av/media/libstagefright/ACodec.cpp#4601

Comment 20

[John Lin [:jhlin][:jolin]]

(In reply to Sotaro Ikeda [:sotaro] from comment #19)
> Comment on attachment 8674677 [details] [diff] [review]
> Assert output buffer with EOS flag.
> 
> Review of attachment 8674677 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The patch seems not related to the crash in this bug. But the patch seems to
> address EOS problem. As in ACodec::BaseState::onOMXFillBufferDone(), ACodec
> permit valid buffer + EOS flag. It might better to be handled as a different
> bug.
> 
> http://androidxref.com/5.1.1_r6/xref/frameworks/av/media/libstagefright/
> ACodec.cpp#4601

Good point. Will file a new bug and move the patch there. Thanks a lot.

Comment 21

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8207861574e5

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8207861574e5

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8207861574e5