Build filter pads on access instead of caching them

Author: WyattBlue

CHANGELOG.rst

@@ -37,6 +37,7 @@ Fixes:
 - Reading :attr:`.Frame.side_data` no longer holds the frame in a reference cycle, which kept its picture buffer alive until gc ran. By :gh-user:`WyattBlue`.
 - :class:`.MotionVectors` no longer memoizes the vectors it hands out, a cache that never evicted and put every one of them in a reference cycle with the side data, retaining 1640 objects for a single 640x360 frame. By :gh-user:`WyattBlue`.
 - :attr:`.Stream.index_entries` is now a view built on access rather than an object allocated for every stream whether or not anything reads it, which also takes the stream out of a reference cycle with it. Fixes (:issue:`2425`) by :gh-user:`WyattBlue`.
+- :class:`.Filter` and :class:`.FilterContext` pads are now built on access rather than cached, which takes a filter out of a reference cycle with its pads and makes a pad compare equal by what it denotes. By :gh-user:`WyattBlue`.
 
 
 v19.0.0

av/filter/context.pxd

@@ -8,8 +8,6 @@ cdef class FilterContext:
     cdef lib.AVFilterContext *ptr
     cdef readonly Graph graph
     cdef readonly Filter filter
-    cdef tuple _inputs
-    cdef tuple _outputs
     cdef bint inited
     cdef unsigned char _kind
 

av/filter/context.py

@@ -64,19 +64,11 @@ def name(self):
 
     @property
     def inputs(self):
-        if self._inputs is None:
-            self._inputs = alloc_filter_pads(
-                self.filter, self.ptr.input_pads, True, self
-            )
-        return self._inputs
+        return alloc_filter_pads(self.filter, self.ptr.input_pads, True, self)
 
     @property
     def outputs(self):
-        if self._outputs is None:
-            self._outputs = alloc_filter_pads(
-                self.filter, self.ptr.output_pads, False, self
-            )
-        return self._outputs
+        return alloc_filter_pads(self.filter, self.ptr.output_pads, False, self)
 
     def init(self, args=None, **kwargs):
         if self.inited:
@@ -126,13 +118,14 @@ def push(self, frame: Frame | None):
             return
 
         # Delegate to the input.
-        if len(self.inputs) != 1:
+        inputs = self.inputs
+        if len(inputs) != 1:
             raise ValueError(
-                f"cannot delegate push without single input; found {len(self.inputs)}"
+                f"cannot delegate push without single input; found {len(inputs)}"
             )
-        if not self.inputs[0].link:
+        if not inputs[0].link:
             raise ValueError("cannot delegate push without linked input")
-        self.inputs[0].linked.context.push(frame)
+        inputs[0].linked.context.push(frame)
 
     def pull(self):
         frame: Frame
@@ -143,13 +136,14 @@ def pull(self):
             frame = alloc_audio_frame()
         else:
             # Delegate to the output.
-            if len(self.outputs) != 1:
+            outputs = self.outputs
+            if len(outputs) != 1:
                 raise ValueError(
-                    f"cannot delegate pull without single output; found {len(self.outputs)}"
+                    f"cannot delegate pull without single output; found {len(outputs)}"
                 )
-            if not self.outputs[0].link:
+            if not outputs[0].link:
                 raise ValueError("cannot delegate pull without linked output")
-            return self.outputs[0].linked.context.pull()
+            return outputs[0].linked.context.pull()
 
         self.graph.configure()
 

av/filter/filter.pxd

@@ -3,8 +3,6 @@ cimport libav as lib
 
 cdef class Filter:
     cdef const lib.AVFilter *ptr
-    cdef tuple _inputs
-    cdef tuple _outputs
 
 
 cdef Filter wrap_filter(const lib.AVFilter *ptr)

av/filter/filter.py

@@ -39,15 +39,11 @@ def flags(self):
 
     @property
     def inputs(self):
-        if self._inputs is None:
-            self._inputs = alloc_filter_pads(self, self.ptr.inputs, True)
-        return self._inputs
+        return alloc_filter_pads(self, self.ptr.inputs, True)
 
     @property
     def outputs(self):
-        if self._outputs is None:
-            self._outputs = alloc_filter_pads(self, self.ptr.outputs, False)
-        return self._outputs
+        return alloc_filter_pads(self, self.ptr.outputs, False)
 
 
 @cython.cfunc

av/filter/link.py

@@ -1,6 +1,7 @@
 import cython
 import cython.cimports.libav as lib
 from cython.cimports.av.filter.graph import Graph
+from cython.cimports.libc.stdint import uintptr_t
 
 _cinit_sentinel = cython.declare(object, object())
 
@@ -65,6 +66,34 @@ def __repr__(self):
             f"<av.FilterPad {_filter}.{_io}[{self.index}]: {self.name} ({self.type})>"
         )
 
+    def __eq__(self, other):
+        """Two pads are equal when they denote the same pad.
+
+        Pads are built on access rather than cached, so ``is`` compares the
+        wrappers while this compares what they point at.
+        """
+        if not isinstance(other, FilterPad):
+            return NotImplemented
+
+        o: FilterPad = other
+        if self.base_ptr != o.base_ptr or self.index != o.index:
+            return False
+        if self.is_input != o.is_input:
+            return False
+        if (self.context is None) != (o.context is None):
+            return False
+        return self.context is None or self.context.ptr == o.context.ptr
+
+    def __hash__(self):
+        return hash(
+            (
+                cython.cast(uintptr_t, self.base_ptr),
+                self.index,
+                self.is_input,
+                0 if self.context is None else cython.cast(uintptr_t, self.context.ptr),
+            )
+        )
+
     @property
     def is_output(self):
         return not self.is_input

av/filter/link.pyi

@@ -7,6 +7,8 @@ class FilterPad:
     context: FilterContext
     is_input: bool
     index: int
+    def __eq__(self, other: object) -> bool: ...
+    def __hash__(self) -> int: ...
     @property
     def is_output(self) -> bool: ...
     @property

tests/test_filters.py

@@ -74,8 +74,8 @@ def test_generator_graph(self):
         lutrgb.link_to(sink)
 
         # pads and links
-        assert src.outputs[0].link.output is lutrgb.inputs[0]
-        assert lutrgb.inputs[0].link.input is src.outputs[0]
+        assert src.outputs[0].link.output == lutrgb.inputs[0]
+        assert lutrgb.inputs[0].link.input == src.outputs[0]
 
         frame = sink.pull()
         assert isinstance(frame, VideoFrame)