Skip to content

Preserve codec identity after failed open and allow safe retries - #2503

Closed
Robolightning wants to merge 1 commit into
PyAV-Org:masterfrom
StableKite:upstream/codec-failed-open-retry
Closed

Robolightning wants to merge 1 commit into
PyAV-Org:masterfrom
StableKite:upstream/codec-failed-open-retry

Conversation

@Robolightning

Copy link
Copy Markdown
Contributor

After avcodec_open2 rejects an invalid audio configuration, FFmpeg clears AVCodecContext.codec during cleanup. PyAV subsequently reads that pointer for codec identity and retry, so a second encode/open can crash the interpreter instead of raising an exception.

Keep the selected const AVCodec pointer independently of the mutable FFmpeg context and use it for open, static codec metadata/capabilities, parser selection and profile guards. The selected implementation is retained, including libopus, rather than looking up a default codec by ID.

The subprocess regression performs three actual AAC open failures, checks codec identity, restores the full valid audio configuration (FFmpeg also clears ch_layout), then encodes, muxes and decodes on the same context. A native crash or hang fails the parent test.

Validation: clean CPython 3.15t Windows/Linux builds of all extensions against the official FFmpeg 9.0.2-2 shared SDK; canonical suite 543 passed, 31 SDK capability skips on each platform, plus real libopus/Ogg retries and recovery, every native module import and lossless encoding across native threads with natural GIL OFF. Ruff, configured isort and mypy passed. These local native results apply to the matching runtime-source change on the StableKite branch; this PR preserves newer upstream changes including boolean property results and GIL release when freeing a codec context. Upstream CI is the separate validation of this PR on current master.

@Robolightning
Robolightning marked this pull request as ready for review November 2, 2026 17:39
@WyattBlue

Copy link
Copy Markdown
Member

Thanks for digging into this — the diagnosis here is spot on. I've opened #2506, which fixes the same root cause with a narrower approach: instead of a shadow _codec field read at each call site, it captures the codec pointer in open() and restores ptr.codec on failure, so every existing reader (codec, name, is_encoder, parse, _decode, …) stays correct without new guards. AVCodec structs are static globals, so holding the pointer is safe.

Since #2506 supersedes this, I'll close it in its favor — but credit to you for the root-cause analysis and repro. Happy to hear if you spot anything the pointer-restore approach misses.

@WyattBlue WyattBlue closed this Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants