Fix 'fps' not defined for 'CompositeAudioClip' at initialization (#1462)

* Fix 'fps' not being defined for 'CompositeAudioClip' if not created through 'concatenate_audioclips'

* Minor change in test

* Add CHANGELOG entry

* Rename test

* Fix error

* Improve 'CompositeAudioClip' initialization

* Improve attribute build

CHANGELOG.md

@@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `AudioClip.max_volume(stereo=True)` now can return more than 2 channels [\#1464](https://github.com/Zulko/moviepy/pull/1464)
 - Fixed `Clip.cutout` transformation not being applied to audio [\#1468](https://github.com/Zulko/moviepy/pull/1468)
 - Fixed arguments inconsistency in `video.tools.drawing.color_gradient` [\#1467](https://github.com/Zulko/moviepy/pull/1467)
+- Fixed `fps` not defined in `CompositeAudioClip` at initialization [\#1462](https://github.com/Zulko/moviepy/pull/1462)
 
 
 ## [v2.0.0.dev2](https://github.com/zulko/moviepy/tree/v2.0.0.dev2) (2020-10-05)

moviepy/audio/AudioClip.py

@@ -1,3 +1,4 @@
+import numbers
 import os
 
 import numpy as np
@@ -308,53 +309,68 @@ class CompositeAudioClip(AudioClip):
 
     clips
       List of audio clips, which may start playing at different times or
-      together. If all have their ``duration`` attribute set, the
-      duration of the composite clip is computed automatically.
+      together, depends on their ``start`` attributes. If all have their
+      ``duration`` attribute set, the duration of the composite clip is
+      computed automatically.
 
     """
 
     def __init__(self, clips):
-
-        Clip.__init__(self)
         self.clips = clips
+        self.nchannels = max(clip.nchannels for clip in self.clips)
 
-        ends = [clip.end for clip in self.clips]
-        self.nchannels = max([clip.nchannels for clip in self.clips])
-        if not any([(end is None) for end in ends]):
-            self.duration = max(ends)
-            self.end = max(ends)
+        # self.duration is setted at AudioClip
+        duration = None
+        for end in self.ends:
+            if end is None:
+                break
+            duration = max(end, duration or 0)
 
-        def make_frame(t):
+        # self.fps is setted at AudioClip
+        fps = None
+        for clip in self.clips:
+            if hasattr(clip, "fps") and isinstance(clip.fps, numbers.Number):
+                fps = max(clip.fps, fps or 0)
 
-            played_parts = [clip.is_playing(t) for clip in self.clips]
+        super().__init__(duration=duration, fps=fps)
 
-            sounds = [
-                clip.get_frame(t - clip.start) * np.array([part]).T
-                for clip, part in zip(self.clips, played_parts)
-                if (part is not False)
-            ]
+    @property
+    def starts(self):
+        return (clip.start for clip in self.clips)
 
-            if isinstance(t, np.ndarray):
-                zero = np.zeros((len(t), self.nchannels))
+    @property
+    def ends(self):
+        return (clip.end for clip in self.clips)
 
-            else:
-                zero = np.zeros(self.nchannels)
+    def make_frame(self, t):
+        played_parts = [clip.is_playing(t) for clip in self.clips]
 
-            return zero + sum(sounds)
+        sounds = [
+            clip.get_frame(t - clip.start) * np.array([part]).T
+            for clip, part in zip(self.clips, played_parts)
+            if (part is not False)
+        ]
 
-        self.make_frame = make_frame
+        if isinstance(t, np.ndarray):
+            zero = np.zeros((len(t), self.nchannels))
+        else:
+            zero = np.zeros(self.nchannels)
+
+        return zero + sum(sounds)
 
 
 def concatenate_audioclips(clips):
-    """
-    The clip with the highest FPS will be the FPS of the result clip.
-    """
-    durations = [clip.duration for clip in clips]
-    timings = np.cumsum([0] + durations)  # start times, and end time.
-    newclips = [clip.with_start(t) for clip, t in zip(clips, timings)]
+    """Concatenates one AudioClip after another, in the order that are passed
+    to ``clips`` parameter.
+
+    Parameters
+    ----------
 
-    result = CompositeAudioClip(newclips).with_duration(timings[-1])
+    clips
+      List of audio clips, which will be played one after other.
+    """
+    # start, end/start2, end2/start3... end
+    starts_end = np.cumsum([0, *[clip.duration for clip in clips]])
+    newclips = [clip.with_start(t) for clip, t in zip(clips, starts_end[:-1])]
 
-    fpss = [clip.fps for clip in clips if getattr(clip, "fps", None)]
-    result.fps = max(fpss) if fpss else None
-    return result
+    return CompositeAudioClip(newclips).with_duration(starts_end[-1])

tests/test_AudioClips.py

@@ -8,6 +8,7 @@
 from moviepy.audio.AudioClip import (
     AudioArrayClip,
     AudioClip,
+    CompositeAudioClip,
     concatenate_audioclips,
 )
 from moviepy.audio.io.AudioFileClip import AudioFileClip
@@ -50,51 +51,137 @@ def test_audioclip_io():
     )
     assert (output_array[len(input_array) :] == 0).all()
 
+    close_all_clips(locals())
+
 
-def test_audioclip_concat():
+def test_concatenate_audioclips_render():
+    """Concatenated AudioClips through ``concatenate_audioclips`` should return
+    a clip that can be rendered to a file.
+    """
     make_frame_440 = lambda t: [np.sin(440 * 2 * np.pi * t)]
     make_frame_880 = lambda t: [np.sin(880 * 2 * np.pi * t)]
 
-    clip1 = AudioClip(make_frame_440, duration=1, fps=44100)
-    clip2 = AudioClip(make_frame_880, duration=2, fps=22050)
+    clip_440 = AudioClip(make_frame_440, duration=0.01, fps=44100)
+    clip_880 = AudioClip(make_frame_880, duration=0.000001, fps=22050)
 
-    concat_clip = concatenate_audioclips((clip1, clip2))
-    # concatenate_audioclips should return a clip with an fps of the greatest
-    # fps passed into it
+    concat_clip = concatenate_audioclips((clip_440, clip_880))
+    concat_clip.write_audiofile(os.path.join(TMP_DIR, "concatenate_audioclips.mp3"))
+
+    assert concat_clip.duration == clip_440.duration + clip_880.duration
+    close_all_clips(locals())
+
+
+def test_concatenate_audioclips_CompositeAudioClip():
+    """Concatenated AudioClips through ``concatenate_audioclips`` should return
+    a CompositeAudioClip whose attributes should be consistent:
+
+    - Returns CompositeAudioClip.
+    - Their fps is taken from the maximum of their audios.
+    - Audios are placed one after other:
+      - Duration is the sum of their durations.
+      - Ends are the accumulated sum of their durations.
+      - Starts are the accumulated sum of their durations, but first start is 0
+        and lastest is ignored.
+    - Channels are the max channels of their clips.
+    """
+    frequencies = [440, 880, 1760]
+    durations = [2, 5, 1]
+    fpss = [44100, 22050, 11025]
+
+    clips = [
+        AudioClip(
+            lambda t: [np.sin(frequency * 2 * np.pi * t)], duration=duration, fps=fps
+        )
+        for frequency, duration, fps in zip(frequencies, durations, fpss)
+    ]
+
+    concat_clip = concatenate_audioclips(clips)
+
+    # should return a CompositeAudioClip
+    assert isinstance(concat_clip, CompositeAudioClip)
+
+    # fps of the greatest fps passed into it
     assert concat_clip.fps == 44100
 
-    return
-    # Does run without errors, but the length of the audio is way to long,
-    # so it takes ages to run.
-    concat_clip.write_audiofile(os.path.join(TMP_DIR, "concat_audioclip.mp3"))
+    # audios placed on after other
+    assert concat_clip.duration == sum(durations)
+    assert list(concat_clip.ends) == list(np.cumsum(durations))
+    assert list(concat_clip.starts), list(np.cumsum([0, *durations[:-1]]))
 
+    # channels are maximum number of channels of the clips
+    assert concat_clip.nchannels == max(clip.nchannels for clip in clips)
 
-def test_audioclip_with_file_concat():
-    make_frame_440 = lambda t: [np.sin(440 * 2 * np.pi * t)]
-    clip1 = AudioClip(make_frame_440, duration=1, fps=44100)
+    close_all_clips(locals())
+
+
+def test_CompositeAudioClip_by__init__():
+    """The difference between the CompositeAudioClip returned by
+    ``concatenate_audioclips`` and a CompositeAudioClip created using the class
+    directly, is that audios in ``concatenate_audioclips`` are played one after
+    other and AudioClips passed to CompositeAudioClip can be played at different
+    times, it depends on their ``start`` attributes.
+    """
+    frequencies = [440, 880, 1760]
+    durations = [2, 5, 1]
+    fpss = [44100, 22050, 11025]
+    starts = [0, 1, 2]
+
+    clips = [
+        AudioClip(
+            lambda t: [np.sin(frequency * 2 * np.pi * t)], duration=duration, fps=fps
+        ).with_start(start)
+        for frequency, duration, fps, start in zip(frequencies, durations, fpss, starts)
+    ]
+
+    compound_clip = CompositeAudioClip(clips)
+
+    # should return a CompositeAudioClip
+    assert isinstance(compound_clip, CompositeAudioClip)
 
+    # fps of the greatest fps passed into it
+    assert compound_clip.fps == 44100
+
+    # duration depends on clips starts and durations
+    ends = [start + duration for start, duration in zip(starts, durations)]
+    assert compound_clip.duration == max(ends)
+    assert list(compound_clip.ends) == ends
+    assert list(compound_clip.starts) == starts
+
+    # channels are maximum number of channels of the clips
+    assert compound_clip.nchannels == max(clip.nchannels for clip in clips)
+
+    close_all_clips(locals())
+
+
+def test_concatenate_audioclip_with_audiofileclip():
+    # stereo A note
+    make_frame = lambda t: np.array(
+        [np.sin(440 * 2 * np.pi * t), np.sin(880 * 2 * np.pi * t)]
+    ).T
+
+    clip1 = AudioClip(make_frame, duration=1, fps=44100)
     clip2 = AudioFileClip("media/crunching.mp3")
 
     concat_clip = concatenate_audioclips((clip1, clip2))
-
-    return
-    # Fails with strange error
-    # "ValueError: operands could not be broadcast together with
-    # shapes (1993,2) (1993,1993)1
     concat_clip.write_audiofile(
         os.path.join(TMP_DIR, "concat_clip_with_file_audio.mp3")
     )
 
+    assert concat_clip.duration == clip1.duration + clip2.duration
+
 
-def test_audiofileclip_concat():
-    sound = AudioFileClip("media/crunching.mp3")
-    sound = sound.subclip(1, 4)
+def test_concatenate_audiofileclips():
+    clip1 = AudioFileClip("media/crunching.mp3").subclip(1, 4)
 
     # Checks it works with videos as well
-    sound2 = AudioFileClip("media/big_buck_bunny_432_433.webm")
-    concat = concatenate_audioclips((sound, sound2))
+    clip2 = AudioFileClip("media/big_buck_bunny_432_433.webm")
+    concat_clip = concatenate_audioclips((clip1, clip2))
 
-    concat.write_audiofile(os.path.join(TMP_DIR, "concat_audio_file.mp3"))
+    concat_clip.write_audiofile(os.path.join(TMP_DIR, "concat_audio_file.mp3"))
+
+    assert concat_clip.duration == clip1.duration + clip2.duration
+
+    close_all_clips(locals())
 
 
 def test_audioclip_mono_max_volume():