Repository: Zulko/moviepy
Pull request #2331 (2025-01-17 14:38:45)

Add support for new argument ch_layout on ffmpeg >= 7 and make compatible with ffmpeg 7
Adding support for most recent version of ffplay who dropped support of the old -ac param in favor of ch_layout.
Also make moviepy compatible with latests change introduced with ffmpeg v7


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1d8f0b4..25cc039 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,7 +18,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Support pillow 11
 
 ### Changed <!-- for changes in existing functionality -->
-- Subclipping outside of clip boundaries now raise an exception
 
 ### Deprecated <!-- for soon-to-be removed features -->
 
@@ -30,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Improve perfs of decorator by pre-computing arguments
 - Fix textclip being cut or of impredictable height (see issues #2325, #2260 and #2268)
 - Fix TimeMirror and TimeSymmetrize cutting last second of clip
+- Fix audiopreview not working with ffplay >= 7.0.0
 
 ## [v2.1.2](https://github.com/zulko/moviepy/tree/master)
 
diff --git a/moviepy/Clip.py b/moviepy/Clip.py
index 2d4c0b2..e931f55 100644
--- a/moviepy/Clip.py
+++ b/moviepy/Clip.py
@@ -422,14 +422,6 @@ class Clip:
                 end_time = self.duration + end_time
 
         if end_time is not None:
-            # 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/audio/io/ffplay_audiopreviewer.py b/moviepy/audio/io/ffplay_audiopreviewer.py
index e1217ab..c7bdb25 100644
--- a/moviepy/audio/io/ffplay_audiopreviewer.py
+++ b/moviepy/audio/io/ffplay_audiopreviewer.py
@@ -5,6 +5,7 @@ import subprocess as sp
 from moviepy.config import FFPLAY_BINARY
 from moviepy.decorators import requires_duration
 from moviepy.tools import cross_platform_popen_params
+from moviepy.video.io import ffmpeg_tools
 
 
 class FFPLAY_AudioPreviewer:
@@ -24,7 +25,6 @@ class FFPLAY_AudioPreviewer:
 
     nchannels:
       Number of audio channels in the clip. Default to 2 channels.
-
     """
 
     def __init__(
@@ -42,8 +42,22 @@ class FFPLAY_AudioPreviewer:
             "s%dle" % (8 * nbytes),
             "-ar",
             "%d" % fps_input,
-            "-ac",
-            "%d" % nchannels,
+        ]
+
+        # Adapt number of channels argument to ffplay version
+        ffplay_version = ffmpeg_tools.ffplay_version()[1]
+        if int(ffplay_version.split(".")[0]) >= 7:
+            cmd += [
+                "-ch_layout",
+                "stereo" if nchannels == 2 else "mono",
+            ]
+        else:
+            cmd += [
+                "-ac",
+                "%d" % nchannels,
+            ]
+
+        cmd += [
             "-i",
             "-",
         ]
@@ -62,11 +76,6 @@ class FFPLAY_AudioPreviewer:
             _, ffplay_error = self.proc.communicate()
             if ffplay_error is not None:
                 ffplay_error = ffplay_error.decode()
-            else:
-                # The error was redirected to a logfile with `write_logfile=True`,
-                # so read the error from that file instead
-                self.logfile.seek(0)
-                ffplay_error = self.logfile.read()
 
             error = (
                 f"{err}\n\nMoviePy error: FFPLAY encountered the following error while "
diff --git a/moviepy/version.py b/moviepy/version.py
index 58039f5..4eabd0b 100644
--- a/moviepy/version.py
+++ b/moviepy/version.py
@@ -1,1 +1,1 @@
-__version__ = "2.1.1"
+__version__ = "2.1.2"
diff --git a/moviepy/video/io/ffmpeg_reader.py b/moviepy/video/io/ffmpeg_reader.py
index 8f6835a..c9d073b 100644
--- a/moviepy/video/io/ffmpeg_reader.py
+++ b/moviepy/video/io/ffmpeg_reader.py
@@ -418,7 +418,7 @@ class FFmpegInfosParser:
                     self.result["duration"] = self.parse_duration(line)
 
                 # parse global bitrate (in kb/s)
-                bitrate_match = re.search(r"bitrate: (\d+) kb/s", line)
+                bitrate_match = re.search(r"bitrate: (\d+) k(i?)b/s", line)
                 self.result["bitrate"] = (
                     int(bitrate_match.group(1)) if bitrate_match else None
                 )
@@ -476,12 +476,12 @@ class FFmpegInfosParser:
                 # 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:
@@ -528,8 +528,11 @@ class FFmpegInfosParser:
 
                 if self._current_stream["stream_type"] == "video":
                     field, value = self.video_metadata_type_casting(field, value)
+                    # ffmpeg 7 now use displaymatrix instead of rotate
                     if field == "rotate":
                         self.result["video_rotation"] = value
+                    elif field == "displaymatrix":
+                        self.result["video_rotation"] = value
 
                 # multiline metadata value parsing
                 if field == "":
@@ -644,7 +647,7 @@ class FFmpegInfosParser:
             # AttributeError: 'NoneType' object has no attribute 'group'
             # ValueError: invalid literal for int() with base 10: '<string>'
             stream_data["fps"] = "unknown"
-        match_audio_bitrate = re.search(r"(\d+) kb/s", line)
+        match_audio_bitrate = re.search(r"(\d+) k(i?)b/s", line)
         stream_data["bitrate"] = (
             int(match_audio_bitrate.group(1)) if match_audio_bitrate else None
         )
@@ -672,7 +675,7 @@ class FFmpegInfosParser:
                 % (self.filename, self.infos)
             )
 
-        match_bitrate = re.search(r"(\d+) kb/s", line)
+        match_bitrate = re.search(r"(\d+) k(i?)b/s", line)
         stream_data["bitrate"] = int(match_bitrate.group(1)) if match_bitrate else None
 
         # Get the frame rate. Sometimes it's 'tbr', sometimes 'fps', sometimes
@@ -785,6 +788,14 @@ class FFmpegInfosParser:
         """Cast needed video metadata fields to other types than the default str."""
         if field == "rotate":
             return (field, float(value))
+
+        elif field == "displaymatrix":
+            match = re.search(r"[-+]?\d+(\.\d+)?", value)
+            if match:
+                # We must multiply by -1 because displaymatrix return info
+                # about how to rotate to show video, not about video rotation
+                return (field, float(match.group()) * -1)
+
         return (field, value)
 
 
diff --git a/moviepy/video/io/ffmpeg_tools.py b/moviepy/video/io/ffmpeg_tools.py
index e9716ff..c154d9b 100644
--- a/moviepy/video/io/ffmpeg_tools.py
+++ b/moviepy/video/io/ffmpeg_tools.py
@@ -1,8 +1,10 @@
 """Miscellaneous bindings to ffmpeg."""
 
 import os
+import re
+import subprocess
 
-from moviepy.config import FFMPEG_BINARY
+from moviepy.config import FFMPEG_BINARY, FFPLAY_BINARY
 from moviepy.decorators import convert_parameter_to_seconds, convert_path_to_string
 from moviepy.tools import ffmpeg_escape_filename, subprocess_call
 
@@ -207,3 +209,81 @@ def ffmpeg_stabilize_video(
         cmd.append("-y")
 
     subprocess_call(cmd, logger=logger)
+
+
+def ffmpeg_version():
+    """
+    Retrieve the FFmpeg version.
+
+    This function retrieves both the full and numeric version of FFmpeg
+    by executing the `ffmpeg -version` command. The full version includes
+    additional details like build information, while the numeric version
+    contains only the version numbers (e.g., '7.0.2').
+
+    Return
+    ------
+    tuple
+        A tuple containing:
+        - `full_version` (str): The complete version string (e.g., '7.0.2-static').
+        - `numeric_version` (str): The numeric version string (e.g., '7.0.2').
+
+    Example
+    -------
+    >>> ffmpeg_version()
+    ('7.0.2-static', '7.0.2')
+
+    Raises
+    ------
+    subprocess.CalledProcessError
+        If the FFmpeg command fails to execute properly.
+    """
+    cmd = [
+        FFMPEG_BINARY,
+        "-version",
+        "-v", "quiet",
+    ]
+
+    result = subprocess.run(cmd, capture_output=True, text=True, check=True)
+
+    # Extract the version number from the first line of output
+    full_version = result.stdout.splitlines()[0].split()[2]
+    numeric_version = re.match(r"^[0-9.]+", full_version).group(0)
+    return (full_version, numeric_version)
+
+
+def ffplay_version():
+    """
+    Retrieve the FFplay version.
+
+    This function retrieves both the full and numeric version of FFplay
+    by executing the `ffplay -version` command. The full version includes
+    additional details like build information, while the numeric version
+    contains only the version numbers (e.g., '6.0.1').
+
+    Return
+    ------
+    tuple
+        A tuple containing:
+        - `full_version` (str): The complete version string (e.g., '6.0.1-static').
+        - `numeric_version` (str): The numeric version string (e.g., '6.0.1').
+
+    Example
+    -------
+    >>> ffplay_version()
+    ('6.0.1-static', '6.0.1')
+
+    Raises
+    ------
+    subprocess.CalledProcessError
+        If the FFplay command fails to execute properly.
+    """
+    cmd = [
+        FFPLAY_BINARY,
+        "-version",
+    ]
+
+    result = subprocess.run(cmd, capture_output=True, text=True, check=True)
+    # Extract the version number from the first line of output
+    full_version = result.stdout.splitlines()[0].split()[2]
+    numeric_version = re.match(r"^[0-9.]+", full_version).group(0)
+    return (full_version, numeric_version)


Test patch:
diff --git a/tests/test_Clip.py b/tests/test_Clip.py
index 0dcabf8..397f228 100644
--- a/tests/test_Clip.py
+++ b/tests/test_Clip.py
@@ -184,7 +184,6 @@ 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_PR.py b/tests/test_PR.py
index 85bb2f3..0acd3f6 100644
--- a/tests/test_PR.py
+++ b/tests/test_PR.py
@@ -69,6 +69,8 @@ def test_PR_528(util):
 
 
 def test_PR_529():
+    # print(ffmpeg_tools.ffplay_version())
+    print(ffmpeg_tools.ffmpeg_version())
     with VideoFileClip("media/fire2.mp4") as video_clip:
         assert video_clip.rotation == 180
 
diff --git a/tests/test_VideoClip.py b/tests/test_VideoClip.py
index 6cd61bb..53c0481 100644
--- a/tests/test_VideoClip.py
+++ b/tests/test_VideoClip.py
@@ -389,16 +389,16 @@ def test_afterimage(util):
 
 def test_add():
     clip = VideoFileClip("media/fire2.mp4")
-    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])
+    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])
 
 
 def test_slice_tuples():
     clip = VideoFileClip("media/fire2.mp4")
-    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])
+    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])
 
 
 def test_slice_mirror():
diff --git a/tests/test_ffmpeg_reader.py b/tests/test_ffmpeg_reader.py
index 40a520c..4269616 100644
--- a/tests/test_ffmpeg_reader.py
+++ b/tests/test_ffmpeg_reader.py
@@ -16,6 +16,7 @@ from moviepy.video.io.ffmpeg_reader import (
     FFmpegInfosParser,
     ffmpeg_parse_infos,
 )
+from moviepy.video.io.ffmpeg_tools import ffmpeg_version
 from moviepy.video.io.VideoFileClip import VideoFileClip
 from moviepy.video.VideoClip import BitmapClip, ColorClip
 
@@ -59,7 +60,7 @@ def test_ffmpeg_parse_infos_video_nframes():
     ("decode_file", "expected_duration"),
     (
         (False, 30),
-        (True, 30.02),
+        (True, 30),
     ),
     ids=(
         "decode_file=False",
@@ -69,6 +70,11 @@ def test_ffmpeg_parse_infos_video_nframes():
 def test_ffmpeg_parse_infos_decode_file(decode_file, expected_duration):
     """Test `decode_file` argument of `ffmpeg_parse_infos` function."""
     d = ffmpeg_parse_infos("media/big_buck_bunny_0_30.webm", decode_file=decode_file)
+
+    # On old version of ffmpeg, duration and video duration was different
+    if int(ffmpeg_version()[1].split(".")[0]) < 7:
+        expected_duration += 0.02
+
     assert d["duration"] == expected_duration
 
     # check metadata is fine
diff --git a/tests/test_ffmpeg_tools.py b/tests/test_ffmpeg_tools.py
index bc9a832..af66de2 100644
--- a/tests/test_ffmpeg_tools.py
+++ b/tests/test_ffmpeg_tools.py
@@ -9,6 +9,7 @@ from moviepy.video.io.ffmpeg_tools import (
     ffmpeg_extract_subclip,
     ffmpeg_resize,
     ffmpeg_stabilize_video,
+    ffmpeg_version,
 )
 from moviepy.video.io.VideoFileClip import VideoFileClip
 
@@ -57,9 +58,10 @@ def test_ffmpeg_resize(util):
     ffmpeg_resize("media/bitmap.mp4", outputfile, expected_size, logger=None)
     assert os.path.isfile(outputfile)
 
-    # overwrite file
-    with pytest.raises(OSError):
-        ffmpeg_resize("media/bitmap.mp4", outputfile, expected_size, logger=None)
+    # overwrite file on old version of ffmpeg
+    if int(ffmpeg_version()[1].split(".")[0]) < 7:
+        with pytest.raises(OSError):
+            ffmpeg_resize("media/bitmap.mp4", outputfile, expected_size, logger=None)
 
     clip = VideoFileClip(outputfile)
     assert clip.size[0] == expected_size[0]
@@ -98,15 +100,16 @@ def test_ffmpeg_stabilize_video(util):
     expected_filepath = os.path.join(stabilize_video_tempdir, "foo.mp4")
     assert os.path.isfile(expected_filepath)
 
-    # don't overwrite file
-    with pytest.raises(OSError):
-        ffmpeg_stabilize_video(
-            "media/bitmap.mp4",
-            output_dir=stabilize_video_tempdir,
-            outputfile="foo.mp4",
-            overwrite_file=False,
-            logger=None,
-        )
+    # don't overwrite file on old version of ffmpeg
+    if int(ffmpeg_version()[1].split(".")[0]) < 7:
+        with pytest.raises(OSError):
+            ffmpeg_stabilize_video(
+                "media/bitmap.mp4",
+                output_dir=stabilize_video_tempdir,
+                outputfile="foo.mp4",
+                overwrite_file=False,
+                logger=None,
+            )
 
     if os.path.isdir(stabilize_video_tempdir):
         try:
diff --git a/tests/test_issues.py b/tests/test_issues.py
index 79a086d..ae2d6ac 100644
--- a/tests/test_issues.py
+++ b/tests/test_issues.py
@@ -303,8 +303,9 @@ def test_issue_470(util):
     audio_clip = AudioFileClip("media/crunching.mp3")
 
     # end_time is out of bounds
-    with pytest.raises(ValueError):
-        subclip = audio_clip.subclipped(start_time=6, end_time=9)
+    subclip = audio_clip.subclipped(start_time=6, end_time=9)
+
+    with pytest.raises(IOError):
         subclip.write_audiofile(wav_filename, write_logfile=True)
 
     # but this one should work..
@@ -333,7 +334,7 @@ def test_issue_636():
 
 def test_issue_655():
     video_file = "media/fire2.mp4"
-    for subclip in [(0, 2), (1, 2), (2, 2.10)]:
+    for subclip in [(0, 2), (1, 2), (2, 3)]:
         with VideoFileClip(video_file) as v:
             with v.subclipped(1, 2) as _:
                 pass