1387801 - Assertion failure: AnnexB::IsAVCC(aSample) in [@ mp4_demuxer::H264::ExtractExtraData]

Status: RESOLVED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.mp4

Assertion failure: AnnexB::IsAVCC(aSample), at src/media/libstagefright/binding/H264.cpp:820

#0 0x7f8689fd6c5f in mp4_demuxer::H264::ExtractExtraData(mozilla::MediaRawData const*) src/media/libstagefright/binding/H264.cpp:836:16
#1 0x7f868e8b521a in mozilla::H264Converter::CheckForSPSChange(mozilla::MediaRawData*) src/dom/media/platforms/wrappers/H264Converter.cpp:396:5
#2 0x7f868e8b4883 in mozilla::H264Converter::Decode(mozilla::MediaRawData*) src/dom/media/platforms/wrappers/H264Converter.cpp:98:10
#3 0x7f868e5873b7 in mozilla::MediaFormatReader::DecoderFactory::Wrapper::Decode(mozilla::MediaRawData*) src/dom/media/MediaFormatReader.cpp:544:22
#4 0x7f868e559ac9 in mozilla::MediaFormatReader::DecodeDemuxedSamples(mozilla::TrackInfo::TrackType, mozilla::MediaRawData*) src/dom/media/MediaFormatReader.cpp:1908:21
#5 0x7f868e55a6cf in mozilla::MediaFormatReader::HandleDemuxedSamples(mozilla::TrackInfo::TrackType, mozilla::FrameStatistics::AutoNotifyDecoded&) src/dom/media/MediaFormatReader.cpp:2010:5
#6 0x7f868e558202 in mozilla::MediaFormatReader::Update(mozilla::TrackInfo::TrackType) src/dom/media/MediaFormatReader.cpp:2367:3
#7 0x7f868e5d8b55 in mozilla::detail::RunnableMethodImpl<mozilla::MediaFormatReader*, void (mozilla::MediaFormatReader::*)(mozilla::TrackInfo::TrackType), true, (mozilla::RunnableKind)0, mozilla::TrackInfo::TrackType>::Run() src/obj-firefox/dist/include/nsThreadUtils.h:1172:13
#8 0x7f868a217c17 in mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() src/obj-firefox/dist/include/mozilla/TaskDispatcher.h:209:37
#9 0x7f868a20c645 in mozilla::TaskQueue::Runner::Run() src/xpcom/threads/TaskQueue.cpp:246:12
#10 0x7f868a24895e in nsThreadPool::Run() src/xpcom/threads/nsThreadPool.cpp:225:14
#11 0x7f868a248ddc in non-virtual thunk to nsThreadPool::Run() src/xpcom/threads/nsThreadPool.cpp:154:15
#12 0x7f868a2404b0 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1446:14
#13 0x7f868a2460f0 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:480:10
#14 0x7f868ada913c in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:369:5
#15 0x7f868acfa6e7 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:326:10
#16 0x7f868acfa579 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299:3
#17 0x7f868a23861b in nsThread::ThreadFunc(void*) src/xpcom/threads/nsThread.cpp:506:11
#18 0x7f86a67015ed in _pt_root src/nsprpub/pr/src/pthreads/ptthread.c:216:5
#19 0x7f86a9d0b6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
#20 0x7f86a8d943dc in clone /build/glibc-bfm8X4/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1387801 - Reject invalid H264 content.

Review commit: https://reviewboard.mozilla.org/r/165320/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/165320/

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8894235 [details]
Bug 1387801 - Reject invalid H264 content.

https://reviewboard.mozilla.org/r/165320/#review170596

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e14c39f8c87
Reject invalid H264 content. r=gerald

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0e14c39f8c87

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Can we land a test for this? And is it worth backporting to 56 or should we let it ride the 57 train?

Comment 6

[Jean-Yves Avenard [:jya]]

While it would crash on a debug build, it would just cause a decoding error down the chain on release build.

We can certainly back port it to 56. It's actually a regression introduced in 55, we used to have this test in ExtractExtraData before, and this became just an assertion

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Doesn't sound worth the backport. Feel free to nominate it for approval if you feel otherwise.