Skip to content

Avoid blocking all threads when decoding AVIF image - #9930

Merged
radarhere merged 1 commit into
python-pillow:mainfrom
WillyJL:fix/avif-decode-blocking
Sep 4, 2026
Merged

Avoid blocking all threads when decoding AVIF image#9930
radarhere merged 1 commit into
python-pillow:mainfrom
WillyJL:fix/avif-decode-blocking

Conversation

@WillyJL

@WillyJL WillyJL commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Allow threads during AVIF decode as it can be a relatively lengthy operation and, from what I understood of the code, other Python threads executing in the meantime shouldn't cause issues (there's just decoder->buffer which is provided by Python code, but it is owned by this class for its whole lifetime, there should be no side effects from this change).

I develop a Dear ImGui based app where I use a thread to decode images into raw pixels to give to OpenGL, and with 4K AVIF images it stutters to the point of being unusable while decoding is happening (from 60 FPS normally to 1-2 FPS during decode), which can be up to 5-10 seconds in a row in some cases. With this patch, everything is back to being smooth.

Passed selftest on my machine, and I did not observe any other obvious issue using the decoder with this patch.

@WillyJL
WillyJL force-pushed the fix/avif-decode-blocking branch from 0b3b624 to 3fd310f Compare August 29, 2026 22:18
@radarhere radarhere added the Free-threading PEP 703 support label Aug 29, 2026

@fallenmi fallenmi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex-assisted review of exact head 3fd310f710a54146d8bf2bd030a1833371815099. I found one blocking same-decoder concurrency regression; details and the reproducer results are in the inline comment. The current upstream matrix is green, but it does not exercise concurrent get_frame() calls on one decoder instance. No other findings.

Comment thread src/_avif.c
return NULL;
}

Py_BEGIN_ALLOW_THREADS;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Releasing the GIL here allows two Python threads to enter avifDecoderNthImage() concurrently on the same AvifDecoder. That routine mutates shared diagnostics, imageIndex, tile/codec state, and decoder->image; a seek/reset in one call can destroy codec state while another call is decoding. I compiled exact base e41083f383c9cd3db95de52564cc0b6452313d4a, head 3fd310f710a54146d8bf2bd030a1833371815099, and GitHub merge e93744326a73993b88b667a6c597d678bd74182a against Pillow's pinned libavif 1.4.2 with dav1d 1.5.3 and the libyuv fast path, then ran the bundled star.avifs through one shared decoder from two threads. Across ten subprocess trials, base completed 10/10 with zero mismatches, while head crashed 10/10 (8 SIGSEGV, 2 SIGBUS) and merge crashed 10/10 (8 SIGSEGV, 2 SIGBUS). Please add per-decoder serialization covering avifDecoderNthImage() through consumption/conversion of the decoder-owned image, and add a concurrency regression test. Locking only avifDecoderNthImage() is insufficient because decoder->image remains shared through avifImageYUVToRGB().

@WillyJL WillyJL Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure how to go about this...

I don't see any cross-platform mutex primitives being used anywhere in Pillow, there is just MUTEX_LOCK() and MUTEX_UNLOCK() macros which however are based on PyMutex which is python 3.13+ exclusive, and these macros are only defined when running a python build with GIL entirely disabled, so these cannot be used to enforce a mutex in either a python version prior to 3.13 or in a python build with GIL enabled (most builds).

Am I supposed to make my own mutex primitives and ensure they'll work on a bunch of exotic platforms?

And, I don't see this being a concern anywhere else in Pillow either: WebP decoder does the same thing, releases the GIL (ImagingSectionEnter() which runs PyEval_SaveThread()) around a WebPAnimDecoderGetNext() which uses shared context (469db51), and libImaging's _decode() does the same with ImagingSectionEnter() which just releases the GIL for all other format decoders with seemingly no mutexes or thread safety...

Apologies if I'm missing something, I'm happy to make changes if they are necessary but currently I'm more confused than anything.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, you're not even from the Pillow team?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, you're not even from the Pillow team?

Don't worry we are here watching 😄 Thank you for working on this!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ran the bundled star.avifs through one shared decoder from two threads

Yeah, I'd say "don't do that" 😄

We could document it's not safe to share the internal decoder objects between threads, but someone who manually constructs _avif.AVIFDecoder() should already know they're playing with sharp objects...

@WillyJL
WillyJL marked this pull request as draft August 30, 2026 14:27
@WillyJL
WillyJL marked this pull request as ready for review September 3, 2026 20:42
@radarhere

Copy link
Copy Markdown
Member

@fdintino as the original author of AvifImagePlugin, would you like to give you seal of approval?

@fdintino fdintino left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me, good catch.

@radarhere radarhere changed the title Avoid blocking all threads during AVIF decode Avoid blocking all threads when decoding AVIF image Sep 4, 2026
@radarhere
radarhere merged commit 82e3362 into python-pillow:main Sep 4, 2026
57 checks passed
@radarhere

Copy link
Copy Markdown
Member

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Free-threading PEP 703 support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants