From fc3f63c0a205010f0df63b2cde401e354aa3799e Mon Sep 17 00:00:00 2001 From: WyattBlue Date: Tue, 7 Apr 2026 11:31:01 -0400 Subject: [PATCH 1/2] Fix crash with container closing with GC --- .github/workflows/smoke.yml | 1 - av/container/output.py | 7 ++++++- tests/test_open.py | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index 02de77f0c..6fb33ad02 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -38,7 +38,6 @@ jobs: - {os: ubuntu-24.04, python: "3.12", ffmpeg: "8.0.1", extras: true} - {os: ubuntu-24.04, python: "3.10", ffmpeg: "8.0.1"} - {os: ubuntu-24.04, python: "3.13", ffmpeg: "8.1"} - - {os: ubuntu-24.04, python: "pypy3.11", ffmpeg: "8.0.1"} - {os: macos-14, python: "3.11", ffmpeg: "8.0.1"} - {os: macos-14, python: "3.14", ffmpeg: "8.1"} diff --git a/av/container/output.py b/av/container/output.py index b2a643a7a..1e9ac3201 100644 --- a/av/container/output.py +++ b/av/container/output.py @@ -35,8 +35,13 @@ def __cinit__(self, *args, **kwargs): with cython.nogil: self.packet_ptr = lib.av_packet_alloc() + def __del__(self): + try: + close_output(self) + except Exception: + pass + def __dealloc__(self): - close_output(self) with cython.nogil: lib.av_packet_free(cython.address(self.packet_ptr)) diff --git a/tests/test_open.py b/tests/test_open.py index 9341e1f56..60cb60072 100644 --- a/tests/test_open.py +++ b/tests/test_open.py @@ -1,3 +1,5 @@ +import gc +import io from pathlib import Path import av @@ -35,3 +37,19 @@ def test_str_output() -> None: container = av.open(path, "w") assert type(container) is av.container.OutputContainer + + +def _container_no_close() -> None: + buf = io.BytesIO() + container = av.open(buf, mode="w", format="mp4") + stream = container.add_stream("mpeg4", rate=24) + stream.width = 320 + stream.height = 240 + stream.pix_fmt = "yuv420p" + container.start_encoding() + + +def test_container_no_close() -> None: + # Do not close so that container is freed through GC. + _container_no_close() + gc.collect() From ac944be625acbaa1891c3549502e5916b5dc4158 Mon Sep 17 00:00:00 2001 From: WyattBlue Date: Tue, 7 Apr 2026 23:23:38 -0400 Subject: [PATCH 2/2] Save 12 bytes for each OutputContainer --- av/container/core.pxd | 5 ++--- av/container/core.py | 18 +++++++++++------- av/container/core.pyi | 4 ++-- av/container/input.pxd | 1 - av/container/input.py | 6 +++--- av/container/output.pxd | 3 --- av/container/output.py | 10 +++++----- 7 files changed, 23 insertions(+), 24 deletions(-) diff --git a/av/container/core.pxd b/av/container/core.pxd index 1067b678d..7cba26407 100644 --- a/av/container/core.pxd +++ b/av/container/core.pxd @@ -1,4 +1,5 @@ cimport libav as lib +from libc.stdint cimport uint8_t from av.codec.hwaccel cimport HWAccel from av.container.pyio cimport PyIOFile @@ -14,16 +15,13 @@ ctypedef struct timeout_info: cdef class Container: - cdef readonly bint writeable cdef lib.AVFormatContext *ptr - cdef readonly object name cdef readonly str metadata_encoding cdef readonly str metadata_errors cdef readonly PyIOFile file cdef int buffer_size - cdef bint input_was_opened cdef readonly object io_open cdef readonly object open_files @@ -39,6 +37,7 @@ cdef class Container: cdef readonly dict metadata # Private API. + cdef uint8_t _myflag # enum: writeable, input_was_opened, started, done cdef _assert_open(self) cdef int err_check(self, int value) except -1 diff --git a/av/container/core.py b/av/container/core.py index e8aa51854..ab6c889b7 100755 --- a/av/container/core.py +++ b/av/container/core.py @@ -246,8 +246,8 @@ def __cinit__( if sentinel is not _cinit_sentinel: raise RuntimeError("cannot construct base Container") - self.writeable = isinstance(self, OutputContainer) - if not self.writeable and not isinstance(self, InputContainer): + writeable: cython.bint = isinstance(self, OutputContainer) + if not writeable and not isinstance(self, InputContainer): raise RuntimeError("Container cannot be directly extended.") if isinstance(file_, str): @@ -276,13 +276,13 @@ def __cinit__( format_name, acodec = format_name.split(":") self.format = ContainerFormat(format_name) - self.input_was_opened = False res: cython.int name_obj: bytes = os.fsencode(self.name) name: cython.p_char = name_obj ofmt: cython.pointer[cython.const[lib.AVOutputFormat]] - if self.writeable: + if writeable: + self._myflag |= 1 # enum.writeable = True ofmt = ( self.format.optr if self.format @@ -320,7 +320,7 @@ def __cinit__( # Setup Python IO. self.open_files = {} if not isinstance(file_, basestring): - self.file = PyIOFile(file_, buffer_size, self.writeable) + self.file = PyIOFile(file_, buffer_size, writeable) self.ptr.pb = self.file.iocontext if io_open is not None: @@ -330,7 +330,7 @@ def __cinit__( ifmt: cython.pointer[cython.const[lib.AVInputFormat]] c_options: Dictionary - if not self.writeable: + if not writeable: ifmt = self.format.iptr if self.format else cython.NULL c_options = Dictionary(self.options, self.container_options) @@ -342,7 +342,7 @@ def __cinit__( ) self.set_timeout(None) self.err_check(res) - self.input_was_opened = True + self._myflag |= 2 # enum.input_was_opened = True if format_name is None: self.format = build_container_format(self.ptr.iformat, self.ptr.oformat) @@ -400,6 +400,10 @@ def flags(self, value: cython.int): self._assert_open() self.ptr.flags = value + @property + def input_was_opened(self): + return self._myflag & 2 + def chapters(self): self._assert_open() result: list = [] diff --git a/av/container/core.pyi b/av/container/core.pyi index cb891cbdc..f62b6c813 100644 --- a/av/container/core.pyi +++ b/av/container/core.pyi @@ -75,13 +75,11 @@ class Chapter(TypedDict): metadata: dict[str, str] class Container: - writeable: bool name: str metadata_encoding: str metadata_errors: str file: Any buffer_size: int - input_was_opened: bool io_open: Any open_files: Any format: ContainerFormat @@ -100,6 +98,8 @@ class Container: exc_val: BaseException | None, exc_tb: TracebackType | None, ) -> None: ... + @property + def input_was_opened(self) -> bool: ... def close(self) -> None: ... def chapters(self) -> list[Chapter]: ... def set_chapters(self, chapters: list[Chapter]) -> None: ... diff --git a/av/container/input.pxd b/av/container/input.pxd index 8c369d8ad..2e65025b9 100644 --- a/av/container/input.pxd +++ b/av/container/input.pxd @@ -5,5 +5,4 @@ from av.stream cimport Stream cdef class InputContainer(Container): - cdef flush_buffers(self) diff --git a/av/container/input.py b/av/container/input.py index 304fe3e3f..97037457f 100644 --- a/av/container/input.py +++ b/av/container/input.py @@ -13,11 +13,11 @@ @cython.cfunc def close_input(self: InputContainer): self.streams = StreamContainer() - if self.input_was_opened: - with cython.nogil: + with cython.nogil: + if self._myflag & 2: # This causes `self.ptr` to be set to NULL. lib.avformat_close_input(cython.address(self.ptr)) - self.input_was_opened = False + self._myflag &= ~2 # enum.input_was_opened = False @cython.cclass diff --git a/av/container/output.pxd b/av/container/output.pxd index 51d3f308e..71edc3bf2 100644 --- a/av/container/output.pxd +++ b/av/container/output.pxd @@ -5,8 +5,5 @@ from av.stream cimport Stream cdef class OutputContainer(Container): - cdef bint _started - cdef bint _done cdef lib.AVPacket *packet_ptr - cpdef start_encoding(self) diff --git a/av/container/output.py b/av/container/output.py index 1e9ac3201..61a370c06 100644 --- a/av/container/output.py +++ b/av/container/output.py @@ -15,16 +15,16 @@ @cython.cfunc def close_output(self: OutputContainer): self.streams = StreamContainer() - if self._started and not self._done: + if self._myflag & 12 == 4: # enum.started and not enum.done # We must only ever call av_write_trailer *once*, otherwise we get a # segmentation fault. Therefore no matter whether it succeeds or not - # we must absolutely set self._done. + # we must absolutely set enum.done. try: self.err_check(lib.av_write_trailer(self.ptr)) finally: if self.file is None and not (self.ptr.oformat.flags & lib.AVFMT_NOFILE): lib.avio_closep(cython.address(self.ptr.pb)) - self._done = True + self._myflag |= 8 # enum.done = True @cython.cclass @@ -431,7 +431,7 @@ def add_data_stream(self, codec_name=None, options: dict | None = None): @cython.ccall def start_encoding(self): """Write the file header! Called automatically.""" - if self._started: + if self._myflag & 4: # started return # TODO: This does NOT handle options coming from 3 sources. @@ -491,7 +491,7 @@ def start_encoding(self): log = logging.getLogger(__name__) log.warning("Some options were not used: %s" % unused_options) - self._started = True + self._myflag |= 4 @property def supported_codecs(self):