Assertion failure: mGotTimecodeScale, at dom/media/webm/WebMBufferedParser.cpp:183

RESOLVED FIXED in Firefox 42

Status

()

defect
P1
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jwwang, Assigned: jya)

Tracking

unspecified
mozilla43
Points:
---

Firefox Tracking Flags

(firefox42 fixed, firefox43 fixed)

Details

Attachments

(2 attachments)

Posted patch test.patchSplinter Review
Assertion failure: mGotTimecodeScale, at /media/jwwang/DATA/codebase/mozilla-central2/dom/media/webm/WebMBufferedParser.cpp:183                                                                          [21/77530]
#01: nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::Length() const (/media/jwwang/DATA/codebase/mozilla-central2/obj-x86_64-unknown-linux-gnu/dom/media/webm/../../../dist/include/nsTArray.h:360 (discriminator 1))
#02: mozilla::WebMBufferedState::UpdateIndex(nsTArray<mozilla::MediaByteRange> const&, mozilla::MediaResource*) (/media/jwwang/DATA/codebase/mozilla-central2/dom/media/webm/WebMBufferedParser.cpp:426)
#03: mozilla::WebMDemuxer::EnsureUpToDateIndex() (/media/jwwang/DATA/codebase/mozilla-central2/dom/media/webm/WebMDemuxer.cpp:451)
#04: mozilla::WebMDemuxer::InitBufferedState() (/media/jwwang/DATA/codebase/mozilla-central2/dom/media/webm/WebMDemuxer.cpp:179)
#05: mozilla::WebMDemuxer::Init() (/media/jwwang/DATA/codebase/mozilla-central2/dom/media/webm/WebMDemuxer.cpp:156)
#06: mozilla::MediaFormatReader::AsyncReadMetadata() (/media/jwwang/DATA/codebase/mozilla-central2/dom/media/MediaFormatReader.cpp:296)
#07: nsAutoPtr<mozilla::detail::MethodCall<mozilla::MozPromise<nsRefPtr<mozilla::MetadataHolder>, mozilla::ReadMetadataFailureReason, true>, mozilla::MediaDecoderReader> >::assign(mozilla::detail::MethodCall<mo$illa::MozPromise<nsRefPtr<mozilla::MetadataHolder>, mozilla::ReadMetadataFailureReason, true>, mozilla::MediaDecoderReader>*) (/media/jwwang/DATA/codebase/mozilla-central2/obj-x86_64-unknown-linux-gnu/dom/media$../../dist/include/nsAutoPtr.h:34)
#08: mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() (/media/jwwang/DATA/codebase/mozilla-central2/obj-x86_64-unknown-linux-gnu/xpcom/threads/../../dist/include/mozilla/TaskDispatcher.h:183 (discriminator2))
#09: mozilla::TaskQueue::Runner::Run() (/media/jwwang/DATA/codebase/mozilla-central2/xpcom/threads/TaskQueue.cpp:170)
#10: nsThreadPool::Run() (/media/jwwang/DATA/codebase/mozilla-central2/xpcom/threads/nsThreadPool.cpp:232)
#11: nsThread::ProcessNextEvent(bool, bool*) (/media/jwwang/DATA/codebase/mozilla-central2/xpcom/threads/nsThread.cpp:874)
#12: NS_ProcessNextEvent(nsIThread*, bool) (/media/jwwang/DATA/codebase/mozilla-central2/xpcom/glue/nsThreadUtils.cpp:277)
#13: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) (/media/jwwang/DATA/codebase/mozilla-central2/ipc/glue/MessagePump.cpp:355)
#14: MessageLoop::RunInternal() (/media/jwwang/DATA/codebase/mozilla-central2/ipc/chromium/src/base/message_loop.cc:235)
#15: ~AutoRunState (/media/jwwang/DATA/codebase/mozilla-central2/ipc/chromium/src/base/message_loop.cc:520)
#16: nsThread::ThreadFunc(void*) (/media/jwwang/DATA/codebase/mozilla-central2/xpcom/threads/nsThread.cpp:365)
#17: _pt_root (/media/jwwang/DATA/codebase/mozilla-central2/nsprpub/pr/src/pthreads/ptthread.c:215)
#18: start_thread (/build/buildd/eglibc-2.19/nptl/pthread_create.c:312 (discriminator 2))
#19: __clone (/build/buildd/eglibc-2.19/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:113)

repro steps:
1. apply the patch to run 10 media files in parallel
2. ./mach mochitest dom/media/test/test_played.html
Hi jya,
Is this crash familiar to you?
Flags: needinfo?(jyavenard)
We had seen that crash when the WebMDemuxer was being developed in bug 1148102 and what caused quite a while to make it active by default (bug 1185792). The fix never made sense to me (on why it would work that is).

I guess it didn't work :)
Flags: needinfo?(jyavenard)
Assignee: nobody → jyavenard
Priority: -- → P1
can't reproduce it here (OS X on desktop or laptop).
Will fire linux and see,
Can't reproduce it in a linux VM after enabling ffmpeg (gstreamer causes some assers on some MP4).

JW, can you still reproduce it today?
what OS are you using, what's your mozconfig ?
Flags: needinfo?(jwwang)
mozconfig:
ac_add_options --enable-debug
ac_add_options --enable-debug-symbols
mk_add_options AUTOCLOBBER=1
mk_add_options MOZ_MAKE_FLAGS="-j6"

on Linux.

I will try again today to see if it makes a difference with and without ffmpeg enabled.
Flags: needinfo?(jwwang)
It still crashes after I turn on all mp4 and ffmpeg prefs.
Btw, the crash is in WebMDemuxer. I can't see how enabling ffmpeg for MP4 would help.
(In reply to JW Wang [:jwwang] from comment #6)
> It still crashes after I turn on all mp4 and ffmpeg prefs.
> Btw, the crash is in WebMDemuxer. I can't see how enabling ffmpeg for MP4
> would help.

Because otherwise for mp4 it uses gstreamer and I get some assertions in Intervals that end < start. Worth its own bug I guess

In any case ; I've reworked the demuxer going to submit something for kinetik's review ; would appreciate if you could test that it does fix the issue for you (I'm 99.99% certain it will)
We also now limit the use of the WebMBufferedState for calculating the buffered range and seeking on buffered data.
Attachment #8660554 - Flags: review?(kinetik)
Comment on attachment 8660554 [details] [diff] [review]
Ensure WebMBufferedState is only used after reading metadata.

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

::: dom/media/webm/WebMDemuxer.cpp
@@ +179,3 @@
>  WebMDemuxer::InitBufferedState()
>  {
> +  MOZ_ASSERT(!mBufferedState, "Developer error");

Just make it |MOZ_ASSERT(!mBufferedState)|; I don't think the text adds anything.

@@ +186,5 @@
>  WebMDemuxer::Clone() const
>  {
>    nsRefPtr<WebMDemuxer> demuxer = new WebMDemuxer(mResource.GetResource());
> +  demuxer->InitBufferedState();
> +  if (demuxer->ReadMetadata() != NS_OK) {

|if (NS_FAILED(demuxer->ReadMetadata())) {|

@@ +277,5 @@
> +    if (!buffer) {
> +      return NS_ERROR_FAILURE;
> +    }
> +    mBufferedState->NotifyDataArrived(buffer->Elements(), buffer->Length(), 0);
> +    if (mBufferedState->GetInitEndOffset() < 0) {

Probably makes sense to assert that GetInitEndOffset() <= mResource.Tell() (or buffer->Length() if you prefer).
Attachment #8660554 - Flags: review?(kinetik) → review+
WFM. No crash after applying the patch.
Comment on attachment 8660554 [details] [diff] [review]
Ensure WebMBufferedState is only used after reading metadata.

Approval Request Comment
[Feature/regressing bug #]:1148102
[User impact if declined]: Invalid playback ; asserts. Would have to disable the new WebM player and revert to the old one which would reintroduce several bugs
[Describe test coverage new/current, TreeHerder]: Local tests, try runs.
[Risks and why]: Low ; this is fixing a regression.
[String/UUID change made/needed]: None
Attachment #8660554 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/c93cd72f8d40
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Comment on attachment 8660554 [details] [diff] [review]
Ensure WebMBufferedState is only used after reading metadata.

Fix an assert, taking it.
Attachment #8660554 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.