Tyson Smith [:tsmith] Reporter
	Description • 7 years ago

Assertion failure: false, at src/media/libstagefright/binding/include/mp4_demuxer/ByteReader.h:78

Marking as s-s for now.

#0 0x7f5a559069f9 in mp4_demuxer::ByteReader::ReadU16() src/media/libstagefright/binding/include/mp4_demuxer/ByteReader.h:78:7
#1 0x7f5a5590e773 in mp4_demuxer::SPSNALIterator::operator*() const src/media/libstagefright/binding/H264.cpp:311:30
#2 0x7f5a5590e42a in mp4_demuxer::H264::DecodeSPSFromExtraData(mozilla::MediaByteBuffer const*, mp4_demuxer::SPSData&) src/media/libstagefright/binding/H264.cpp:738:11
#3 0x7f5a5a412d2a in mozilla::AccumulateSPSTelemetry(mozilla::MediaByteBuffer const*) src/dom/media/fmp4/MP4Demuxer.cpp:87:7
#4 0x7f5a5a415bba in mozilla::MP4TrackDemuxer::MP4TrackDemuxer(mozilla::MP4Demuxer*, mozilla::UniquePtr<mozilla::TrackInfo, mozilla::DefaultDelete<mozilla::TrackInfo> >&&, mp4_demuxer::IndiceWrapper const&) src/dom/media/fmp4/MP4Demuxer.cpp:373:28
#5 0x7f5a5a413dbc in mozilla::MP4Demuxer::Init() src/dom/media/fmp4/MP4Demuxer.cpp:255:13
#6 0x7f5a59ee6465 in mozilla::MediaFormatReader::DemuxerProxy::Init()::$_10::operator()() const src/dom/media/MediaFormatReader.cpp:1027:47
#7 0x7f5a59ee6116 in mozilla::detail::ProxyFunctionRunnable<mozilla::MediaFormatReader::DemuxerProxy::Init()::$_10, mozilla::MozPromise<mozilla::MediaResult, mozilla::MediaResult, true> >::Run() src/obj-firefox/dist/include/mozilla/MozPromise.h:1510:29
#8 0x7f5a55b45645 in mozilla::TaskQueue::Runner::Run() src/xpcom/threads/TaskQueue.cpp:246:12
#9 0x7f5a55b8195e in nsThreadPool::Run() src/xpcom/threads/nsThreadPool.cpp:225:14
#10 0x7f5a55b81ddc in non-virtual thunk to nsThreadPool::Run() src/xpcom/threads/nsThreadPool.cpp:154:15
#11 0x7f5a55b794b0 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1446:14
#12 0x7f5a55b7f0f0 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:480:10
#13 0x7f5a566e1f64 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:339:20
#14 0x7f5a566336e7 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:326:10
#15 0x7f5a56633579 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299:3
#16 0x7f5a55b7161b in nsThread::ThreadFunc(void*) src/xpcom/threads/nsThread.cpp:506:11
#17 0x7f5a7203a5ed in _pt_root src/nsprpub/pr/src/pthreads/ptthread.c:216:5
#18 0x7f5a756446b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
#19 0x7f5a746cd3dc in clone /build/glibc-bfm8X4/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

	Blake Wu [:bwu][:blakewu]
	Comment 1 • 7 years ago

Alfredo,
This looks at your wheelhouse.

	Alfredo Yang (:alfredo) Assignee
	Comment 2 • 7 years ago

Hi, do you run this in debug?

The assertion is caused by incorrect streams.
The extra data in the stream is:

01 42 c0 0d ff e1 00

The final '00' is the number of SPS; however, it should be 2 bytes, not 1 byte. So it causes assertion. There is nothing wrong in codes.

I have tried 54.0.1, and it can't be played as I expected.

	Tyson Smith [:tsmith] Reporter
	Comment 3 • 7 years ago

(In reply to Alfredo Yang (:alfredo) from comment #2)
> Hi, do you run this in debug?
> 
> The assertion is caused by incorrect streams.
> The extra data in the stream is:
> 
> 01 42 c0 0d ff e1 00
> 
> The final '00' is the number of SPS; however, it should be 2 bytes, not 1
> byte. So it causes assertion. There is nothing wrong in codes.
> 
> I have tried 54.0.1, and it can't be played as I expected.

Yes this assertion came from a debug build. This was found by fuzzing. If this assertion is incorrect or invalid please remove it.

	Alfredo Yang (:alfredo) Assignee
	Comment 4 • 7 years ago

The assertion is here for developer to find the problem quickly because the stream is incorrect. Since it's not affect on release, I'd prefer to keep it.

	Ryan VanderMeulen [:RyanVM]
	Comment 5 • 7 years ago

Can it be downgraded to a warning instead of an assertion?

	Alfredo Yang (:alfredo) Assignee
	Comment 6 • 7 years ago

(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)
> Can it be downgraded to a warning instead of an assertion?

Sure, I'll add a patch for that.

	Pulsebot
	Comment 8 • 7 years ago

Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c8a770bde30
Downgrade assertion to warning. r=kinetik

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 9 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/9c8a770bde30