Avoid blocking all threads during AVIF decode - #9930
Conversation
0b3b624 to
3fd310f
Compare
fallenmi
left a comment
There was a problem hiding this comment.
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.
| return NULL; | ||
| } | ||
|
|
||
| Py_BEGIN_ALLOW_THREADS; |
There was a problem hiding this comment.
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().
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->bufferwhich 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
selfteston my machine, and I did not observe any other obvious issue using the decoder with this patch.