diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ffb2d22e2..274fa159b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -47,6 +47,8 @@ Features: Fixes: - ``av.dump_codecs()`` no longer drops the canonical names ``h264``, ``hevc``, ``av1``, ``dirac``, and ``ilbc``, each of which was overwritten by the row of whichever encoder it resolved to. +- ``FilterLink.input`` and ``FilterLink.output`` now follow the filters the graph auto-inserts while configuring. They cached the pad they first resolved, so reading one before ``Graph.configure()`` reported the filter the link no longer pointed at. +- Fix a segfault when a ``FilterLink`` outlives its ``Graph``. It held the graph by weak reference and dereferenced ``AVFilterLink`` before consulting it, so ``link.input`` and ``link.output`` read freed memory. It now holds the graph, as a ``FilterContext`` already did. - Reading a :class:`.Stream` or its :attr:`~av.stream.Stream.index_entries` after the container is closed now raises instead of reading freed memory, since ``avformat_close_input()`` frees the underlying ``AVStream``. Holding an ``index_entries`` also keeps its container alive, and an :class:`.IndexEntry` is a copy, so it stays readable after the close and is unaffected by the demuxer reallocating the index. - Attaching one object to the ``opaque`` of more than one frame or packet no longer loses it. The objects were keyed by ``id()``, so every holder shared an entry and whichever was freed first took it away from the rest. - ``Frame.side_data`` now satisfies the ``Mapping`` protocol it advertises: iteration yields :class:`~av.sidedata.sidedata.Type` keys, so ``items()``, ``keys()``, and ``values()`` work instead of raising ``KeyError``. Values remain reachable positionally by an integer or, for the first time, a slice. Its type stub was a ``TypedDict`` with a single literal key, and is now ``SideDataContainer``. diff --git a/av/audio/resampler.pxd b/av/audio/resampler.pxd index 6cb6c5c1d..73b928185 100644 --- a/av/audio/resampler.pxd +++ b/av/audio/resampler.pxd @@ -5,15 +5,13 @@ from av.filter.graph cimport Graph cdef class AudioResampler: - cdef readonly bint is_passthrough cdef AudioFrame template - - # Destination descriptors + cdef Graph graph cdef readonly AudioFormat format cdef readonly AudioLayout layout + cdef readonly dict options cdef readonly int rate cdef readonly unsigned int frame_size - cdef readonly dict options + cdef readonly bint is_passthrough - cdef Graph graph cpdef list[AudioFrame] resample(self, AudioFrame) diff --git a/av/filter/context.pxd b/av/filter/context.pxd index d67e7e1e7..654d8e0a6 100644 --- a/av/filter/context.pxd +++ b/av/filter/context.pxd @@ -6,7 +6,7 @@ from av.filter.graph cimport Graph cdef class FilterContext: cdef lib.AVFilterContext *ptr - cdef readonly object _graph + cdef readonly Graph graph cdef readonly Filter filter cdef object _inputs cdef object _outputs diff --git a/av/filter/context.py b/av/filter/context.py index edfb93617..cc72795ae 100644 --- a/av/filter/context.py +++ b/av/filter/context.py @@ -21,7 +21,7 @@ def wrap_filter_context( graph: Graph, filter: Filter, ptr: cython.pointer[lib.AVFilterContext] ) -> FilterContext: self: FilterContext = FilterContext(_cinit_sentinel) - self._graph = graph + self.graph = graph self.filter = filter self.ptr = ptr @@ -106,17 +106,13 @@ def link_to( ): err_check(lib.avfilter_link(self.ptr, output_idx, input_.ptr, input_idx)) - @property - def graph(self): - return self._graph - def push(self, frame: Frame | None): res: cython.int # av_buffersrc_write_frame() dereferences graph internals that only # exist after configuration; pushing first would segfault. if self._kind == _KIND_SOURCE or frame is None: - self._graph.configure() + self.graph.configure() if frame is None: with cython.nogil: diff --git a/av/filter/context.pyi b/av/filter/context.pyi index 2350019f3..8c8462f95 100644 --- a/av/filter/context.pyi +++ b/av/filter/context.pyi @@ -1,9 +1,14 @@ from av.filter import Graph +from av.filter.link import FilterContextPad from av.frame import Frame class FilterContext: name: str | None + @property + def inputs(self) -> tuple[FilterContextPad, ...]: ... + @property + def outputs(self) -> tuple[FilterContextPad, ...]: ... def init(self, args: str | None = None, **kwargs: str | None) -> None: ... def link_to( self, input_: FilterContext, output_idx: int = 0, input_idx: int = 0 diff --git a/av/filter/graph.pyi b/av/filter/graph.pyi index de28e5e30..e37e91674 100644 --- a/av/filter/graph.pyi +++ b/av/filter/graph.pyi @@ -28,7 +28,7 @@ class Graph: template: VideoStream | None = None, width: int | None = None, height: int | None = None, - format: VideoFormat | None = None, + format: VideoFormat | str | None = None, name: str | None = None, time_base: AVRational | Fraction | None = None, ) -> FilterContext: ... diff --git a/av/filter/link.pxd b/av/filter/link.pxd index 3a66b2879..4fdb80924 100644 --- a/av/filter/link.pxd +++ b/av/filter/link.pxd @@ -3,27 +3,22 @@ cimport libav as lib from av.filter.context cimport FilterContext from av.filter.filter cimport Filter from av.filter.graph cimport Graph -from av.filter.link cimport FilterContextPad, FilterLink cdef class FilterLink: - cdef object _graph + cdef readonly Graph graph cdef lib.AVFilterLink *ptr - cdef FilterContextPad _input - cdef FilterContextPad _output - cdef FilterLink wrap_filter_link(Graph graph, lib.AVFilterLink *ptr) cdef class FilterPad: cdef readonly Filter filter cdef readonly FilterContext context + cdef const lib.AVFilterPad *base_ptr cdef readonly bint is_input cdef readonly int index - cdef const lib.AVFilterPad *base_ptr - cdef class FilterContextPad(FilterPad): cdef FilterLink _link diff --git a/av/filter/link.py b/av/filter/link.py index 6cc3b023a..199e8ff7b 100644 --- a/av/filter/link.py +++ b/av/filter/link.py @@ -1,5 +1,3 @@ -import weakref - import cython import cython.cimports.libav as lib from cython.cimports.av.filter.graph import Graph @@ -14,17 +12,8 @@ def __cinit__(self, sentinel): if sentinel is not _cinit_sentinel: raise RuntimeError("cannot instantiate FilterLink") - @property - def graph(self) -> Graph: - if g := self._graph(): - return g - else: - raise RuntimeError("graph is unallocated") - @property def input(self): - if self._input: - return self._input cctx: cython.pointer[lib.AVFilterContext] = self.ptr.src i: cython.Py_ssize_t for i in range(cctx.nb_outputs): @@ -32,15 +21,11 @@ def input(self): break else: # nobreak raise RuntimeError("could not find link in context") - graph: Graph = self.graph - ctx = graph._context_by_ptr[cython.cast(cython.size_t, cctx)] - self._input = ctx.outputs[i] - return self._input + ctx = self.graph._context_by_ptr[cython.cast(cython.size_t, cctx)] + return ctx.outputs[i] @property def output(self): - if self._output: - return self._output cctx: cython.pointer[lib.AVFilterContext] = self.ptr.dst i: cython.Py_ssize_t for i in range(cctx.nb_inputs): @@ -55,14 +40,13 @@ def output(self): raise RuntimeError( "could not find context in graph", (cctx.name, cctx.filter.name) ) - self._output = ctx.inputs[i] - return self._output + return ctx.inputs[i] @cython.cfunc def wrap_filter_link(graph: Graph, ptr: cython.pointer[lib.AVFilterLink]) -> FilterLink: link: FilterLink = FilterLink(_cinit_sentinel) - link._graph = weakref.ref(graph) + link.graph = graph link.ptr = ptr return link diff --git a/av/filter/link.pyi b/av/filter/link.pyi index 9d199a272..bfebe48c0 100644 --- a/av/filter/link.pyi +++ b/av/filter/link.pyi @@ -1,2 +1,29 @@ +from av.filter.context import FilterContext +from av.filter.filter import Filter +from av.filter.graph import Graph + +class FilterPad: + filter: Filter + context: FilterContext + is_input: bool + index: int + @property + def is_output(self) -> bool: ... + @property + def name(self) -> str: ... + @property + def type(self) -> str: ... + +class FilterContextPad(FilterPad): + @property + def link(self) -> FilterLink | None: ... + @property + def linked(self) -> FilterContextPad | None: ... + class FilterLink: - pass + @property + def graph(self) -> Graph: ... + @property + def input(self) -> FilterContextPad: ... + @property + def output(self) -> FilterContextPad: ... diff --git a/tests/test_filters.py b/tests/test_filters.py index 565df3507..835d95b75 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -4,9 +4,10 @@ import numpy as np import av -from av import AudioFrame, VideoFrame +from av import AudioFrame, AVRational, VideoFrame from av.audio.frame import format_dtypes from av.filter import Filter, Graph +from av.filter.context import FilterContext from .common import TestCase, has_pillow @@ -73,6 +74,54 @@ def test_generator_graph(self): if has_pillow: frame.to_image().save(self.sandboxed("mandelbrot2.png")) + def test_link_keeps_graph_alive(self) -> None: + import gc + + graph = Graph() + src = graph.add("testsrc") + sink = graph.add("buffersink") + src.link_to(sink) + graph.configure() + + link = src.outputs[0].link + assert link is not None + del graph, src, sink + gc.collect() + + # The link points into the graph's memory, so holding it must hold the + # graph, exactly as holding a filter context does. + assert link.graph is not None + assert link.input.context.name == "testsrc" + assert link.output.context.name == "buffersink" + + def test_link_pads_follow_auto_inserted_filters(self) -> None: + # Configuring a graph splices a converter in where formats disagree, + # which re-points the link. Reading a pad early must not pin the answer. + def build() -> tuple[Graph, FilterContext]: + graph = Graph() + src = graph.add_buffer( + width=64, height=64, format="yuv420p", time_base=AVRational(1, 24) + ) + lutrgb = graph.add("lutrgb", "r=maxval-val:g=maxval-val:b=maxval-val") + sink = graph.add("buffersink") + src.link_to(lutrgb) + lutrgb.link_to(sink) + return graph, src + + early_graph, early_src = build() + early_link = early_src.outputs[0].link + assert early_link is not None + early_link.output # would have been cached + early_graph.configure() + + late_graph, late_src = build() + late_graph.configure() + late_link = late_src.outputs[0].link + assert late_link is not None + + assert early_link.output.context.name == late_link.output.context.name + assert early_link.input.context.name == late_link.input.context.name + def test_auto_find_sink(self) -> None: graph = Graph() src = graph.add("testsrc")