convert: Make sure no real closures exist.

That should prevent reference cycles. Hopefully.

Author: cid-chan

vsengine/_helpers.py

@@ -49,12 +49,12 @@ def wrap_variable_size(
     if node.format is not None and node.width != 0 and node.height != 0:
         return func(node)
 
-    _node_cache = {}
     def _do_resize(f: vs.VideoFrame) -> vs.VideoNode:
         # Resize the node to make them assume a specific format.
         # As the node should aready have this format, this should be a no-op.
         return func(node.resize.Point(format=f.format, width=f.width, height=f.height))
 
+    _node_cache = {}
     def _assume_format(n: int, f: vs.VideoFrame) -> vs.VideoNode:
         nonlocal _node_cache
         selector = (int(f.format), f.width, f.height)
@@ -75,6 +75,10 @@ def _assume_format(n: int, f: vs.VideoFrame) -> vs.VideoNode:
 
         return wrapped
 
-    evaled = vs.core.std.FrameEval(node, _assume_format, node)
-    return evaled.resize.Point(format=force_assumed_format)
+    # This clip must not become part of the closure,
+    # or otherwise we risk cyclic references.
+    return (
+        node.std.FrameEval(_assume_format, [node], [node])
+        .resize.Point(format=force_assumed_format)
+    )
 

vsengine/convert.py

@@ -53,6 +53,69 @@ def yuv_heuristic(width: int, height: int) -> t.Mapping[str, str]:
     return result
 
 
+# Move this function out of the closure to avoid capturing clip.
+def _convert_yuv(
+        c: vs.VideoNode,
+        *,
+        core: vs.Core,
+        real_rgb24: vs.VideoFormat,
+        default_args: t.Dict[str, t.Any],
+        scaler: t.Union[str, t.Callable[..., vs.VideoNode]]
+):
+    # We make yuv_heuristic not configurable so the heuristic
+    # will be shared across projects.
+    #
+    # In my opinion, this is a quirk that should be shared.
+
+    args = {
+        **yuv_heuristic(c.width, c.height),
+        **default_args
+    }
+
+    if c.format.subsampling_w != 0 or c.format.subsampling_h != 0:
+        # To be clear, scaler should always be a string.
+        # Being able to provide a callable just makes testing args easier.
+        resizer = getattr(core.resize, scaler) if isinstance(scaler, str) else scaler
+    else:
+        # In this case we only do cs transforms, point resize is more then enough.
+        resizer = core.resize.Point
+
+    # Keep bitdepth so we can dither futher down in the RGB part.
+    return resizer(
+        c,
+        format=real_rgb24.replace(bits_per_sample=c.format.bits_per_sample),
+        **args
+    )
+
+
+# Move this function out of the closure to avoid capturing clip.
+def _actually_resize(
+        c: vs.VideoNode,
+        *,
+        core: vs.Core,
+        convert_yuv: t.Callable[[vs.VideoNode], vs.VideoNode],
+        target_rgb: vs.VideoFormat
+) -> vs.VideoNode:
+    # Converting to YUV is a little bit more complicated,
+    # so I extracted it to its own function.
+    if c.format.color_family == vs.YUV:
+        c = convert_yuv(c)
+
+    # Defaulting prefer_props to True makes resizing choke
+    # on GRAY clips.
+    if c.format == vs.GRAY:
+        c = c.std.RemoveFrameProps("_Matrix")
+
+    # Actually perform the format conversion on a non-subsampled clip.
+    if c.format.color_family != vs.RGB or c.format.bits_per_sample != target_rgb.bits_per_sample:
+        c = core.resize.Point(
+            c,
+            format=target_rgb
+        )
+
+    return c
+
+
 def to_rgb(
         clip: vs.VideoNode,
         env: t.Optional[EnvironmentTypes] = None,
@@ -61,7 +124,7 @@ def to_rgb(
         bits_per_sample: int = 8,
 
         # Input: YUV
-        scaler: t.Union[str, t.Callable[..., vs.VideoNode]] = "Spline36",
+        scaler: t.Union[str, t.Callable[..., vs.VideoNode]] = "Bicubic",
         default_matrix: t.Optional[str] = None,
         default_transfer: t.Optional[str] = None,
         default_primaries: t.Optional[str] = None,
@@ -78,59 +141,46 @@ def to_rgb(
     :param default_*: Manually override the defaults predicted by the heuristics.
     :param yuv_heuristic: The heuristic function that takes the frame size and returns a set of yuv-metadata. (For test purposes)
     """
+
+    # This function does a lot.
+    # This is why there are so many comments.
+
+    default_args = {}
+    if default_matrix is not None:
+        default_args["matrix_in_s"] = default_matrix
+    if default_transfer is not None:
+        default_args["transfer_in_s"] = default_transfer
+    if default_primaries is not None:
+        default_args["primaries_in_s"] = default_primaries
+    if default_range is not None:
+        default_args["range_in_s"] = default_range
+    if default_chromaloc is not None:
+        default_args["chromaloc_in_s"] = default_chromaloc
+
     with use_inline("to_rgb", env):
         core = vs.core.core
         real_rgb24 = core.get_video_format(vs.RGB24)
         target_rgb = real_rgb24.replace(bits_per_sample=bits_per_sample)
 
-    def _convert_yuv(c: vs.VideoNode):
-        # We make yuv_heuristic not configurable so the heuristic
-        # 
-        args = {
-            **yuv_heuristic(c.width, c.height)
-        }
-
-        # Override heuristics with default values if specified.
-        if default_matrix is not None:
-            args["matrix_in_s"] = default_matrix
-        if default_transfer is not None:
-            args["transfer_in_s"] = default_transfer
-        if default_primaries is not None:
-            args["primaries_in_s"] = default_primaries
-        if default_range is not None:
-            args["range_in_s"] = default_range
-        if default_chromaloc is not None:
-            args["chromaloc_in_s"] = default_chromaloc
-
-        # Detect subsampling
-        if clip.format.subsampling_w != 0 or clip.format.subsampling_h != 0:
-            # Allowing to manually specify a scaler function allows for testing of the args.
-            resizer = getattr(core.resize, scaler) if isinstance(scaler, str) else scaler
-        else:
-            resizer = core.resize.Point
-
-        return resizer(
-            c,
-            format=target_rgb,
-            **args
+        # This avoids capturing `clip` in a closure creating a self-reference. 
+        convert_yuv = functools.partial(
+            _convert_yuv,
+            core=core,
+            real_rgb24=real_rgb24,
+            default_args=default_args,
+            scaler=scaler
         )
 
-    def _actually_resize(c: vs.VideoNode) -> vs.VideoNode:
-        if c.format.color_family == vs.YUV:
-            c = _convert_yuv(c)
-
-        if c.format == vs.GRAY:
-            c = c.std.RemoveFrameProps("_Matrix")
-
-        if c.format.color_family != vs.RGB or c.format.bits_per_sample != bits_per_sample:
-            c = core.resize.Point(c, format=target_rgb)
-
-        return c
+        actually_resize = functools.partial(
+            _actually_resize,
+            core=core,
+            target_rgb=target_rgb,
+            convert_yuv=convert_yuv
+        )
 
-    with use_inline("to_rgb", env):
         return wrap_variable_size(
             clip,
             force_assumed_format=target_rgb,
-            func=_actually_resize
+            func=actually_resize
         )
 

vsengine/loops.py

@@ -48,6 +48,8 @@ def attach(self) -> None:
     def detach(self) -> None:
         """
         Called when another event-loop should take over.
+
+        For example, when you restarting your application.
         """
         ...