Closed Bug 1207214 Opened 4 years ago Closed 4 years ago

Vine site crash at GonkVideoDecoderManager

Categories

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

Unspecified
Gonk (Firefox OS)
defect

Tracking

()

RESOLVED DUPLICATE of bug 1210045
blocking-b2g 2.5+

People

(Reporter: sotaro, Assigned: ayang)

References

Details

(Whiteboard: [ft:conndevices][partner-blocker])

Attachments

(3 files)

+++ 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.
[Blocking Requested - why for this release]: Crash happens easily on the link. And vine is popular service.
blocking-b2g: --- → 2.5?
Attached file stack of the crash
(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.
ni Blake. could you help on this? Thanks.
blocking-b2g: 2.5? → 2.5+
Flags: needinfo?(bwu)
Priority: -- → P1
(In reply to Bobby Chien [:bchien] from comment #4)
> ni Blake. could you help on this? Thanks.
Sure. I will check it.
Assignee: nobody → bwu
Flags: needinfo?(bwu)
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 ?? ()
See Also: → 1204622
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.
Depends on: 1204622
See Also: 1204622
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
Whiteboard: [ft:conndevices][partner-blocker]
Blocks: TV_P1
Per discussion with Blake, I will take a look.
Assignee: bwu → jwwang
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.
Assignee: jwwang → nobody
Flags: needinfo?(ayang)
(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.
Flags: needinfo?(ayang)
Assignee: nobody → ayang
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
Flags: needinfo?(jolin)
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.
(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
Flags: needinfo?(jolin)
Assert that decoder attaches EOS flag to final output buffer.
Attachment #8674677 - Flags: review?(jwwang)
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.
Attachment #8674677 - Flags: review?(jwwang) → review?(sotaro.ikeda.g)
Bug 1210045 also solves this problem.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1210045
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
Attachment #8674677 - Flags: review?(sotaro.ikeda.g) → review+
(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.
You need to log in before you can comment on or make changes to this bug.