From ac2ef2df522e5f9bc73faa23d1ddae5cf7307a17 Mon Sep 17 00:00:00 2001 From: osaajani <> Date: Sat, 11 Jan 2025 17:38:17 +0100 Subject: [PATCH 1/2] Add exception on outside of boundaries subclip --- CHANGELOG.md | 1 + moviepy/Clip.py | 8 ++++++++ tests/test_Clip.py | 1 + tests/test_VideoClip.py | 13 +++++++------ 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6319cb9d0..2bad3dea1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add background radius to text clips ### Changed +- Subclipping outside of clip boundaries now raise an exception ### Deprecated diff --git a/moviepy/Clip.py b/moviepy/Clip.py index 871ed8e3e..dfd8bc502 100644 --- a/moviepy/Clip.py +++ b/moviepy/Clip.py @@ -402,6 +402,7 @@ def subclipped(self, start_time=0, end_time=None): + "should be smaller than the clip's " + "duration (%.02f)." % self.duration ) + new_clip = self.time_transform(lambda t: t + start_time, apply_to=[]) @@ -422,6 +423,13 @@ def subclipped(self, start_time=0, end_time=None): end_time = self.duration + end_time if end_time is not None: + if (self.duration is not None) and (end_time > self.duration): + raise ValueError( + "end_time (%.02f) " % end_time + + "should be smaller or equal to the clip's " + + "duration (%.02f)." % self.duration + ) + new_clip.duration = end_time - start_time new_clip.end = new_clip.start + new_clip.duration diff --git a/tests/test_Clip.py b/tests/test_Clip.py index 397f228bb..0dcabf841 100644 --- a/tests/test_Clip.py +++ b/tests/test_Clip.py @@ -184,6 +184,7 @@ def test_clip_copy(copy_func): (3, 3, None, ValueError), # start_time == duration (3, 1, -1, 1), # negative end_time (None, 1, -1, ValueError), # negative end_time for clip without duration + (1, 0, 2, ValueError), # end_time after video end should raise exception ), ) def test_clip_subclip(duration, start_time, end_time, expected_duration): diff --git a/tests/test_VideoClip.py b/tests/test_VideoClip.py index 53c04817b..238133f4f 100644 --- a/tests/test_VideoClip.py +++ b/tests/test_VideoClip.py @@ -389,16 +389,17 @@ def test_afterimage(util): def test_add(): clip = VideoFileClip("media/fire2.mp4") - new_clip = clip[0:1] + clip[2:3.2] - assert new_clip.duration == 2.2 - assert np.array_equal(new_clip[1.1], clip[2.1]) + print(clip.duration) + new_clip = clip[0:1] + clip[1.5:2] + assert new_clip.duration == 1.5 + assert np.array_equal(new_clip[1.1], clip[1.6]) def test_slice_tuples(): clip = VideoFileClip("media/fire2.mp4") - new_clip = clip[0:1, 2:3.2] - assert new_clip.duration == 2.2 - assert np.array_equal(new_clip[1.1], clip[2.1]) + new_clip = clip[0:1, 1.5:2] + assert new_clip.duration == 1.5 + assert np.array_equal(new_clip[1.1], clip[1.6]) def test_slice_mirror(): From 1ab989ae410df05d1b3650abfbc7960db741ac17 Mon Sep 17 00:00:00 2001 From: osaajani <> Date: Sat, 11 Jan 2025 22:22:30 +0100 Subject: [PATCH 2/2] Add some tolerance to account for rounding errors --- moviepy/Clip.py | 6 +++--- moviepy/video/io/ffmpeg_reader.py | 12 ++++++------ tests/test_VideoClip.py | 1 - tests/test_issues.py | 7 +++---- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/moviepy/Clip.py b/moviepy/Clip.py index dfd8bc502..258fcd193 100644 --- a/moviepy/Clip.py +++ b/moviepy/Clip.py @@ -402,7 +402,6 @@ def subclipped(self, start_time=0, end_time=None): + "should be smaller than the clip's " + "duration (%.02f)." % self.duration ) - new_clip = self.time_transform(lambda t: t + start_time, apply_to=[]) @@ -423,13 +422,14 @@ def subclipped(self, start_time=0, end_time=None): end_time = self.duration + end_time if end_time is not None: - if (self.duration is not None) and (end_time > self.duration): + # Allow a slight tolerance to account for rounding errors + if (self.duration is not None) and (end_time - self.duration > 0.00000001): raise ValueError( "end_time (%.02f) " % end_time + "should be smaller or equal to the clip's " + "duration (%.02f)." % self.duration ) - + new_clip.duration = end_time - start_time new_clip.end = new_clip.start + new_clip.duration diff --git a/moviepy/video/io/ffmpeg_reader.py b/moviepy/video/io/ffmpeg_reader.py index 41de84142..8f6835aa6 100644 --- a/moviepy/video/io/ffmpeg_reader.py +++ b/moviepy/video/io/ffmpeg_reader.py @@ -476,12 +476,12 @@ def parse(self): # for default streams, set their numbers globally, so it's # easy to get without iterating all if self._current_stream["default"]: - self.result[f"default_{stream_type_lower}_input_number"] = ( - input_number - ) - self.result[f"default_{stream_type_lower}_stream_number"] = ( - stream_number - ) + self.result[ + f"default_{stream_type_lower}_input_number" + ] = input_number + self.result[ + f"default_{stream_type_lower}_stream_number" + ] = stream_number # exit chapter if self._current_chapter: diff --git a/tests/test_VideoClip.py b/tests/test_VideoClip.py index 238133f4f..6cd61bb76 100644 --- a/tests/test_VideoClip.py +++ b/tests/test_VideoClip.py @@ -389,7 +389,6 @@ def test_afterimage(util): def test_add(): clip = VideoFileClip("media/fire2.mp4") - print(clip.duration) new_clip = clip[0:1] + clip[1.5:2] assert new_clip.duration == 1.5 assert np.array_equal(new_clip[1.1], clip[1.6]) diff --git a/tests/test_issues.py b/tests/test_issues.py index ae2d6acda..79a086df9 100644 --- a/tests/test_issues.py +++ b/tests/test_issues.py @@ -303,9 +303,8 @@ def test_issue_470(util): audio_clip = AudioFileClip("media/crunching.mp3") # end_time is out of bounds - subclip = audio_clip.subclipped(start_time=6, end_time=9) - - with pytest.raises(IOError): + with pytest.raises(ValueError): + subclip = audio_clip.subclipped(start_time=6, end_time=9) subclip.write_audiofile(wav_filename, write_logfile=True) # but this one should work.. @@ -334,7 +333,7 @@ def test_issue_636(): def test_issue_655(): video_file = "media/fire2.mp4" - for subclip in [(0, 2), (1, 2), (2, 3)]: + for subclip in [(0, 2), (1, 2), (2, 2.10)]: with VideoFileClip(video_file) as v: with v.subclipped(1, 2) as _: pass