Declare _get_hw_format nogil - #2410
Closed
adrianrfreedman wants to merge 1 commit into
Closed
adrianrfreedman wants to merge 1 commit into
adrianrfreedman wants to merge 1 commit into
Conversation
_get_hw_format is installed as AVCodecContext.get_format, so libavcodec calls it from inside avcodec_send_packet, which runs under `with nogil`. Any Python object operation in the callback would execute without the GIL and crash the interpreter. It is already GIL-free in practice -- AVCodecPrivateData holds only plain C fields and the loop counter is typed -- but nothing enforces that. Declaring it nogil makes Cython reject a future change that reintroduces a Python object at compile time rather than at runtime. No functional change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
|
The commit message is so lousy, so I'd rather just add this myself. |
Author
This seems a bit unfair. Why not give me the opportunity to amend the commit? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_get_hw_formatis installed asAVCodecContext.get_format, so libavcodec calls it from insideavcodec_send_packet, which PyAV runs underwith nogil(av/codec/context.pyx). The callback therefore runs with no GIL held, and any Python object operation in it segfaults the interpreter.It is already GIL-free in practice.
AVCodecPrivateDataholds onlyAVPixelFormatandbint, and since the pure-Python-mode migration the loop counter is typed (i: cython.int = 0), so nothing gets boxed. Nothing enforces that though. Declaring the callbacknogilmakes Cython reject a future change that reintroduces a Python object at compile time, rather than at runtime as a crash.This is hardening, not a fix. The crash was real in 16.x, where the counter was untyped:
i += 1compiled toPyLong_FromLong/PyNumber_Add, so every hwaccel initialisation failure took the process down with SIGSEGV instead of falling back to software decoding. 17.0.0 fixed it as a side effect of the typing change rather than deliberately, which is why I think the constraint is worth pinning. Background in #2411.Verified on an L40S. 16.x segfaults on a stream NVDEC cannot decode (10-bit H.264), and this branch decodes the same file through the software fallback cleanly. Built with
python setup.py build_ext --inplaceagainst the vendored ffmpeg fromscripts/ffmpeg-latest.json(libavcodec 63.1.101).No functional change.