1751859 - Crash in [@ mozilla::VideoFramePool::ReleaseUnusedVAAPIFrames]

Status: RESOLVED DUPLICATE of bug 1724385

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

Description

[Pascal Chevrel:pascalc (PTO until April 26)]

Maybe Fission related. (DOMFissionEnabled=1)

Crash report: https://crash-stats.mozilla.org/report/index/96df68b6-754a-44cd-b11c-0ffaf0220125

Reason: SIGSEGV / SEGV_MAPERR

Top 10 frames of crashing thread:

0 libxul.so mozilla::VideoFramePool::ReleaseUnusedVAAPIFrames dom/media/platforms/ffmpeg/FFmpegVideoFramePool.cpp:87
1 libxul.so mozilla::FFmpegVideoDecoder<46465650>::DoDecode dom/media/platforms/ffmpeg/FFmpegVideoDecoder.cpp:525
2 libxul.so mozilla::FFmpegDataDecoder<46465650>::DoDecode dom/media/platforms/ffmpeg/FFmpegDataDecoder.cpp:188
3 libxul.so mozilla::FFmpegDataDecoder<46465650>::ProcessDecode dom/media/platforms/ffmpeg/FFmpegDataDecoder.cpp:142
4 libxul.so mozilla::detail::ProxyRunnable<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true>, RefPtr<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true> >  xpcom/threads/MozPromise.h:1536
5 libxul.so mozilla::TaskQueue::Runner::Run xpcom/threads/TaskQueue.cpp:206
6 libxul.so nsThreadPool::Run xpcom/threads/nsThreadPool.cpp:305
7 libxul.so nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1189
8 libxul.so mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:300
9 libxul.so MessageLoop::Run ipc/chromium/src/base/message_loop.cc:306

Martin, we have a huge volume of crashes (but very few users affected) and it seems to correlate with bug 1743638, could you have a look please? Do you want sheriffs to backout the regressor? Thanks

Comment 1

[Emilio Cobos Álvarez (:emilio)]

I'm hitting this crash on youtube all the time fwiw, if you need help reproing or debugging lmk.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1751859 - Add missing null-check in FFmpegVideoFramePool. r=stransky,alwu

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1751859 - Cleanup VideoFramePool::GetFreeVideoFrameSurface. r=stransky,alwu

Depends on D136873

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1751859 - Cleanup VideoFrameSurface a bit more. r=stransky,alwu

Depends on D136874

Comment 5

[Martin Stránský [:stransky] (ni? me)]

Great, Thanks!

Comment 6

[Martin Stránský [:stransky] (ni? me)]

Emilio, can you please provide MOZ_LOG="PlatformDecoderModule:5" log when it crashes? As I wrote in https://phabricator.services.mozilla.com/D136873#4453078 we should investigate why we use wrong pool type.
Thanks.

Comment 7

[Martin Stránský [:stransky] (ni? me)]

Also, this is caused by mixed dmabuf surface types. I think we want to remove plain dmabuf surfaces and use SHM ones instead (Bug 1713276) so this may be solved by removing dmabuf surfaces - but I'd like to understand what's going on here first.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

We hit this assert on a debug build: https://searchfox.org/mozilla-central/rev/02bd3e02b72d431f2c600a7328697a521f87d9b6/dom/media/platforms/ffmpeg/FFmpegVideoFramePool.cpp#150

So mVAAPIDeviceContext is null, but the video frame pool which is created earlier still thinks that VAAPI should be used.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: log

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Is the log above useful?

Comment 11

[Emilio Cobos Álvarez (:emilio)]

So mEnableHardwareDecoding (which is the video frame pool mUseVAApi) can be true, but we might still fall back to software decoding in a bunch of cases here.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1751859 - Don't create video frame pool too early. r=stransky

This makes sure that the decoder and the video frame pool agree on
hardware decoding status.

Depends on D136874

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Comment 12 is the really right fix, though I think we should also land the null-check.

Comment 14

[Martin Stránský [:stransky] (ni? me)]

The crashes are fixed by Bug 1724385.

Comment 16

[Martin Stránský [:stransky] (ni? me)]

With the null check we'll silently fail in case of mixed pool - I'd rather to crash cleanly when there's such pool misconfiguration.

Comment 17

[Martin Stránský [:stransky] (ni? me)]

Emilio, https://phabricator.services.mozilla.com/D136874 looks good, I'd like to land it.

Comment 18

[Calixte Denizet (:calixte)]

:gsvelto, is there a way to avoid to have so many crashes from a single user ?

Comment 19

[Gabriele Svelto [:gsvelto] (PTO)]

These crashes are being auto-submitted, we could put a throttling mechanism here or in the UnsubmittedCrashHandler code.

Comment 20

[Martin Stránský [:stransky] (ni? me)]

(In reply to Gabriele Svelto [:gsvelto] from comment #19)

These crashes are being auto-submitted, we could put a throttling mechanism here or in the UnsubmittedCrashHandler code.

Bug 1746232 may be related here.