[backends] Fix behavior of MoviePy backend (#462)

* [backends] Fix behavior of MoviePy backend

There are still some fixes required in MoviePy itself to make corrupt video tests pass.

* [backends] Fix MoviePy 2.0 EOF behavior

Skip corrupt video test on MoviePy awaiting Zulko/moviepy#2253

* [build] Enable Moviepy tests for builds and document workarounds for #461

.github/workflows/build.yml

@@ -62,6 +62,12 @@ jobs:
           pip install av opencv-python-headless --only-binary :all:
           pip install -r requirements_headless.txt
 
+      - name: Install MoviePy
+        # TODO: We can only run MoviePy tests on systems that have ffmpeg.
+        if: ${{ runner.arch == 'X64' }}
+        run: |
+          pip install moviepy
+
       - name: Checkout test resources
         run: |
           git fetch --depth=1 https://github.com/Breakthrough/PySceneDetect.git refs/heads/resources:refs/remotes/origin/resources

scenedetect/_cli/controller.py

@@ -16,8 +16,10 @@
 import os
 import time
 import typing as ty
+import warnings
 
 from scenedetect._cli.context import CliContext
+from scenedetect.backends import VideoStreamCv2, VideoStreamMoviePy
 from scenedetect.frame_timecode import FrameTimecode
 from scenedetect.platform import get_and_create_path
 from scenedetect.scene_manager import CutList, SceneList, get_scenes_from_cuts
@@ -39,6 +41,12 @@ def run_scenedetect(context: CliContext):
         logger.debug("No input specified.")
         return
 
+    # Suppress warnings when reading past EOF in MoviePy (#461).
+    if VideoStreamMoviePy and isinstance(context.video_stream, VideoStreamMoviePy):
+        is_debug = context.config.get_value("global", "verbosity") != "debug"
+        if not is_debug:
+            warnings.filterwarnings("ignore", module="moviepy")
+
     if context.load_scenes_input:
         # Skip detection if load-scenes was used.
         logger.info("Skipping detection, loading scenes from: %s", context.load_scenes_input)
@@ -49,7 +57,10 @@ def run_scenedetect(context: CliContext):
         logger.info("Loaded %d scenes.", len(scenes))
     else:
         # Perform scene detection on input.
-        scenes, cuts = _detect(context)
+        result = _detect(context)
+        if result is None:
+            return
+        scenes, cuts = result
         scenes = _postprocess_scene_list(context, scenes)
         # Handle -s/--stats option.
         _save_stats(context)
@@ -110,7 +121,7 @@ def _detect(context: CliContext) -> ty.Optional[ty.Tuple[SceneList, CutList]]:
 
     # Handle case where video failure is most likely due to multiple audio tracks (#179).
     # TODO(#380): Ensure this does not erroneusly fire.
-    if num_frames <= 0 and context.video_stream.BACKEND_NAME == "opencv":
+    if num_frames <= 0 and isinstance(context.video_stream, VideoStreamCv2):
         logger.critical(
             "Failed to read any frames from video file. This could be caused by the video"
             " having multiple audio tracks. If so, try installing the PyAV backend:\n"

scenedetect/backends/moviepy.py

@@ -174,29 +174,38 @@ def seek(self, target: Union[FrameTimecode, float, int]):
             SeekError: An error occurs while seeking, or seeking is not supported.
             ValueError: `target` is not a valid value (i.e. it is negative).
         """
+        success = False
         if not isinstance(target, FrameTimecode):
             target = FrameTimecode(target, self.frame_rate)
         try:
-            self._reader.get_frame(target.get_seconds())
+            self._last_frame = self._reader.get_frame(target.get_seconds())
+            if hasattr(self._reader, "last_read") and target >= self.duration:
+                raise SeekError("MoviePy > 2.0 does not have proper EOF semantics (#461).")
+            self._frame_number = min(
+                target.frame_num,
+                FrameTimecode(self._reader.infos["duration"], self.frame_rate).frame_num - 1,
+            )
+            success = True
         except OSError as ex:
-            # Leave the object in a valid state.
-            self.reset()
             # TODO(#380): Other backends do not currently throw an exception if attempting to seek
             # past EOF. We need to ensure consistency for seeking past end of video with respect to
             # errors and behaviour, and should probably gracefully stop at the last frame instead
             # of throwing an exception.
             if target >= self.duration:
                 raise SeekError("Target frame is beyond end of video!") from ex
             raise
-        self._last_frame = self._reader.lastread
-        self._frame_number = target.frame_num
+        finally:
+            # Leave the object in a valid state on any errors.
+            if not success:
+                self.reset()
 
-    def reset(self):
+    def reset(self, print_infos=False):
         """Close and re-open the VideoStream (should be equivalent to calling `seek(0)`)."""
-        self._reader.initialize()
-        self._last_frame = self._reader.read_frame()
+        self._last_frame = False
+        self._last_frame_rgb = None
         self._frame_number = 0
         self._eof = False
+        self._reader = FFMPEG_VideoReader(self._path, print_infos=print_infos)
 
     def read(self, decode: bool = True, advance: bool = True) -> Union[np.ndarray, bool]:
         """Read and decode the next frame as a np.ndarray. Returns False when video ends.
@@ -210,21 +219,27 @@ def read(self, decode: bool = True, advance: bool = True) -> Union[np.ndarray, b
             If decode = False, a bool indicating if advancing to the the next frame succeeded.
         """
         if not advance:
+            last_frame_valid = self._last_frame is not None and self._last_frame is not False
+            if not last_frame_valid:
+                return False
             if self._last_frame_rgb is None:
                 self._last_frame_rgb = cv2.cvtColor(self._last_frame, cv2.COLOR_BGR2RGB)
             return self._last_frame_rgb
-        if not hasattr(self._reader, "lastread"):
+        if not hasattr(self._reader, "lastread") or self._eof:
             return False
-        self._last_frame = self._reader.lastread
-        self._reader.read_frame()
-        if self._last_frame is self._reader.lastread:
-            # Didn't decode a new frame, must have hit EOF.
+        has_last_read = hasattr(self._reader, "last_read")
+        # In MoviePy 2.0 there is a separate property we need to read named differently (#461).
+        self._last_frame = self._reader.last_read if has_last_read else self._reader.lastread
+        # Read the *next* frame for the following call to read, and to check for EOF.
+        frame = self._reader.read_frame()
+        if frame is self._last_frame:
             if self._eof:
                 return False
             self._eof = True
         self._frame_number += 1
         if decode:
-            if self._last_frame is not None:
+            last_frame_valid = self._last_frame is not None and self._last_frame is not False
+            if last_frame_valid:
                 self._last_frame_rgb = cv2.cvtColor(self._last_frame, cv2.COLOR_BGR2RGB)
-            return self._last_frame_rgb
-        return True
+                return self._last_frame_rgb
+        return not self._eof

tests/test_video_stream.py

@@ -42,6 +42,12 @@
 MOVIEPY_WARNING_FILTER = "ignore:.*Using the last valid frame instead.:UserWarning"
 
 
+def get_moviepy_major_version() -> int:
+    import moviepy
+
+    return int(moviepy.__version__.split(".")[0])
+
+
 def calculate_frame_delta(frame_a, frame_b, roi=None) -> float:
     if roi:
         raise RuntimeError("TODO")
@@ -354,10 +360,16 @@ def test_corrupt_video(vs_type: Type[VideoStream], corrupt_video_file: str):
     """Test that backend handles video with corrupt frame gracefully with defaults."""
     if vs_type == VideoManager:
         pytest.skip(reason="VideoManager does not support handling corrupt videos.")
+    if vs_type == VideoStreamMoviePy and get_moviepy_major_version() >= 2:
+        # Due to changes in MoviePy 2.0 (#461), loading this file causes an exception to be thrown.
+        # See https://github.com/Zulko/moviepy/pull/2253 for a PR that attempts to more gracefully
+        # handle this case, however even once that is fixed, we will be unable to run this test
+        # on certain versions of MoviePy.
+        pytest.skip(reason="https://github.com/Zulko/moviepy/pull/2253")
 
     stream = vs_type(corrupt_video_file)
 
-    # OpenCV usually fails to read the video at frame 45, so we make sure all backends can
-    # get to 60 without reporting a failure.
+    # OpenCV usually fails to read the video at frame 45, but the remaining frames all seem to
+    # decode just fine. Make sure all backends can get to 60 without reporting a failure.
     for frame in range(60):
         assert stream.read() is not False, "Failed on frame %d!" % frame