Closed Bug 1000841 Opened 10 years ago Closed 10 years ago

MOZ_Assert: Assertion failure: mIsActive, at ../../../../gecko/content/media/omx/MediaOmxReader.cpp:344

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla32

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(1 file)

1000841_fix_MediaOmxReader.patch
23.29 KB, patch
cpearce
: review+
Details | Diff | Splinter Review 
https://tbpl.mozilla.org/php/getParsedLog.php?id=38381120&tree=Try&full=1#error3

call stack:
00:18:08  WARNING -  PROCESS-CRASH | /tests/content/media/test/test_bug465498.html | application crashed [@ mozilla::MediaOmxReader::DecodeAudioData()]
00:18:08     INFO -  Crash dump filename: /tmp/tmpORt3eS/236bb006-d7d8-9647-0c577c4e-246e5698.dmp
00:18:08     INFO -  Operating system: Android
00:18:08     INFO -                    0.0.0 Linux 2.6.29-00302-g586075d #31 Mon Feb 24 10:28:23 PST 2014 armv7l Android/full/generic:4.0.4.0.4.0.4/OPENMASTER/eng.cltbld.20140423.235559:eng/test-keys
00:18:08     INFO -  CPU: arm
00:18:08     INFO -       0 CPUs
00:18:08     INFO -  Crash reason:  SIGSEGV
00:18:08     INFO -  Crash address: 0x0
00:18:08     INFO -  Thread 18 (crashed)
00:18:08     INFO -   0  libxul.so!mozilla::MediaOmxReader::DecodeAudioData() [MediaOmxReader.cpp:9e0487ab833d : 344 + 0x2]
00:18:08     INFO -       r4 = 0x44186580    r5 = 0x00000000    r6 = 0x441d64ec    r7 = 0x43ececf0
00:18:08     INFO -       r8 = 0x4451a820    r9 = 0x4216b084   r10 = 0x4216c153    fp = 0x00000001
00:18:08     INFO -       sp = 0x43ecec48    lr = 0x41362a33    pc = 0x41362a34
00:18:08     INFO -      Found by: given as instruction pointer in context
00:18:08     INFO -   1  libxul.so!mozilla::MediaDecoderStateMachine::DecodeAudio() [MediaDecoderStateMachine.cpp:9e0487ab833d : 685 + 0xb]
00:18:08     INFO -       r4 = 0x441d6460    r5 = 0x441d6464    r6 = 0x441d64ec    r7 = 0x43ececf0
00:18:08     INFO -       r8 = 0x4451a820    r9 = 0x4216b084   r10 = 0x4216c153    fp = 0x00000001
00:18:08     INFO -       sp = 0x43ecece8    pc = 0x4130855f
00:18:08     INFO -      Found by: call frame info
00:18:08     INFO -   2  libxul.so!nsRunnableMethodImpl<unsigned int (mozilla::MediaDecoderStateMachine::*)(), void, true>::Run() + 0x1b
00:18:08     INFO -       r4 = 0x44a28620    r5 = 0x441fd8c0    r6 = 0x4451a828    r7 = 0x4451a820
00:18:08     INFO -       r8 = 0x4451a820    r9 = 0x4216b084   r10 = 0x4216c153    fp = 0x00000001
00:18:08     INFO -       sp = 0x43eced10    pc = 0x405f0089
00:18:08     INFO -      Found by: call frame info
00:18:08     INFO -   3  libxul.so!mozilla::MediaTaskQueue::Runner::Run() [MediaTaskQueue.cpp:9e0487ab833d : 127 + 0x7]
00:18:08     INFO -       r4 = 0x44a28620    r5 = 0x441fd8c0    r6 = 0x4451a828    r7 = 0x4451a820
00:18:08     INFO -       r8 = 0x4451a820    r9 = 0x4216b084   r10 = 0x4216c153    fp = 0x00000001
00:18:08     INFO -       sp = 0x43eced18    pc = 0x4131934f
00:18:08     INFO -      Found by: call frame info
00:18:08     INFO -   4  libxul.so!nsThreadPool::Run() [nsThreadPool.cpp:9e0487ab833d : 211 + 0x5]
00:18:08     INFO -       r4 = 0x4451a2e0    r5 = 0x00000000    r6 = 0x00000000    r7 = 0x4217c344
00:18:08     INFO -       r8 = 0x4216b04b    r9 = 0x4216b084   r10 = 0x4216c153    fp = 0x00000001
00:18:08     INFO -       sp = 0x43eced40    pc = 0x40626315
00:18:08     INFO -      Found by: call frame info
00:18:08     INFO -   5  libxul.so!nsThread::ProcessNextEvent(bool, bool*) [nsThread.cpp:9e0487ab833d : 701 + 0x9]
00:18:08     INFO -       r4 = 0x4486db00    r5 = 0x00000000    r6 = 0x43ecedd4    r7 = 0x43ecedd0
00:18:08     INFO -       r8 = 0x00000000    r9 = 0x43ecee0f   r10 = 0x4486db3c    fp = 0x00000000
00:18:08     INFO -       sp = 0x43eceda0    pc = 0x4062473b
00:18:08     INFO -      Found by: call frame info
00:18:08     INFO -   6  libxul.so!NS_ProcessNextEvent(nsIThread*, bool) [nsThreadUtils.cpp:9e0487ab833d : 263 + 0xd]
00:18:08     INFO -       r4 = 0x4486db00    r5 = 0x00000000    r6 = 0x43ff0f70    r7 = 0x43ff0f88
00:18:08     INFO -       r8 = 0x448fa320    r9 = 0x42a9a744   r10 = 0x00020000    fp = 0x00000001
00:18:08     INFO -       sp = 0x43ecee08    pc = 0x405d2d8d
00:18:08     INFO -      Found by: call frame info
00:18:08     INFO -   7  libxul.so!mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) [MessagePump.cpp:9e0487ab833d : 307 + 0x7]
00:18:08     INFO -       r4 = 0x43ff0f80    r5 = 0x441821a0    r6 = 0x43ff0f70    r7 = 0x43ff0f88
00:18:08     INFO -       r8 = 0x448fa320    r9 = 0x42a9a744   r10 = 0x00020000    fp = 0x00000001
00:18:08     INFO -       sp = 0x43ecee20    pc = 0x408037d1
00:18:08     INFO -      Found by: call frame info
00:18:08     INFO -   8  libxul.so!MessageLoop::RunInternal() [message_loop.cc:9e0487ab833d : 226 + 0x7]
00:18:08     INFO -       r4 = 0x448fa320    r5 = 0x448fa320    r6 = 0x42a9a744    r7 = 0x00000000
00:18:08     INFO -       r8 = 0x43eceea0    r9 = 0x4486db10   r10 = 0x00020000    fp = 0x00000001
00:18:08     INFO -       sp = 0x43ecee58    pc = 0x407eece3
00:18:08     INFO -      Found by: call frame info
00:18:08     INFO -   9  libxul.so!MessageLoop::Run() [message_loop.cc:9e0487ab833d : 219 + 0x5]
00:18:08     INFO -       r4 = 0x448fa320    r5 = 0x448fa320    r6 = 0x42a9a744    r7 = 0x00000000
00:18:08     INFO -       r8 = 0x43eceea0    r9 = 0x4486db10   r10 = 0x00020000    fp = 0x00000001
00:18:08     INFO -       sp = 0x43ecee70    pc = 0x407eecfb
00:18:08     INFO -      Found by: call frame info
00:18:08     INFO -  10  libxul.so!nsThread::ThreadFunc(void*) [nsThread.cpp:9e0487ab833d : 311 + 0x3]
00:18:08     INFO -       r4 = 0x4486db00    r5 = 0x448fa320    r6 = 0x42a9a744    r7 = 0x00000000
00:18:08     INFO -       r8 = 0x43eceea0    r9 = 0x4486db10   r10 = 0x00020000    fp = 0x00000001
00:18:08     INFO -       sp = 0x43ecee88    pc = 0x40623bfb
00:18:08     INFO -      Found by: call frame info
00:18:08     INFO -  11  libnss3.so!_pt_root [ptthread.c:9e0487ab833d : 212 + 0x5]
00:18:08     INFO -       r4 = 0x44a05e00    r5 = 0x00000000    r6 = 0x42e263d8    r7 = 0x0000dab0
00:18:08     INFO -       r8 = 0x00000000    r9 = 0x00000001   r10 = 0x00020000    fp = 0x00000001
00:18:08     INFO -       sp = 0x43eceed0    pc = 0x42d30685
00:18:08     INFO -      Found by: call frame info
00:18:08     INFO -  12  libc.so!__thread_entry [pthread.c : 217 + 0x6]
00:18:08     INFO -       r4 = 0x43ecef00    r5 = 0x42d305d9    r6 = 0x44a05e00    r7 = 0x00000078
00:18:08     INFO -       r8 = 0x42d305d9    r9 = 0x44a05e00   r10 = 0x00020000    fp = 0x00000001
00:18:08     INFO -       sp = 0x43eceef0    pc = 0x4005ee4c
00:18:08     INFO -      Found by: call frame info
00:18:08     INFO -  13  libc.so!pthread_create [pthread.c : 357 + 0xe]
00:18:08     INFO -       r4 = 0x43ecef00    r5 = 0x0000dab0    r6 = 0x4583fbe4    r7 = 0x00000078
00:18:08     INFO -       r8 = 0x42d305d9    r9 = 0x44a05e00   r10 = 0x00020000    fp = 0x00000001
00:18:08     INFO -       sp = 0x43ecef00    pc = 0x4005e99c
00:18:08     INFO -      Found by: call frame info

In MediaDecoderStateMachine::DispatchDecodeTasksIfNeeded(), MediaDecoderStateMachine::SetReaderIdle() should be dispatched before MediaDecoderStateMachine::DecodeAudio() so that reader idle status is updated correctly before calling mReader->DecodeAudioData which will check the idle status.
It looks like this bug escape our test case for B2G debug disables all media tests.
Depends on: 993753
Ok, I see why MediaDecoderStateMachine::EnsureActive fails to work.

1. |mIsReaderIdle| is set to false in MediaDecoderStateMachine::DispatchDecodeTasksIfNeeded()
2. MediaDecoderStateMachine::DecodeAudio() call EnsureActive() to ensure the reader is active.
3. EnsureActive() returns immediately for |mIsReaderIdle| is changed to false in (1).

I think we should remove EnsureActive() and dispatch SetReaderIdle/Active() before Audio/VideoDecodeTask. We should also move checking |mIsActive| to its base class which is MediaDecoderReader to ensure the reader status is correct across all its sub-classes.
Flags: needinfo?(cpearce)
(In reply to JW Wang [:jwwang] from comment #2)
> Ok, I see why MediaDecoderStateMachine::EnsureActive fails to work.
> 
> 1. |mIsReaderIdle| is set to false in
> MediaDecoderStateMachine::DispatchDecodeTasksIfNeeded()
> 2. MediaDecoderStateMachine::DecodeAudio() call EnsureActive() to ensure the
> reader is active.
> 3. EnsureActive() returns immediately for |mIsReaderIdle| is changed to
> false in (1).
> 
> I think we should remove EnsureActive() and dispatch SetReaderIdle/Active()
> before Audio/VideoDecodeTask. 

Sounds good.

> We should also move checking |mIsActive| to
> its base class which is MediaDecoderReader to ensure the reader status is
> correct across all its sub-classes.

I would prefer to remove as much functionality from MediaDecoderReader as possible, so we rely on composition rather than inheritance in our code. What did you have in mind?
Flags: needinfo?(cpearce)
Can we also move the active/idle status keeping into MediaOmxReader? If we always have to call EnsureActive() before decoding/seeking in MediaDecoderStateMachine, we can move the code to MediaOmxReader so that it is always safe to call MediaDecoderReader::DecodeVideoFrame or MediaDecoderReader::DecodeAudioData. However, only MediaDecoderStateMachine knows when the reader need to be idle, so MediaDecoderReader should expose a function like SetIdle() to be called from MediaDecoderStateMachine.
See Also: → 1003037
Remove SetActive() from MediaDecoderReader. Its user needs to know when to call SetIdle() only.

try: https://tbpl.mozilla.org/?tree=Try&rev=a45e8df55ee0
most green.
Assignee: nobody → jwwang
Status: NEW → ASSIGNED
Attachment #8424760 - Flags: review?(cpearce)
Comment on attachment 8424760 [details] [diff] [review]
1000841_fix_MediaOmxReader.patch

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

Looks good.
Attachment #8424760 - Flags: review?(cpearce) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a8ebf53bc0cf
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: