Repository: just-work/fffw

Issue:
Invalid naming
`test_filter_graph` fails if disable Yadif filter. This is because of inconsistent naming logic.

Discussion:

Patch:
diff --git a/.gitignore b/.gitignore
index 1dbc687..eab57e0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -60,3 +60,8 @@ target/
 
 #Ipython Notebook
 .ipynb_checkpoints
+
+.idea/
+
+.mypy_cache/
+.dmypy.json
\ No newline at end of file
diff --git a/fffw/encoding/codec.py b/fffw/encoding/codec.py
index 2718fb8..aef4053 100644
--- a/fffw/encoding/codec.py
+++ b/fffw/encoding/codec.py
@@ -1,5 +1,5 @@
 import abc
-from typing import Optional, List
+from typing import List
 
 from fffw.graph import base
 from fffw.wrapper import BaseWrapper
@@ -39,8 +39,7 @@ class BaseCodec(BaseWrapper, base.Node, metaclass=abc.ABCMeta):
     def map(self, value: str) -> None:
         self._args['map'] = value
 
-    def render(self, namer: base.Namer, name: Optional[str] = None,
-               partial: bool = False) -> List[str]:
+    def render(self, partial: bool = False) -> List[str]:
         """ codec output node is already rendered in filter graph."""
         return []
 
diff --git a/fffw/graph/base.py b/fffw/graph/base.py
index ceff198..95ab4b9 100644
--- a/fffw/graph/base.py
+++ b/fffw/graph/base.py
@@ -5,20 +5,53 @@ __all__ = [
 ]
 
 import abc
-from enum import Enum, auto
-from typing import Callable, Optional, List, Union
+from collections import Counter
+from enum import Enum
+from typing import Optional, List, Union, Dict, Any
 
 
 class StreamType(Enum):
-    VIDEO = auto()
-    AUDIO = auto()
+    VIDEO = 'v'
+    AUDIO = 'a'
 
 
 VIDEO = StreamType.VIDEO
 AUDIO = StreamType.AUDIO
 
-Namer = Callable[[str], str]
-""" Callable type for generating unique stream identifiers."""
+
+class Namer:
+    """ Unique stream identifiers generator."""
+    _stack: List["Namer"] = []
+
+    @classmethod
+    def name(cls, edge: "Edge") -> str:
+        current = cls._stack[0]
+        return current._name(edge)
+
+    def __init__(self) -> None:
+        self._counters: Dict[str, int] = Counter()
+        self._cache: Dict[int, str] = dict()
+
+    def __enter__(self) -> "Namer":
+        self._stack.append(self)
+        return self._stack[0]
+
+    def __exit__(self, *_: Any) -> None:
+        self._stack.pop(-1)
+
+    def _name(self, edge: "Edge") -> str:
+        if id(edge) not in self._cache:
+            node = edge.input
+            if isinstance(node, Node):
+                prefix = f'{node.kind.value}:{node.name}'
+                # generating unique edge id by src node kind and name
+                name = f'{prefix}{self._counters[prefix]}'
+                self._counters[prefix] += 1
+            else:
+                name = node.name
+            # caching edge name for idempotency
+            self._cache[id(edge)] = name
+        return self._cache[id(edge)]
 
 
 class Renderable(metaclass=abc.ABCMeta):
@@ -27,42 +60,19 @@ class Renderable(metaclass=abc.ABCMeta):
     """
 
     @abc.abstractmethod
-    def render(self,
-               namer: Namer,
-               name: Optional[str] = None,
-               partial: bool = False) -> List[str]:
-        """ Returns a list of filter_graph edge descriptions.
-
-        :param namer: callable used to generate unique edge identifiers
-        :param name: edge identifier
-        :param partial: partially formatted graph render mode floag
-        :return: edge description list ["[v:0]yadif[v:t1]", "[v:t1]scale[out]"]
+    def render(self, partial: bool = False) -> List[str]:
         """
-        raise NotImplementedError()
-
-
-class NameMixin:
-    """
-    A mixin for single-time object name initialization
-    """
-
-    def __init__(self, name: Optional[str] = None):
-        super().__init__()
-        self.__name: Optional[str] = name
+        Returns a list of filter_graph edge descriptions.
 
-    @property
-    def name(self) -> Optional[str]:
-        return self.__name
+        This method must be called in Namer context.
 
-    @name.setter
-    def name(self, value: str) -> None:
-        """ Set name only once."""
-        if self.__name is not None:
-            raise RuntimeError("Name is already set")
-        self.__name = value
+        :param partial: partially formatted graph render mode flag
+        :return: edge description list ["[v:0]yadif[v:t1]", "[v:t1]scale[out]"]
+        """
+        raise NotImplementedError()
 
 
-class Dest(Renderable, NameMixin):
+class Dest(Renderable):
     """
     Audio/video output stream node.
 
@@ -74,9 +84,20 @@ class Dest(Renderable, NameMixin):
         :param name: internal ffmpeg stream name ("v:0", "a:1")
         :param kind: stream kind (VIDEO/AUDIO)
         """
-        super().__init__(name=name)
         self._edge: Optional[Edge] = None
-        self.kind = kind
+        self._kind = kind
+        self._name = name
+
+    def __repr__(self) -> str:
+        return f"Dest('{self.name}')"
+
+    @property
+    def name(self) -> str:
+        return self._name
+
+    @property
+    def kind(self) -> StreamType:
+        return self._kind
 
     def connect_edge(self, edge: "Edge") -> "Edge":
         """ Connects and edge to output stream.
@@ -92,29 +113,22 @@ class Dest(Renderable, NameMixin):
         if self._edge is not None:
             raise RuntimeError("Dest is already connected to %s" % self._edge)
         self._edge = edge
-        self._edge.name = self.name
         return edge
 
-    def __repr__(self) -> str:
-        return f"Dest('{self.name}')"
-
-    def render(self, namer: Namer, name: Optional[str] = None,
-               partial: bool = False) -> List[str]:
+    def render(self, partial: bool = False) -> List[str]:
         # Previous nodes/edges already rendered destination node.
         return []
 
 
-InputType = Union[None, "Source", "Node"]
-OutputType = Union[None, "Dest", "Node"]
+InputType = Union["Source", "Node"]
+OutputType = Union["Dest", "Node"]
 
 
-class Edge(Renderable, NameMixin):
+class Edge(Renderable):
     """ Internal ffmpeg data stream graph."""
 
     # noinspection PyShadowingBuiltins
-    def __init__(self,
-                 input: InputType = None,
-                 output: OutputType = None) -> None:
+    def __init__(self, input: InputType, output: OutputType) -> None:
         """
         :param input: input node
         :param output: output node
@@ -126,6 +140,10 @@ class Edge(Renderable, NameMixin):
     def __repr__(self) -> str:
         return f'Edge#{self.name}[{self.input}, {self.output}]'
 
+    @property
+    def kind(self) -> StreamType:
+        return self.__input.kind
+
     @property
     def input(self) -> InputType:
         return self.__input
@@ -134,6 +152,28 @@ class Edge(Renderable, NameMixin):
     def output(self) -> OutputType:
         return self.__output
 
+    @property
+    def name(self) -> str:
+        """
+        Get actual name for edge from source node.
+
+        Property must be accessed within Namer context.
+
+        :returns: edge identifier generated from output node name if connected
+        to Dest, or a  name of last enabled filter before (and including)
+        current node.
+        """
+        if isinstance(self.output, Dest):
+            return self.output.name
+        edge = self
+        node = self.input
+        while not getattr(node, 'enabled', True) and isinstance(node, Node):
+            if node.inputs[0] is None:
+                raise RuntimeError("Node input is None")
+            edge = node.inputs[0]
+            node = edge.input
+        return Namer.name(edge)
+
     def _connect_source(self, src: Union["Source", "Node"]) -> None:
         """ Connects input node to the edge.
 
@@ -157,19 +197,12 @@ class Edge(Renderable, NameMixin):
         self.__output = dest
         return dest
 
-    def render(self,
-               namer: Namer,
-               name: Optional[str] = None,
-               partial: bool = False) -> List[str]:
+    def render(self, partial: bool = False) -> List[str]:
         if not self.__output:
             if partial:
                 return []
             raise RuntimeError("output is none")
-        if self.name is None:
-            if self.__output.name is None:
-                raise RuntimeError("output name is not set")
-            self.name = namer(self.__output.name)
-        return self.__output.render(namer, name=name, partial=partial)
+        return self.__output.render(partial=partial)
 
 
 class Node(Renderable):
@@ -193,6 +226,15 @@ class Node(Renderable):
         outputs = [f"[{str(i.name if i else '---')}]" for i in self.outputs]
         return f"{''.join(inputs)}{self.name}{''.join(outputs)}"
 
+    def __or__(self, other: Union["Node", "Dest"]) -> Union["Node", "Dest"]:
+        """
+        connect output edge to node
+        :return: connected edge
+        """
+        if not isinstance(other, (Node, Dest)):
+            return NotImplemented
+        return self.connect_dest(other)
+
     @property
     def enabled(self) -> bool:
         return self.__enabled
@@ -201,8 +243,14 @@ class Node(Renderable):
     def enabled(self, value: bool) -> None:
         self.__enabled = value
 
-    def render(self, namer: Namer, name: Optional[str] = None,
-               partial: bool = False) -> List[str]:
+    @property
+    def args(self) -> str:
+        """
+        Generates filter params as a string
+        """
+        return ''
+
+    def render(self, partial: bool = False) -> List[str]:
         if not self.enabled:
             # filter skipped, input is connected to output directly
             next_edge = self.outputs[0]
@@ -210,9 +258,9 @@ class Node(Renderable):
                 if partial:
                     return []
                 raise RuntimeError("output is None")
-            return next_edge.render(namer, name=name, partial=partial)
+            return next_edge.render(partial=partial)
 
-        result = [self.get_filter_cmd(namer, gid=name, partial=partial)]
+        result = [self.get_filter_cmd(partial=partial)]
 
         for dest in self.outputs:
             if dest is None:
@@ -221,23 +269,18 @@ class Node(Renderable):
                 raise RuntimeError("destination is none")
             if isinstance(dest.output, Dest):
                 continue
-            result.extend(dest.render(namer, partial=partial))
+            result.extend(dest.render(partial=partial))
         return result
 
-    # noinspection PyShadowingBuiltins
-    def get_filter_cmd(self, namer: Namer,
-                       gid: Optional[str] = None,
-                       partial: bool = False) -> str:
+    def get_filter_cmd(self, partial: bool = False) -> str:
         """
         Returns filter description.
 
         output format is like [IN] FILTER ARGS [OUT]
         where IN, OUT - lists of input/output edge id,
-        FILTER - fiter name, ARGS - filter params
+        FILTER - filter name, ARGS - filter params
 
-        :param namer: callable used to generate unique edge identifiers
-        :param gid: edge identifier
-        :param partial: partially formatted graph render mode floag
+        :param partial: partially formatted graph render mode flag
         :return: current description string like "[v:0]yadif[v:t1]"
         """
         inputs = []
@@ -245,9 +288,7 @@ class Node(Renderable):
         for edge in self.inputs:
             if edge is None:
                 raise RuntimeError("input is none")
-            if edge.name is None:
-                edge.name = namer(self.name)
-            inputs.append("[%s]" % str(gid or edge.name))
+            inputs.append(f"[{edge.name}]")
 
         for edge in self.outputs:
             if edge is None:
@@ -264,19 +305,10 @@ class Node(Renderable):
                 if edge is None:
                     raise RuntimeError("output is none")
                 node = edge.output
-            if edge.name is None:
-                edge.name = namer(self.name)
             outputs.append(f"[{edge.name}]")
         args = '=' + self.args if self.args else ''
         return ''.join(inputs) + self.name + args + ''.join(outputs)
 
-    @property
-    def args(self) -> str:
-        """
-        Generates filter params as a string
-        """
-        return ''
-
     def connect_edge(self, edge: "Edge") -> "Edge":
         """ Connects and edge to one of filter inputs
 
@@ -302,17 +334,8 @@ class Node(Renderable):
         other.connect_edge(edge)
         return other
 
-    def __or__(self, other: Union["Node", "Dest"]) -> Union["Node", "Dest"]:
-        """
-        connect output edge to node
-        :return: connected edge
-        """
-        if not isinstance(other, (Node, Dest)):
-            return NotImplemented
-        return self.connect_dest(other)
-
 
-class Source(Renderable, NameMixin):
+class Source(Renderable):
     """ Graph node containing audio or video input.
 
     Must connect to single graph edge only as a source
@@ -323,9 +346,27 @@ class Source(Renderable, NameMixin):
         :param name: ffmpeg internal input stream name ("v:0", "a:1")
         :param kind: stream type (VIDEO/AUDIO)
         """
-        super().__init__(name=name)
         self._edge: Optional[Edge] = None
         self._kind = kind
+        self.__name = name
+
+    def __repr__(self) -> str:
+        return f"Source('[{self.name}]')"
+
+    def __or__(self, other: Node) -> Node:
+        """
+        Connect a filter to a source
+        :return: connected filter
+        """
+        if not isinstance(other, Node):
+            return NotImplemented
+        return self.connect(other)
+
+    @property
+    def name(self) -> str:
+        if self.__name is None:
+            raise RuntimeError("Source name not set")
+        return self.__name
 
     @property
     def edge(self) -> Optional["Edge"]:
@@ -351,23 +392,10 @@ class Source(Renderable, NameMixin):
             raise RuntimeError("Source %s is already connected to %s"
                                % (self.name, self._edge))
         edge = Edge(input=self, output=other)
-        edge.name = self.name
         self._edge = self._edge or other.connect_edge(edge)
         return other
 
-    def __or__(self, other: Node) -> Node:
-        """
-        Connect a filter to a source
-        :return: connected filter
-        """
-        if not isinstance(other, Node):
-            return NotImplemented
-        return self.connect(other)
-
-    def render(self,
-               namer: Namer,
-               name: Optional[str] = None,
-               partial: bool = False) -> List[str]:
+    def render(self, partial: bool = False) -> List[str]:
         if self._edge is None:
             if partial:
                 return []
@@ -376,7 +404,6 @@ class Source(Renderable, NameMixin):
         node = self._edge.output
         # if output node is disabled, use next edge identifier.
         if isinstance(node, Node) and not node.enabled:
-            name = self._edge.name
             edge: Optional[Edge] = node.outputs[0]
             if edge is None:
                 if partial:
@@ -384,8 +411,4 @@ class Source(Renderable, NameMixin):
                 raise RuntimeError("Skipped node is not ready for render")
         else:
             edge = self._edge
-            name = None
-        return edge.render(namer, name=name, partial=partial)
-
-    def __repr__(self) -> str:
-        return f"Source('[{self.name}]')"
+        return edge.render(partial=partial)
diff --git a/fffw/graph/complex.py b/fffw/graph/complex.py
index 7d672fa..d3b3fc2 100644
--- a/fffw/graph/complex.py
+++ b/fffw/graph/complex.py
@@ -21,8 +21,6 @@ class FilterComplex:
         self.audio = audio or sources.Input(kind=base.AUDIO)
         self.__video_outputs: Dict[int, base.Dest] = {}
         self.__audio_outputs: Dict[int, base.Dest] = {}
-        self._video_tmp: Dict[str, int] = collections.Counter()
-        self._audio_tmp: Dict[str, int] = collections.Counter()
 
     def get_video_dest(self, index: int = 0, create: bool = True) -> base.Dest:
         """ Returns video output by index.
@@ -59,16 +57,23 @@ class FilterComplex:
         Returns filter_graph description in corresponding ffmpeg param syntax.
         """
         result = []
-        for src in self.video.streams:
-            # noinspection PyProtectedMember
-            if not src._edge:
-                continue
-            result.extend(src.render(self.video_naming, partial=partial))
-        for src in self.audio.streams:
-            # noinspection PyProtectedMember
-            if not src._edge:
-                continue
-            result.extend(src.render(self.audio_naming, partial=partial))
+        with base.Namer():
+            # Initialize namer context to track unique edge identifiers.
+            # In name generation there is no access to namer, so it is accessed
+            # via Namer singleton's method. Within context it is guaranteed that
+            # same edges will receive same names and different edges will
+            # receive unique names. This includes idempotent results for
+            # subsequent render() calls for outer Namer context.
+            for src in self.video.streams:
+                # noinspection PyProtectedMember
+                if not src._edge:
+                    continue
+                result.extend(src.render(partial=partial))
+            for src in self.audio.streams:
+                # noinspection PyProtectedMember
+                if not src._edge:
+                    continue
+                result.extend(src.render(partial=partial))
 
         # There are no visit checks in recurse graph traversing, so remove
         # duplicates respecting order of appearance.
@@ -76,25 +81,3 @@ class FilterComplex:
 
     def __str__(self) -> str:
         return self.render()
-
-    def video_naming(self, name: str = 'tmp') -> str:
-        """ Unique video edge identifier generator.
-
-        :param name: prefix used in name generation.
-        :type name: str
-        :rtype: str
-        """
-        res = 'v:%s%s' % (name, self._video_tmp[name])
-        self._video_tmp[name] += 1
-        return res
-
-    def audio_naming(self, name: str = 'tmp') -> str:
-        """ Unique audio edge identifier generator.
-
-        :param name: prefix used in name generation.
-        :type name: str
-        :rtype: str
-        """
-        res = 'a:%s%s' % (name, self._audio_tmp[name])
-        self._audio_tmp[name] += 1
-        return res
diff --git a/fffw/graph/sources.py b/fffw/graph/sources.py
index e668598..9385edf 100644
--- a/fffw/graph/sources.py
+++ b/fffw/graph/sources.py
@@ -83,10 +83,13 @@ class Input:
         assert isinstance(other, base.Node)
         input_map = getattr(other, 'map', None)
         for stream in self.streams:
-            if stream.name is None:
-                # skip streams not present in source file
+            try:
+                stream_name = stream.name
+            except RuntimeError:
+                # skip streams not present in input file
                 continue
-            if input_map and input_map == stream.name:
+
+            if input_map and input_map == stream_name:
                 return stream.connect(other)
             if stream.edge is None:
                 return stream.connect(other)


Test patch:
diff --git a/tests/test_ffmpeg.py b/tests/test_ffmpeg.py
index 54db7d7..331538d 100644
--- a/tests/test_ffmpeg.py
+++ b/tests/test_ffmpeg.py
@@ -117,8 +117,8 @@ class FFMPEGTestCase(TestCase):
             '-filter_complex',
             (
                 '[0:v]scale=640x360[v:scale0];'
-                '[v:scale0][v:overlay0]overlay=x=0:y=0[vout0];'
-                '[1:v]scale=1280x720[v:overlay0];'
+                '[v:scale0][v:scale1]overlay=x=0:y=0[vout0];'
+                '[1:v]scale=1280x720[v:scale1];'
                 '[1:a]volume=-20.00[aout0]'),
             '-map', '[vout0]', '-c:v', 'libx264', '-b:v', '700000',
             '-map', '[aout0]', '-c:a', 'aac', '-b:a', '128000',
diff --git a/tests/test_graph.py b/tests/test_graph.py
index 06e1c10..ca7202c 100644
--- a/tests/test_graph.py
+++ b/tests/test_graph.py
@@ -24,14 +24,14 @@ class FilterGraphTestCase(TestCase):
         fc = FilterComplex(video=sources.Input(video_streams, VIDEO),
                            audio=sources.Input(audio_streams, AUDIO))
 
-        deint = Deint(enabled=True)  # deinterlace is disabled
+        deint = Deint(enabled=False)  # deinterlace is disabled
 
         # first video stream is deinterlaced
         next_node = fc.video | deint
 
         left, top = 20, 20  # logo position
 
-        # first overlay input is deinterlaced source (or source itself is
+        # first overlay input is deinterlaced source (or source itself as
         # deint filter is disabled)
         over = next_node | Overlay(left, top)
 
@@ -58,31 +58,30 @@ class FilterGraphTestCase(TestCase):
         for i, size in enumerate(sizes):
             # add scale filters to video streams
             w, h = size
-            scale = Scale(w, h, enabled=size)
+            scale = Scale(w, h, enabled=True)
             # connect scaled streams to video destinations
             split | scale | fc.get_video_dest(i)
 
         result = fc.render()
 
         expected = ';'.join([
-            # deinterlace
-            '[0:v]yadif=0[v:yadif0]',
             # overlay logo
-            '[v:yadif0][v:overlay0]overlay=x=20:y=20[v:overlay1]',
+            '[0:v][v:scale0]overlay=x=20:y=20[v:overlay0]',
             # split video to two streams
-            '[v:overlay1]split[v:split0][v:split1]',
+            '[v:overlay0]split[v:split0][v:split1]',
             # each video is scaled to own size
             '[v:split0]scale=640x480[vout0]',
             '[v:split1]scale=1280x720[vout1]',
 
             # logo scaling
-            '[1:v]scale=200x50[v:overlay0]',
+            '[1:v]scale=200x50[v:scale0]',
 
             # split audio to two streams
             '[0:a]asplit[aout0][aout1]'
         ])
 
-        self.assertEqual(result, expected)
+        self.assertEqual(expected.replace(';', ';\n'),
+                         result.replace(';', ';\n'))
 
     def test_disabled_filters(self):
         """ Filter skipping."""