Remove 'coreader' methods adding formal '__copy__' API (Zulko#1442)

* Replace 'coreader' methods by formal '__copy__' API * Fix code style error * Add support for 'copy.copy(clip)' and 'copy.deepcopy(clip)' to CHANGELOG * Add tests for copied clips rendering * Fine tune comments * Update CHANGELOG.md * Fix flake8 errors Co-authored-by: Tom Burrows <[email protected]>
hly990 · Jan 16, 2021 · cc02587 · cc02587
1 parent 72e1418
commit cc02587
Show file tree

Hide file tree

Showing 15 changed files with 196 additions and 90 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Important Announcements
 
 ### Added <!-- for new features -->
-- `VideoFileClip.coreader()` that creates a new clip created from the same source file as the original (to complement the existing `AudioFileClip.coreader()`) [\#1332](https://github.com/Zulko/moviepy/pull/1332)
+- Support for `copy.copy(clip)` and `copy.deepcopy(clip)` with same behaviour as `clip.copy()` [\#1442](https://github.com/Zulko/moviepy/pull/1442)
 - `audio.fx.multiply_stereo_volume` to control volume by audio channels [\#1424](https://github.com/Zulko/moviepy/pull/1424)
 
 ### Changed <!-- for changes in existing functionality -->
@@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `moviepy.video.fx.all` and `moviepy.audio.fx.all`. Use the fx method directly from the clip instance or import the fx function from `moviepy.video.fx` and `moviepy.audio.fx`. [\#1105](https://github.com/Zulko/moviepy/pull/1105)
 
 ### Removed <!-- for now removed features -->
+- `VideoFileClip.coreader` and `AudioFileClip.coreader` methods removed. Use `VideoFileClip.copy` and `AudioFileClip.copy` instead [\#1442](https://github.com/Zulko/moviepy/pull/1442)
 - `audio.fx.audio_loop` removed. Use `video.fx.loop` instead for all types of clip [\#1451](https://github.com/Zulko/moviepy/pull/1451)
 
 ### Fixed <!-- for any bug fixes -->

diff --git a/docs/getting_started/efficient_moviepy.rst b/docs/getting_started/efficient_moviepy.rst
@@ -32,7 +32,6 @@ However, if you close a clip too early, methods on the clip (and any clips deriv
 So, the rules of thumb are:
 
     * Call ``close()`` on any clip that you **construct** once you have finished using it and have also finished using any clip that was derived from it.
-    * Also close any clips you create through ``AudioFileClip.coreader()``.
     * Even if you close a ``CompositeVideoClip`` instance, you still need to close the clips it was created from.
     * Otherwise, if you have a clip that was created by deriving it from from another clip (e.g. by calling ``with_mask()``), then generally you shouldn't close it. Closing the original clip will also close the copy.
 

diff --git a/examples/example_with_sound.py b/examples/example_with_sound.py
@@ -31,10 +31,7 @@
 mask_clip = ImageClip(mask, is_mask=True)
 
 clip_left = (
-    main_clip.coreader()
-    .subclip(0, duration)
-    .crop(x1=60, x2=60 + 2 * W / 3)
-    .with_mask(mask_clip)
+    main_clip.subclip(0, duration).crop(x1=60, x2=60 + 2 * W / 3).with_mask(mask_clip)
 )
 
 
@@ -47,8 +44,7 @@
 mask_clip = ImageClip(mask, is_mask=True)
 
 clip_right = (
-    main_clip.coreader()
-    .subclip(21, 21 + duration)
+    main_clip.subclip(21, 21 + duration)
     .crop(x1=70, x2=70 + 2 * W / 3)
     .with_mask(mask_clip)
 )

diff --git a/examples/painting_effect.py b/examples/painting_effect.py
@@ -7,10 +7,8 @@
 charade = VideoFileClip("../../videos/charade.mp4")
 tfreeze = convert_to_seconds(19.21)  # Time of the freeze, 19'21
 
-# when using several subclips of a same clip, it can be faster
-# to create 'coreaders' of the clip (=other entrance points).
-clip_before = charade.coreader().subclip(tfreeze - 2, tfreeze)
-clip_after = charade.coreader().subclip(tfreeze, tfreeze + 2)
+clip_before = charade.subclip(tfreeze - 2, tfreeze)
+clip_after = charade.subclip(tfreeze, tfreeze + 2)
 
 
 # THE FRAME TO FREEZE

diff --git a/moviepy/Clip.py b/moviepy/Clip.py
@@ -4,7 +4,7 @@
 and AudioClip.
 """
 
-from copy import copy
+import copy as _copy
 
 import numpy as np
 import proglog
@@ -60,23 +60,10 @@ def __init__(self):
         self.memoized_frame = None
 
     def copy(self):
-        """Shallow copy of the clip.
-
-        Returns a shallow copy of the clip whose mask and audio will
-        be shallow copies of the clip's mask and audio if they exist.
-
-        This method is intensively used to produce new clips every time
-        there is an outplace transformation of the clip (clip.resize,
-        clip.subclip, etc.)
         """
-
-        new_clip = copy(self)
-        if hasattr(self, "audio"):
-            new_clip.audio = copy(self.audio)
-        if hasattr(self, "mask"):
-            new_clip.mask = copy(self.mask)
-
-        return new_clip
+        Allows the usage of ``.copy()`` in clips as chained methods invocation.
+        """
+        return _copy.copy(self)
 
     @convert_parameter_to_seconds(["t"])
     def get_frame(self, t):

diff --git a/moviepy/audio/AudioClip.py b/moviepy/audio/AudioClip.py
@@ -29,6 +29,10 @@ class AudioClip(Clip):
       for a sound, it is just a float. What 'makes' the sound are
       the variations of that float in the time.
 
+    duration
+      Duration of the clip (in seconds). Some clips are infinite, in
+      this case their duration will be ``None``.
+
     nchannels
       Number of channels (one or two for mono or stereo).
 

diff --git a/moviepy/audio/io/AudioFileClip.py b/moviepy/audio/io/AudioFileClip.py
@@ -45,23 +45,11 @@ class AudioFileClip(AudioClip):
     of these instances, you must call close() afterwards, or the subresources
     will not be cleaned up until the process ends.
 
-    If copies are made, and close() is called on one, it may cause methods on
-    the other copies to fail.
-
-    However, coreaders must be closed separately.
-
     Examples
     ----------
 
     >>> snd = AudioFileClip("song.wav")
     >>> snd.close()
-    >>> snd = AudioFileClip("song.mp3", fps = 44100)
-    >>> second_reader = snd.coreader()
-    >>> second_reader.close()
-    >>> snd.close()
-    >>> with AudioFileClip(mySoundArray, fps=44100) as snd:  # from a numeric array
-    >>>     pass  # Close is implicitly performed by context manager.
-
     """
 
     @convert_path_to_string("filename")
@@ -88,12 +76,6 @@ def __init__(
         self.make_frame = lambda t: self.reader.get_frame(t)
         self.nchannels = self.reader.nchannels
 
-    def coreader(self):
-        """Returns a copy of the AudioFileClip, i.e. a new entrance point
-        to the audio file. Use copy when you have different clips
-        watching the audio file at different times."""
-        return AudioFileClip(self.filename, self.buffersize)
-
     def close(self):
         """ Close the internal reader. """
         if self.reader:

diff --git a/moviepy/video/VideoClip.py b/moviepy/video/VideoClip.py
@@ -7,6 +7,7 @@
 import os
 import subprocess as sp
 import tempfile
+import copy as _copy
 
 import numpy as np
 import proglog
@@ -116,6 +117,33 @@ def h(self):
     def aspect_ratio(self):
         return self.w / float(self.h)
 
+    def __copy__(self):
+        """Mixed copy of the clip.
+
+        Returns a shallow copy of the clip whose mask and audio will
+        be shallow copies of the clip's mask and audio if they exist.
+
+        This method is intensively used to produce new clips every time
+        there is an outplace transformation of the clip (clip.resize,
+        clip.subclip, etc.)
+
+        Acts like a deepcopy except for the fact that readers and other
+        possible unpickleables objects are not copied.
+        """
+        cls = self.__class__
+        new_clip = cls.__new__(cls)
+        for attr in self.__dict__:
+            value = getattr(self, attr)
+            if attr in ("mask", "audio"):
+                value = _copy.copy(value)
+            setattr(new_clip, attr, value)
+
+        new_clip.audio = _copy.copy(self.audio)
+        new_clip.mask = _copy.copy(self.mask)
+        return new_clip
+
+    copy = __copy__
+
     # ===============================================================
     # EXPORT OPERATIONS
 

diff --git a/moviepy/video/fx/blink.py b/moviepy/video/fx/blink.py
@@ -1,13 +1,10 @@
-import copy
-
-
 def blink(clip, duration_on, duration_off):
     """
     Makes the clip blink. At each blink it will be displayed ``duration_on``
     seconds and disappear ``duration_off`` seconds. Will only work in
     composite clips.
     """
-    new_clip = copy.copy(clip)
+    new_clip = clip.copy()
     if new_clip.mask is None:
         new_clip = new_clip.with_mask()
     duration = duration_on + duration_off

diff --git a/moviepy/video/io/VideoFileClip.py b/moviepy/video/io/VideoFileClip.py
@@ -147,11 +147,16 @@ def mask_make_frame(t):
                 nbytes=audio_nbytes,
             )
 
-    def coreader(self):
-        """Returns a copy of the VideoFileClip, i.e. a new entrance point
-        to the video file. Use copy when you have different clips
-        watching the video file at different times."""
-        return VideoFileClip(self.filename, audio_buffersize=self.audio.buffersize)
+    def __deepcopy__(self, memo):
+        """AudioFileClip can't be deeply copied because the locked Thread
+        of ``proc`` isn't pickleable. Without this override, calls to
+        ``copy.deepcopy(clip)`` will raise next ``TypeError``:
+
+        ```
+        TypeError: cannot pickle '_thread.lock' object
+        ```
+        """
+        return self.__copy__()
 
     def close(self):
         """ Close the internal reader. """

diff --git a/tests/test_AudioClips.py b/tests/test_AudioClips.py
@@ -16,13 +16,6 @@
 from tests.test_helper import TMP_DIR
 
 
-def test_audio_coreader():
-    sound = AudioFileClip("media/crunching.mp3")
-    sound = sound.subclip(1, 4)
-    sound2 = AudioFileClip("media/crunching.mp3")
-    sound2.write_audiofile(os.path.join(TMP_DIR, "coreader.mp3"))
-
-
 def test_audioclip():
     make_frame = lambda t: [np.sin(440 * 2 * np.pi * t)]
     audio = AudioClip(make_frame, duration=2, fps=22050)

diff --git a/tests/test_Clip.py b/tests/test_Clip.py
@@ -1,5 +1,8 @@
+import copy
+
 import pytest
 
+from moviepy.Clip import Clip
 from moviepy.video.VideoClip import BitmapClip
 
 
@@ -16,5 +19,39 @@ def test_clip_equality():
     assert clip != different_clip
 
 
+@pytest.mark.parametrize(
+    "copy_func",
+    (
+        lambda clip: clip.copy(),
+        lambda clip: copy.copy(clip),
+        lambda clip: copy.deepcopy(clip),
+    ),
+    ids=(
+        "clip.copy()",
+        "copy.copy(clip)",
+        "copy.deepcopy(clip)",
+    ),
+)
+def test_clip_copy(copy_func):
+    """Clip must be copied with `.copy()` method, `copy.copy()` and
+    `copy.deepcopy()` (same behaviour).
+    """
+    clip = Clip()
+    other_clip = Clip()
+
+    # shallow copy of clip
+    for attr in clip.__dict__:
+        setattr(clip, attr, "foo")
+
+    copied_clip = copy_func(clip)
+
+    # assert copied attributes
+    for attr in copied_clip.__dict__:
+        assert getattr(copied_clip, attr) == getattr(clip, attr)
+
+        # other instances are not edited
+        assert getattr(copied_clip, attr) != getattr(other_clip, attr)
+
+
 if __name__ == "__main__":
     pytest.main()
diff --git a/tests/test_VideoClip.py b/tests/test_VideoClip.py
@@ -1,3 +1,4 @@
+import copy
 import os
 
 import numpy as np
@@ -9,7 +10,7 @@
 from moviepy.video.compositing.CompositeVideoClip import CompositeVideoClip
 from moviepy.video.fx.speedx import speedx
 from moviepy.video.io.VideoFileClip import VideoFileClip
-from moviepy.video.VideoClip import ColorClip, BitmapClip
+from moviepy.video.VideoClip import ColorClip, BitmapClip, VideoClip
 
 from tests.test_helper import TMP_DIR
 
@@ -298,5 +299,71 @@ def test_setfps_withchangeduration():
     close_all_clips(locals())
 
 
+def test_copied_videoclip_write_videofile():
+    """Check if a copied ``VideoClip`` instance can render a file which has
+    the same features as the copied clip when opening with ``VideoFileClip``.
+    """
+    clip = BitmapClip([["RRR", "GGG", "BBB"]], fps=1)
+    copied_clip = clip.copy()
+
+    output_filepath = os.path.join(TMP_DIR, "copied_videoclip_from_bitmap.webm")
+    copied_clip.write_videofile(output_filepath)
+    copied_clip_from_file = VideoFileClip(output_filepath)
+
+    assert list(copied_clip.size) == list(copied_clip_from_file.size)
+    assert copied_clip.duration == copied_clip_from_file.duration
+
+
+@pytest.mark.parametrize(
+    "copy_func",
+    (
+        lambda clip: clip.copy(),
+        lambda clip: copy.copy(clip),
+        lambda clip: copy.deepcopy(clip),
+    ),
+    ids=("clip.copy()", "copy.copy(clip)", "copy.deepcopy(clip)"),
+)
+def test_videoclip_copy(copy_func):
+    """It must be possible to do a mixed copy of VideoClip using ``clip.copy()``,
+    ``copy.copy(clip)`` and ``copy.deepcopy(clip)``.
+    """
+    clip = VideoClip()
+    other_clip = VideoClip()
+
+    for attr in clip.__dict__:
+        # mask and audio are shallow copies that should be initialized
+        if attr in ("mask", "audio"):
+            if attr == "mask":
+                nested_object = BitmapClip([["R"]], duration=0.01)
+            else:
+                nested_object = AudioClip(
+                    lambda t: [np.sin(880 * 2 * np.pi * t)], duration=0.01, fps=44100
+                )
+            setattr(clip, attr, nested_object)
+        else:
+            setattr(clip, attr, "foo")
+
+    copied_clip = copy_func(clip)
+
+    # VideoClip attributes are copied
+    for attr in copied_clip.__dict__:
+        value = getattr(copied_clip, attr)
+        assert value == getattr(clip, attr)
+
+        # other instances are not edited
+        assert value != getattr(other_clip, attr)
+
+        # shallow copies of mask and audio
+        if attr in ("mask", "audio"):
+            for nested_attr in value.__dict__:
+                assert getattr(value, nested_attr) == getattr(
+                    getattr(clip, attr), nested_attr
+                )
+
+    # nested objects of instances copies are not edited
+    assert other_clip.mask is None
+    assert other_clip.audio is None
+
+
 if __name__ == "__main__":
     pytest.main()