Keep a FilterLink's Graph alive and resolve a

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``.

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)

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

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:

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

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: ...

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

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,33 +12,20 @@ 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):
             if self.ptr == cctx.outputs[i]:
                 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
 

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: ...

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")