diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6b92cc1e0..62a10684e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -39,6 +39,7 @@ Major: Fixes: - Frames returned by flushing a codec context directly (``CodecContext.decode()`` with no packet) now carry the stream's ``time_base`` instead of ``None``. +- ``VideoFrame.reformat()`` (and so ``to_ndarray(format=...)``, ``to_rgb()``, ``to_image()``) now shares one ``SwsContext`` per thread instead of allocating one per frame. FFmpeg 8's swscale retains megabytes of graph state per context, which showed up as large RSS growth when many frames were alive at once. 18.X and Below diff --git a/av/video/frame.pxd b/av/video/frame.pxd index ad693c722..947bae57a 100644 --- a/av/video/frame.pxd +++ b/av/video/frame.pxd @@ -20,7 +20,6 @@ cdef class CudaContext: cdef class VideoFrame(Frame): cdef CudaContext _cuda_ctx - cdef VideoReformatter reformatter cdef readonly VideoFormat format cdef readonly int _device_id cdef void _init(self, lib.AVPixelFormat format, unsigned int width, unsigned int height) diff --git a/av/video/frame.py b/av/video/frame.py index d73fd68bc..a1194f247 100644 --- a/av/video/frame.py +++ b/av/video/frame.py @@ -1,4 +1,5 @@ import sys +import threading from enum import IntEnum import cython @@ -19,6 +20,9 @@ from cython.cimports.hwcontext_cuda import AVCUDADeviceContext, CUstream from cython.cimports.libc.stdint import int64_t, uint8_t, uintptr_t +# Holds the VideoReformatter shared by all frames converted on this thread. +_thread_local = threading.local() + @cython.cfunc @cython.nogil @@ -759,9 +763,14 @@ def reformat(self, *args, **kwargs): .. seealso:: :meth:`.VideoReformatter.reformat` for arguments. """ - if not self.reformatter: - self.reformatter = VideoReformatter() - return self.reformatter.reformat(self, *args, **kwargs) + # One SwsContext per thread rather than per frame: FFmpeg 8's swscale + # retains ~15MB of graph state for a context's lifetime, so a context + # per live frame is far too expensive. See #2320. + reformatter: VideoReformatter = getattr(_thread_local, "reformatter", None) + if reformatter is None: + reformatter = VideoReformatter() + _thread_local.reformatter = reformatter + return reformatter.reformat(self, *args, **kwargs) def to_rgb(self, **kwargs): """Get an RGB version of this frame. diff --git a/tests/test_videoframe.py b/tests/test_videoframe.py index 36a1d5386..3ed417da7 100644 --- a/tests/test_videoframe.py +++ b/tests/test_videoframe.py @@ -1347,6 +1347,33 @@ def test_reformat_identity() -> None: assert frame1 is frame2 +def test_reformat_shares_one_context_per_thread() -> None: + # An SwsContext retains megabytes of graph state, so frames must not each + # hold their own. See #2320. + from threading import Thread + + from av.video.frame import _thread_local # type: ignore[attr-defined] + + for _ in range(3): + VideoFrame(640, 480, "yuv420p").reformat(format="rgb24") + mine = _thread_local.reformatter + assert mine is not None + + theirs = [] + + def other() -> None: + VideoFrame(640, 480, "yuv420p").reformat(format="rgb24") + theirs.append(_thread_local.reformatter) + + thread = Thread(target=other) + thread.start() + thread.join() + + assert theirs, "worker thread failed" + assert _thread_local.reformatter is mine + assert theirs[0] is not mine + + def test_reformat_colorspace() -> None: frame = VideoFrame(640, 480, "rgb24") frame.reformat(src_colorspace=None, dst_colorspace="smpte240m")