From 497105f929a7a927ee8807519738f48c49b5a3a1 Mon Sep 17 00:00:00 2001 From: Davide Depau Date: Tue, 9 Jan 2018 14:45:27 +0100 Subject: [PATCH] Reimplement `.map` logic making `Node` immutable --- ffmpeg/_ffmpeg.py | 4 +- ffmpeg/nodes.py | 157 +++++++++++++++++++++++++++--------- ffmpeg/tests/test_ffmpeg.py | 5 +- 3 files changed, 122 insertions(+), 44 deletions(-) diff --git a/ffmpeg/_ffmpeg.py b/ffmpeg/_ffmpeg.py index 1abf2ca..231783a 100644 --- a/ffmpeg/_ffmpeg.py +++ b/ffmpeg/_ffmpeg.py @@ -83,9 +83,7 @@ def map(*streams): if not tail: return head - head.node._add_streams(tail) - - return head + return OutputNode(head.node, tail).stream() __all__ = [ diff --git a/ffmpeg/nodes.py b/ffmpeg/nodes.py index fdbd8ff..89fdc11 100644 --- a/ffmpeg/nodes.py +++ b/ffmpeg/nodes.py @@ -1,9 +1,12 @@ from __future__ import unicode_literals +import warnings + from .dag import KwargReprNode from ._utils import escape_chars, get_hash_int from builtins import object -import os +import os, sys +import inspect def _is_of_types(obj, types): @@ -19,6 +22,13 @@ def _get_types_str(types): return ', '.join(['{}.{}'.format(x.__module__, x.__name__) for x in types]) +def _get_arg_count(callable): + if sys.version_info.major >= 3: + return len(inspect.getfullargspec(callable).args) + else: + return len(inspect.getargspec(callable).args) + + class Stream(object): """Represents the outgoing edge of an upstream node; may be used to create more downstream nodes.""" @@ -30,6 +40,7 @@ class Stream(object): self.label = upstream_label self.selector = upstream_selector + def __hash__(self): return get_hash_int([hash(self.node), hash(self.label)]) @@ -85,6 +96,22 @@ def get_stream_spec_nodes(stream_spec): class Node(KwargReprNode): """Node base""" + @property + def min_inputs(self): + return self.__min_inputs + + @property + def max_inputs(self): + return self.__max_inputs + + @property + def incoming_stream_types(self): + return self.__incoming_stream_types + + @property + def outgoing_stream_type(self): + return self.__outgoing_stream_type + @classmethod def __check_input_len(cls, stream_map, min_inputs, max_inputs): if min_inputs is not None and len(stream_map) < min_inputs: @@ -106,19 +133,85 @@ class Node(KwargReprNode): incoming_edge_map[downstream_label] = (upstream.node, upstream.label, upstream.selector) return incoming_edge_map - def __init__(self, stream_spec, name, incoming_stream_types, outgoing_stream_type, min_inputs, max_inputs, args=[], - kwargs={}): + def __init_fromscratch__(self, stream_spec, name, incoming_stream_types, outgoing_stream_type, min_inputs, + max_inputs, args=[], + kwargs={}): stream_map = get_stream_map(stream_spec) self.__check_input_len(stream_map, min_inputs, max_inputs) self.__check_input_types(stream_map, incoming_stream_types) incoming_edge_map = self.__get_incoming_edge_map(stream_map) + super(Node, self).__init__(incoming_edge_map, name, args, kwargs) self.__outgoing_stream_type = outgoing_stream_type - self.__incoming_stream_types = incoming_stream_types self.__min_inputs = min_inputs self.__max_inputs = max_inputs + def __init_fromnode__(self, old_node, stream_spec): + # Make sure old node and new node are of the same type + if type(self) != type(old_node): + raise ValueError("'old_node' should be of type {}".format(self.__class__.__name__)) + + # Copy needed data from old node + name = old_node.name + incoming_stream_types = old_node.incoming_stream_types + outgoing_stream_type = old_node.outgoing_stream_type + min_inputs = old_node.min_inputs + max_inputs = old_node.max_inputs + prev_edges = old_node.incoming_edge_map.values() + args = old_node.args + kwargs = old_node.kwargs + + # Check new stream spec - the old spec should have already been checked + new_stream_map = get_stream_map(stream_spec) + self.__check_input_types(new_stream_map, incoming_stream_types) + + # Generate new edge map + new_inc_edge_map = self.__get_incoming_edge_map(new_stream_map) + new_edges = new_inc_edge_map.values() + + # Rename all edges + new_edge_map = dict(enumerate(list(prev_edges) + list(new_edges))) + + # Check new length + self.__check_input_len(new_edge_map, min_inputs, max_inputs) + + super(Node, self).__init__(new_edge_map, name, args, kwargs) + self.__outgoing_stream_type = outgoing_stream_type + self.__incoming_stream_types = incoming_stream_types + self.__min_inputs = min_inputs + self.__max_inputs = max_inputs + + # noinspection PyMissingConstructor + def __init__(self, *args, **kwargs): + """ + If called with the following arguments, the new Node is created from scratch: + - stream_spec, name, incoming_stream_types, outgoing_stream_type, min_inputs, max_inputs, args=[], kwargs={} + + If called with the following arguments, the new node is a copy of `old_node` that includes the additional + `stream_spec`: + - old_node, stream_spec + """ + # Python doesn't support constructor overloading. This hacky code detects how we want to construct the object + # based on the number of arguments and the type of the first argument, then calls the appropriate constructor + # helper method + + # "1+" is for `self` + argc = 1 + len(args) + len(kwargs) + + first_arg = "old_node" in kwargs and kwargs["old_node"] or args[0] + + if argc == _get_arg_count(self.__init_fromnode__) and type(first_arg) == type(self): + self.__init_fromnode__(*args, **kwargs) + else: + if isinstance(first_arg, Node): + raise ValueError( + "{}.__init__() received an instance of {} as the first argument. If you want to create a " + "copy of an existing node, the types must match and you must provide an additional stream_spec." + .format(self.__class__.__name__, first_arg.__class__.__name__) + ) + self.__init_fromscratch__(*args, **kwargs) + def stream(self, label=None, select=None): """Create an outgoing stream originating from this node. @@ -136,31 +229,6 @@ class Node(KwargReprNode): else: return self.stream(label=item) - def _add_streams(self, stream_spec): - """Attach additional streams after the Node is initialized. - """ - # Back up previous edges - prev_edges = self.incoming_edge_map.values() - - # Check new edges - new_stream_map = get_stream_map(stream_spec) - self.__check_input_types(new_stream_map, self.__incoming_stream_types) - - # Generate new edge map - new_inc_edge_map = self.__get_incoming_edge_map(new_stream_map) - new_edges = new_inc_edge_map.values() - - # Rename all edges - new_edge_map = dict(enumerate(list(prev_edges) + list(new_edges))) - - # Check new length - self.__check_input_len(new_edge_map, self.__min_inputs, self.__max_inputs) - - # Overwrite old map (exploiting the fact that dict is mutable; incoming_edge_map is a read-only property) - if None in self.incoming_edge_map: - self.incoming_edge_map.pop(None) - self.incoming_edge_map.update(new_edge_map) - class FilterableStream(Stream): def __init__(self, upstream_node, upstream_label, upstream_selector=None): @@ -168,11 +236,12 @@ class FilterableStream(Stream): upstream_selector) +# noinspection PyMethodOverriding class InputNode(Node): """InputNode type""" - def __init__(self, name, args=[], kwargs={}): - super(InputNode, self).__init__( + def __init_fromscratch__(self, name, args=[], kwargs={}): + super(InputNode, self).__init_fromscratch__( stream_spec=None, name=name, incoming_stream_types={}, @@ -183,14 +252,18 @@ class InputNode(Node): kwargs=kwargs ) + def __init_fromnode__(self, old_node, stream_spec): + raise TypeError("{} can't be constructed from an existing node".format(self.__class__.__name__)) + @property def short_repr(self): return os.path.basename(self.kwargs['filename']) +# noinspection PyMethodOverriding class FilterNode(Node): - def __init__(self, stream_spec, name, max_inputs=1, args=[], kwargs={}): - super(FilterNode, self).__init__( + def __init_fromscratch__(self, stream_spec, name, max_inputs=1, args=[], kwargs={}): + super(FilterNode, self).__init_fromscratch__( stream_spec=stream_spec, name=name, incoming_stream_types={FilterableStream}, @@ -227,9 +300,10 @@ class FilterNode(Node): return escape_chars(params_text, '\\\'[],;') +# noinspection PyMethodOverriding class OutputNode(Node): - def __init__(self, stream, name, args=[], kwargs={}): - super(OutputNode, self).__init__( + def __init_fromscratch__(self, stream, name, args=[], kwargs={}): + super(OutputNode, self).__init_fromscratch__( stream_spec=stream, name=name, incoming_stream_types={FilterableStream}, @@ -251,9 +325,10 @@ class OutputStream(Stream): upstream_selector=upstream_selector) +# noinspection PyMethodOverriding class MergeOutputsNode(Node): - def __init__(self, streams, name): - super(MergeOutputsNode, self).__init__( + def __init_fromscratch__(self, streams, name): + super(MergeOutputsNode, self).__init_fromscratch__( stream_spec=streams, name=name, incoming_stream_types={OutputStream}, @@ -263,9 +338,10 @@ class MergeOutputsNode(Node): ) +# noinspection PyMethodOverriding class GlobalNode(Node): - def __init__(self, stream, name, args=[], kwargs={}): - super(GlobalNode, self).__init__( + def __init_fromscratch__(self, stream, name, args=[], kwargs={}): + super(GlobalNode, self).__init_fromscratch__( stream_spec=stream, name=name, incoming_stream_types={OutputStream}, @@ -276,6 +352,9 @@ class GlobalNode(Node): kwargs=kwargs ) + def __init_fromnode__(self, old_node, stream_spec): + raise TypeError("{} can't be constructed from an existing node".format(self.__class__.__name__)) + def stream_operator(stream_classes={Stream}, name=None): def decorator(func): diff --git a/ffmpeg/tests/test_ffmpeg.py b/ffmpeg/tests/test_ffmpeg.py index e6d8186..07ae212 100644 --- a/ffmpeg/tests/test_ffmpeg.py +++ b/ffmpeg/tests/test_ffmpeg.py @@ -166,11 +166,12 @@ def test_map_same_effect_as_output(): i1 = ffmpeg.input(TEST_INPUT_FILE1) i2 = ffmpeg.input(TEST_OVERLAY_FILE) - o_map = i1.output(TEST_OUTPUT_FILE1) - o_map.map(i2) + _o_map = i1.output(TEST_OUTPUT_FILE1) + o_map = _o_map.map(i2) o_nomap = ffmpeg.output(i1, i2, TEST_OUTPUT_FILE1) + assert id(o_map) != id(_o_map) # Checks immutability assert o_map.node.incoming_edge_map == o_nomap.node.incoming_edge_map assert o_map.get_args() == o_nomap.get_args() == ['-i', TEST_INPUT_FILE1, '-i', TEST_OVERLAY_FILE,