Closed Bug 1752388 Opened 2 years ago Closed 2 years ago

Assertion failure: mIsLocked, at /builds/worker/checkouts/gecko/gfx/layers/client/TextureClient.cpp:659

Categories

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

defect

Tracking

()

VERIFIED FIXED
99 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox96 --- unaffected
firefox97 --- unaffected
firefox98 --- disabled
firefox99 --- verified

People

(Reporter: tsmith, Assigned: alwu)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [bugmon:bisected,confirmed])

Attachments

(3 files)

Attached video testcase.mp4

Found while fuzzing m-c 20220127-7dff1a12e1a4 (--enable-debug --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -d --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.mp4

Assertion failure: mIsLocked, at /builds/worker/checkouts/gecko/gfx/layers/client/TextureClient.cpp:659

#0 0x7f260b1b31f3 in mozilla::layers::TextureClient::Unlock() /builds/worker/checkouts/gecko/gfx/layers/client/TextureClient.cpp:659:3
#1 0x7f260db5971c in mozilla::FFmpegVideoDecoder<46465650>::CreateImage(long, long, long, nsTArray<RefPtr<mozilla::MediaData> >&) const /builds/worker/checkouts/gecko/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.cpp:1070:16
#2 0x7f260db58673 in mozilla::FFmpegVideoDecoder<46465650>::DoDecode(mozilla::MediaRawData*, unsigned char*, int, bool*, nsTArray<RefPtr<mozilla::MediaData> >&) /builds/worker/checkouts/gecko/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.cpp:882:12
#3 0x7f260db540dd in mozilla::FFmpegDataDecoder<46465650>::DoDecode(mozilla::MediaRawData*, bool*, nsTArray<RefPtr<mozilla::MediaData> >&) /builds/worker/checkouts/gecko/dom/media/platforms/ffmpeg/FFmpegDataDecoder.cpp:192:10
#4 0x7f260db53e0d in mozilla::FFmpegDataDecoder<46465650>::ProcessDecode(mozilla::MediaRawData*) /builds/worker/checkouts/gecko/dom/media/platforms/ffmpeg/FFmpegDataDecoder.cpp:146:20
#5 0x7f260db5b0b5 in applyImpl<mozilla::FFmpegDataDecoder<46465650>, RefPtr<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true> > (mozilla::FFmpegDataDecoder<46465650>::*)(mozilla::MediaRawData *), StoreRefPtrPassByPtr<mozilla::MediaRawData> , 0UL> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1147:12
#6 0x7f260db5b0b5 in apply<mozilla::FFmpegDataDecoder<46465650>, RefPtr<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true> > (mozilla::FFmpegDataDecoder<46465650>::*)(mozilla::MediaRawData *)> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1153:12
#7 0x7f260db5b0b5 in Invoke /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:1516:47
#8 0x7f260db5b0b5 in mozilla::detail::ProxyRunnable<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true>, RefPtr<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true> > (mozilla::FFmpegDataDecoder<46465650>::*)(mozilla::MediaRawData*), mozilla::FFmpegDataDecoder<46465650>, mozilla::MediaRawData*>::Run() /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:1536:42
#9 0x7f2609ac0a91 in mozilla::TaskQueue::Runner::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskQueue.cpp:206:20
#10 0x7f2609adbe8b in nsThreadPool::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadPool.cpp:305:14
#11 0x7f2609ad2689 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1189:16
#12 0x7f2609ad96aa in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:467:10
#13 0x7f260a57fd0b in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:300:20
#14 0x7f260a49ec07 in MessageLoop::RunInternal() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:331:10
#15 0x7f260a49eb12 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:324:3
#16 0x7f260a49eb12 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:306:3
#17 0x7f2609ace29b in nsThread::ThreadFunc(void*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:391:10
#18 0x7f261fcb6997 in _pt_root /builds/worker/checkouts/gecko/nsprpub/pr/src/pthreads/ptthread.c:201:5
#19 0x7f2620a32608 in start_thread /build/glibc-eX1tMB/glibc-2.31/nptl/pthread_create.c:477:8
#20 0x7f26205fa292 in __clone /build/glibc-eX1tMB/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Flags: in-testsuite?
Assignee: nobody → alwu
Severity: -- → S3
Priority: -- → P3
Regressed by: 1713276

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20220127213627-b9121c1a4330.
The bug appears to have been introduced in the following build range:

Start: 9b23d1bb84b2499b94d91c5f588fc93a54e5bdcc (20220124214229)
End: e960e654cbc9f60ce79eb1535fd6ec4e3acc2029 (20220125100058)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9b23d1bb84b2499b94d91c5f588fc93a54e5bdcc&tochange=e960e654cbc9f60ce79eb1535fd6ec4e3acc2029

Whiteboard: [bugmon:bisected,confirmed]

Set release status flags based on info from the regressing bug 1713276

Has Regression Range: --- → yes

Al, could you please re-evaluate this now that there is a bisection available?

Flags: needinfo?(alwu)

Looking at the code, we are trying to unlock an already unlocked TextureClient. Was it supposed to be locked and is there a potential data race at play here?

Bisection actually isn't matter, because I already know this is regressed by Bug 1713276. Considering this is the feature only enabled on Nightly, I think P3 is fair.

In addition, I couldn't reproduce this on my Ubuntu 20.04, I saw a different error while running grizzly.

Hit MOZ_CRASH(Attempting to connect to non-local address!) at /builds/worker/checkouts/gecko/netwerk/base/nsSocketTransport2.cpp:1237 (f8bc2eb4:fe697183)

Tyson, if I remember it correctly, Grizzly can also print the debug log. Would you mind help me to use MOZ_LOG=PlatformDecoderModule:5 to see what happens?

Thank you.

Flags: needinfo?(alwu) → needinfo?(twsmith)
Attached file stderr.txt

To do this via Grizzly just set the environment variable and save the logs.

$ MOZ_LOG=PlatformDecoderModule:5 python3 -m grizzly.replay firefox testcase.mp4 -l .

Flags: needinfo?(twsmith)

Thanks for Tyson's help, now I can also reproduce this issue on my side.

[RDD 2572679: MediaSupervisor #1]: D/PlatformDecoderModule FFVPX: Hardware WebRender is off, disabled DMABuf & VAAPI.
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Initialising FFmpeg decoder
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX:   codec vp9 : Google VP9
[RDD 2572679: MediaPDecoder #2]: D/PlatformDecoderModule FFVPX: Initialising FFmpeg decoder
[RDD 2572679: MediaPDecoder #2]: D/PlatformDecoderModule FFVPX:   codec mp3 : MP3 (MPEG audio layer 3)
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Set get_buffer2 for customized buffer allocation
[vp9 @ 0x7fc5d0002440] Requested frame threading with a custom get_buffer2() implementation which is not marked as thread safe. This is not supported anymore, make your callback thread-safe.
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX:   FFmpeg decoder init successful.
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Choosing FFmpeg pixel format for video decoding.
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Requesting pixel format YUV420P.

# Ask for buffer & decode 1
[RDD 2572679: MediaPDecoder #1]: V/PlatformDecoderModule GetVideoBuffer: aCodecContext=7fc5d0002440 aFrame=7fc5d0008d80
[RDD 2572679: MediaPDecoder #1]: V/PlatformDecoderModule Created plane data, YSize=(128, 128), CbCrSize=(128, 64), CroppedYSize=(128, 128), CroppedCbCrSize=(64, 64), ColorDepth=0
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Created av buffer, buf=7fc5d00232c0, data=7fc5e238d000, image=7fc5d0022e90, sz=24576
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Got one frame output with pts=0 dts=0 duration=33366 opaque=-9223372036854775808
[RDD 2572679: MediaPDecoder #2]: D/PlatformDecoderModule FFVPX:   FFmpeg decoder init successful.

# Ask for buffer & decode 2
[RDD 2572679: MediaPDecoder #3]: V/PlatformDecoderModule GetVideoBuffer: aCodecContext=7fc5d0002440 aFrame=7fc5d0008d80
[RDD 2572679: MediaPDecoder #3]: V/PlatformDecoderModule Created plane data, YSize=(128, 128), CbCrSize=(128, 64), CroppedYSize=(128, 128), CroppedCbCrSize=(64, 64), ColorDepth=0
[RDD 2572679: MediaPDecoder #3]: D/PlatformDecoderModule FFVPX: Created av buffer, buf=7fc5c8006340, data=7fc5e237c000, image=7fc5c8005f20, sz=24576
[RDD 2572679: MediaPDecoder #3]: D/PlatformDecoderModule FFVPX: Got one frame output with pts=33366 dts=33366 duration=33367 opaque=-9223372036854775808

# Ask for buffer & decode 3
[RDD 2572679: MediaPDecoder #1]: V/PlatformDecoderModule GetVideoBuffer: aCodecContext=7fc5d0002440 aFrame=7fc5d0008d80
[RDD 2572679: MediaPDecoder #1]: V/PlatformDecoderModule Created plane data, YSize=(128, 128), CbCrSize=(128, 64), CroppedYSize=(128, 128), CroppedCbCrSize=(64, 64), ColorDepth=0
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Created av buffer, buf=7fc5d00244c0, data=7fc5da639000, image=7fc5d0024700, sz=24576
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Got one frame output with pts=66733 dts=66733 duration=33367 opaque=-9223372036854775808
[RDD 2572679: MediaPDecoder #1]: V/PlatformDecoderModule ReleaseVideoBufferWrapper: PlanarYCbCrImage=7fc5c8005ce0

# Ask for a buffer & decode 4
[RDD 2572679: MediaPDecoder #1]: V/PlatformDecoderModule GetVideoBuffer: aCodecContext=7fc5d0002440 aFrame=7fc5d0008d80
[RDD 2572679: MediaPDecoder #1]: V/PlatformDecoderModule Created plane data, YSize=(128, 128), CbCrSize=(128, 64), CroppedYSize=(128, 128), CroppedCbCrSize=(64, 64), ColorDepth=0
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Created av buffer, buf=7fc5d00244c0, data=7fc5da62f000, image=7fc5d000eb80, sz=24576
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Got one frame output with pts=100100 dts=100100 duration=33366 opaque=-9223372036854775808
[RDD 2572679: MediaPDecoder #1]: V/PlatformDecoderModule ReleaseVideoBufferWrapper: PlanarYCbCrImage=7fc5cc009a10

# Ask for a buffer & decode 5
[RDD 2572679: MediaPDecoder #1]: V/PlatformDecoderModule GetVideoBuffer: aCodecContext=7fc5d0002440 aFrame=7fc5d0008d80
[RDD 2572679: MediaPDecoder #1]: V/PlatformDecoderModule Created plane data, YSize=(128, 128), CbCrSize=(128, 64), CroppedYSize=(128, 128), CroppedCbCrSize=(64, 64), ColorDepth=0
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Created av buffer, buf=7fc5d0029400, data=7fc5da625000, image=7fc5d000f100, sz=24576
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Got one frame output with pts=133466 dts=133466 duration=33367 opaque=-9223372036854775808
[RDD 2572679: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Got one frame output with pts=166833 dts=166833 duration=33366 opaque=-9223372036854775808
Assertion failure: mIsLocked, at /builds/worker/checkouts/gecko/gfx/layers/client/TextureClient.cpp:659

So the assertion happened on the last time we ask for decoded data, and ffmpeg gave us two frames. The second one seems storing in an unlocked texture. My guess is that, ffmpeg reused the buffer for 133466 which was already unlocked. So when we tried to unlock that texture, it hit the assertion. I filed bug 1754274 to add more log and will try to reproduce this again later to see if my assumption is correct.

Here are more logs. From the log below, we can see FFmpeg reused one of the buffer we allocated before, which is bad. In the bad case, the image might haven't been presented to the compositor yet so that shmem buffer should remain unchanged.

In the last section of the log, it's also uncommon that Got one frame output got called twice, but there is only one log for creating image via shmem buffer. It's still not clear to me why this would happen, did we switch back to the normal image (copy) for one of them?

Martin, is it normal that ffmpeg would reuse a shmem buffer multiple times? Are we possible to prevent it from doing that? Or is that unusal (?) situation caused by some wrong operations on our side?

Thank you.

# Create image 1 (7f8210008d00)
[RDD 3019329: MediaPDecoder #1]: V/PlatformDecoderModule GetVideoBuffer: aCodecContext=7f81ec002440 aFrame=7f81ec008d80
[RDD 3019329: MediaPDecoder #1]: V/PlatformDecoderModule Created plane data, YSize=(128, 128), CbCrSize=(128, 64), CroppedYSize=(128, 128), CroppedCbCrSize=(64, 64), ColorDepth=0
[RDD 3019329: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Created av buffer, buf=7f81ec023300, data=7f8214418000, image=7f8210008d00, sz=24576
[RDD 3019329: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Got one frame output with pts=0 dts=0 duration=33366 opaque=-9223372036854775808
[RDD 3019329: MediaPDecoder #1]: V/PlatformDecoderModule Create a video data from a shmem image=7f8210008d00
[RDD 3019329: MediaPDecoder #2]: D/PlatformDecoderModule FFVPX:   FFmpeg decoder init successful.

# Create image 2 (7f81e4003480)
[RDD 3019329: MediaPDecoder #3]: V/PlatformDecoderModule GetVideoBuffer: aCodecContext=7f81ec002440 aFrame=7f81ec008d80
[RDD 3019329: MediaPDecoder #3]: V/PlatformDecoderModule Created plane data, YSize=(128, 128), CbCrSize=(128, 64), CroppedYSize=(128, 128), CroppedCbCrSize=(64, 64), ColorDepth=0
[RDD 3019329: MediaPDecoder #3]: D/PlatformDecoderModule FFVPX: Created av buffer, buf=7f81e40062c0, data=7f81f73e7000, image=7f81e4003480, sz=24576
[RDD 3019329: MediaPDecoder #3]: D/PlatformDecoderModule FFVPX: Got one frame output with pts=33366 dts=33366 duration=33367 opaque=-9223372036854775808
[RDD 3019329: MediaPDecoder #3]: V/PlatformDecoderModule Create a video data from a shmem image=7f81e4003480

# Create image 3 (7f81e800cb30) and relase image 2 (7f81e4003480)
[RDD 3019329: MediaPDecoder #1]: V/PlatformDecoderModule GetVideoBuffer: aCodecContext=7f81ec002440 aFrame=7f81ec008d80
[RDD 3019329: MediaPDecoder #1]: V/PlatformDecoderModule Created plane data, YSize=(128, 128), CbCrSize=(128, 64), CroppedYSize=(128, 128), CroppedCbCrSize=(64, 64), ColorDepth=0
[RDD 3019329: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Created av buffer, buf=7f81ec0245c0, data=7f81f73dd000, image=7f81e800cb30, sz=24576
[RDD 3019329: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Got one frame output with pts=66733 dts=66733 duration=33367 opaque=-9223372036854775808
[RDD 3019329: MediaPDecoder #1]: V/PlatformDecoderModule Create a video data from a shmem image=7f81e800cb30
[RDD 3019329: MediaPDecoder #1]: V/PlatformDecoderModule ReleaseVideoBufferWrapper: PlanarYCbCrImage=7f81e4003480

# Create image 4 (7f81ec00f4e0) and release image 3 (7f81e800cb30)
[RDD 3019329: MediaPDecoder #1]: V/PlatformDecoderModule GetVideoBuffer: aCodecContext=7f81ec002440 aFrame=7f81ec008d80
[RDD 3019329: MediaPDecoder #1]: V/PlatformDecoderModule Created plane data, YSize=(128, 128), CbCrSize=(128, 64), CroppedYSize=(128, 128), CroppedCbCrSize=(64, 64), ColorDepth=0
[RDD 3019329: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Created av buffer, buf=7f81ec0245c0, data=7f81f73d3000, image=7f81ec00f4e0, sz=24576
[RDD 3019329: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Got one frame output with pts=100100 dts=100100 duration=33366 opaque=-9223372036854775808
[RDD 3019329: MediaPDecoder #1]: V/PlatformDecoderModule Create a video data from a shmem image=7f81ec00f4e0
[RDD 3019329: MediaPDecoder #1]: V/PlatformDecoderModule ReleaseVideoBufferWrapper: PlanarYCbCrImage=7f81e800cb30

# Create image 5 (7f81ec00f2a0) and REUSE image 1 (7f8210008d00)
[RDD 3019329: MediaPDecoder #1]: V/PlatformDecoderModule GetVideoBuffer: aCodecContext=7f81ec002440 aFrame=7f81ec008d80
[RDD 3019329: MediaPDecoder #1]: V/PlatformDecoderModule Created plane data, YSize=(128, 128), CbCrSize=(128, 64), CroppedYSize=(128, 128), CroppedCbCrSize=(64, 64), ColorDepth=0
[RDD 3019329: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Created av buffer, buf=7f81ec028e00, data=7f81f73c9000, image=7f81ec00f2a0, sz=24576
[RDD 3019329: MediaPDecoder #1]: D/PlatformDecoderModule FFVPX: Got one frame output with pts=133466 dts=133466 duration=33367 opaque=-9223372036854775808
[RDD 3019329: MediaPDecoder #2]: D/PlatformDecoderModule FFVPX: Got one frame output with pts=166833 dts=166833 duration=33366 opaque=-9223372036854775808
[RDD 3019329: MediaPDecoder #2]: V/PlatformDecoderModule Create a video data from a shmem image=7f8210008d00
Assertion failure: mIsLocked, at /builds/worker/checkouts/gecko/gfx/layers/client/TextureClient.cpp:659
Flags: needinfo?(stransky)

I didn't investigate it much but:

See https://searchfox.org/mozilla-central/rev/69a482382823f42f482e840f65725218d7654cc4/dom/media/platforms/ffmpeg/FFmpegVideoFramePool.cpp#44

We use av_buffer_ref(aAVFrame->buf[0]); to lock AVFrame until we need it.

So perhaps av_buffer_ref(aFrame->buf[0]) in FFmpegVideoDecoder<LIBAV_VER>::GetVideoBuffer() and it's counterpart in release code may help you.

Flags: needinfo?(stransky)

Hm, this actually looks like our bug (and not ffmpeg one). shmem image (7f8210008d00) is allocated by us at mImageContainer->CreatePlanarYCbCrImage() so we re-use the image and pass that to ffmpeg again.

OTOH that's desired behavior as we want to recycle the images but we don't do that because of Bug 1750858.

I don't know what changed but mImageContainer->CreatePlanarYCbCrImage() always create a new image as mRecycleAllocator is null and mImageClient->AsImageClientSingle() return true in ImageContainer::CreatePlanarYCbCrImage().

I think we may be able to reproduce this bug if we hack YCbCrTextureClientAllocationHelper::IsCompatible() to always return true so the shm images will be recycled. Unfortunately the recycling does not work for me now (Comment 11).

shmem image (7f8210008d00) is allocated by us at mImageContainer->CreatePlanarYCbCrImage() so we re-use the image and pass that to ffmpeg again.

We only pass 7f8210008d00 to ffmpeg decoder in the first GetVideoBuffer() call, and we didn't pass that to ffmpeg again. This seems to me that ffmpeg still hold that buffer ref, and reuse it again.

OTOH that's desired behavior as we want to recycle the images

If we fix bug 1750858, then what we would have is recycling BufferTextureData, not recycling image.

I think we may be able to reproduce this bug if we hack YCbCrTextureClientAllocationHelper::IsCompatible() to always return true so the shm images will be recycled.

Probably not? I feel they are two different things. The problem here is that ffmpeg returns an decoded image which is stored in the buffer we allocated before, and then it starts using that buffer again without asking us. The recycle mechanism we expect is totally hidden to ffmpeg, they won't know if given buffer is newly created or used.

In addition, if you want to reuse shmem, the easiest way is to comment out these lines. Then it should be able to recycle shmem because they're all in same size (based on your previous testing)

So perhaps av_buffer_ref(aFrame->buf[0]) in FFmpegVideoDecoder<LIBAV_VER>::GetVideoBuffer() and it's counterpart in release code may help you.

oh btw for this, I guess using av_buffer_ref probably won't help? Because ReleaseVideoBufferWrapper is only called when the ref is zero, so we should suppose not to add additional ref on the buffer. In addition, for shmem texture, we don't have a way to detect if it's finished rendering by the compositor or not.

(In reply to Alastor Wu [:alwu] from comment #13)

In addition, for shmem texture, we don't have a way to detect if it's finished rendering by the compositor or not.

How VFW handles that? Or does it allocate new frames and doesn't recycle them?
I was expecting it works for shared shm surfaces...how we can reuse particular shm texture if we don't know whether is used or not?

btw. there's a global refcounter available for dmabuf surfaces on Linux based on eventfd - we track usage of every dmabuf surface across all processes and when the surface is released by all of them we can recycle it for new rendering. It may be possible to have something similar for Win/MacOS too.

Anyway, this one looks like some ffmpeg internal issue, looking into it.

(In reply to Alastor Wu [:alwu] from comment #13)

shmem image (7f8210008d00) is allocated by us at mImageContainer->CreatePlanarYCbCrImage() so we re-use the image and pass that to ffmpeg again.

We only pass 7f8210008d00 to ffmpeg decoder in the first GetVideoBuffer() call, and we didn't pass that to ffmpeg again. This seems to me that ffmpeg still hold that buffer ref, and reuse it again.

Yes, you're right. VP9 ffmpeg decoder caches the provided frames as reference images and stores them for future use. We need to check frame ref count and reuse (release) them only when they're not used by ffmpeg any more. That's also reason we don't get GetVideoBuffer() call.

Generally GetVideoBuffer() call is not strictly related to FFmpegVideoDecoder::CreateImage() calls. ffmpeg uses the buffers got by GetVideoBuffer() as it needs and returns decoded buffers out of order.

av_buffer_get_ref_count(mFrame->buf[0]) call can be used to get buffer refcount. I'm working on the patch.

Alastor, is there an expected user-facing impact from this bug? With 98 going to RC week, it would be good to understand what the possible severity of this issue would be in the wild.

Flags: needinfo?(alwu)

(In reply to Martin Stránský [:stransky] (ni? me) from comment #14)

How VFW handles that? Or does it allocate new frames and doesn't recycle them?
I was expecting it works for shared shm surfaces...how we can reuse particular shm texture if we don't know whether is used or not?

The shmem buffer is holding by TextureClient, so its lifecycle is determined by the texture client. When the compositor finishes rendering the specific texture client, it would release the refcount (details here) When its refcount drops to zero, then that texture client would be put back into the recycle pool for reuse. Therefore, if we can get a texture client from the recycle pool, it's guarantee that it's safe to be used.

av_buffer_get_ref_count(mFrame->buf[0]) call can be used to get buffer refcount. I'm working on the patch.

Thank you for working on that!

Alastor, is there an expected user-facing impact from this bug?

No, this feature is only enabled on Nightly. Other versions won't be affected.

Flags: needinfo?(alwu)

As we write to BufferTextureData where lock/unlock has limited effect on data integrity we can rearrange the texture->Lock()/Unlock() scheme.
Also ReleaseVideoBufferWrapper() give us clear info when the buffer can be reused - in the case above we haven't got ReleaseVideoBufferWrapper() call for image 1 (from comment 8) so it's still considered as owned by ffmpeg.

So we need to deal somehow with a scenario where a particular buffer is owned/used by ffmpeg but it's also used for rendering by gecko. From the ffmpeg code it looks like the scenario should be:

GetVideoBufferWrapper() - reserved by ffmpeg
ReleaseVideoBufferWrapper() - freed by ffmpeg (we're free to recycle it/delete)

avcodec_send_packet() - ffmpeg internal frame config changes (leads to possible GetVideoBufferWrapper()/ReleaseVideoBufferWrapper() calls).
avcodec_receive_frame(...,mFrame) - after this point a buffer returned by mFrame->buf[0] is complete and can be used by gecko until next avcodec_send_packet() call.

Don't hold TextureClient lock during decode as it's not relevant for BufferTextureData used
as TextureClient backend for software decoding.

The sequence is (from my understanding):

buffer A is get by ffmpeg
avcodec_receive_frame() -> gives us A

buffer B is get by ffmpeg
avcodec_receive_frame() -> gives us B
buffer B is released by ffmpeg

we know that A is captured by ffmpeg now but we already have it in our rendering queue

...
...

avcodec_receive_frame() -> gives us A (again)

In such scenario we only know that A is still hold by ffmpeg, but A is already used by our rendering queue while it can be also used by ffmpeg internally. We don't know when and how will be A released or returned by avcodec_receive_frame().
I guess the only fix for that is to decode some frames in advance and when we detect such scenario (a buffer is captured by ffmpeg) create a copy of the captured buffer and pass such copy to rendering queue.

Alastor, do you want me to file a follow up for that?

Pushed by stransky@redhat.com:
https://hg.mozilla.org/integration/autoland/rev/0e012004ae13
Unlock TextureClient in FFmpegVideoDecoder::GetVideoBuffer() r=alwu
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 99 Branch

Bugmon Analysis
Verified bug as fixed on rev mozilla-central 20220303162847-c6adb5930b38.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Status: RESOLVED → VERIFIED
Keywords: bugmon
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: