[bugfix] Retry transient OSError exceptions with the MoviePy backend

scenedetect/backends/moviepy.py

@@ -16,6 +16,7 @@
 image sequences or AviSynth scripts are supported as inputs.
 """
 
+import time
 import typing as ty
 from fractions import Fraction
 from logging import getLogger
@@ -31,6 +32,33 @@
 
 logger = getLogger("pyscenedetect")
 
+# MoviePy spawns ffmpeg as a subprocess and reads frame bytes over stdout. Under
+# load the parent can read before the child has flushed its first write, which
+# surfaces as OSError (see #496). A short retry clears nearly all such flakes.
+_FFMPEG_RETRY_COUNT = 2
+_FFMPEG_RETRY_BACKOFF_SECS = 0.5
+
+
+def _retry_on_oserror(op_name: str, fn: ty.Callable):
+    """Run ``fn``, retrying up to ``_FFMPEG_RETRY_COUNT`` times on ``OSError``."""
+    last_exc: ty.Optional[OSError] = None
+    for attempt in range(_FFMPEG_RETRY_COUNT + 1):
+        try:
+            return fn()
+        except OSError as ex:
+            last_exc = ex
+            if attempt < _FFMPEG_RETRY_COUNT:
+                logger.warning(
+                    "ffmpeg %s failed (attempt %d/%d), retrying: %s",
+                    op_name,
+                    attempt + 1,
+                    _FFMPEG_RETRY_COUNT + 1,
+                    ex,
+                )
+                time.sleep(_FFMPEG_RETRY_BACKOFF_SECS)
+    assert last_exc is not None
+    raise last_exc
+
 
 class VideoStreamMoviePy(VideoStream):
     """MoviePy `FFMPEG_VideoReader` backend."""
@@ -63,7 +91,9 @@ def __init__(
         # cases return IOErrors (e.g. could not read duration/video resolution). These
         # should be mapped to specific errors, e.g. write a function to map MoviePy
         # exceptions to a new set of equivalents.
-        self._reader = FFMPEG_VideoReader(path, print_infos=print_infos)
+        self._reader = _retry_on_oserror(
+            "open", lambda: FFMPEG_VideoReader(path, print_infos=print_infos)
+        )
         # This will always be one behind self._reader.lastread when we finally call read()
         # as MoviePy caches the first frame when opening the video. Thus self._last_frame
         # will always be the current frame, and self._reader.lastread will be the next.
@@ -184,7 +214,9 @@ def seek(self, target: ty.Union[FrameTimecode, float, int]):
         if not isinstance(target, FrameTimecode):
             target = FrameTimecode(target, self.frame_rate)
         try:
-            self._last_frame = self._reader.get_frame(target.seconds)
+            self._last_frame = _retry_on_oserror(
+                "seek", lambda: self._reader.get_frame(target.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(
@@ -212,7 +244,9 @@ def reset(self, print_infos=False):
         self._last_frame_rgb = None
         self._frame_number = 0
         self._eof = False
-        self._reader = FFMPEG_VideoReader(self._path, print_infos=print_infos)
+        self._reader = _retry_on_oserror(
+            "reset", lambda: FFMPEG_VideoReader(self._path, print_infos=print_infos)
+        )
 
     def read(self, decode: bool = True) -> ty.Union[np.ndarray, bool]:
         if not hasattr(self._reader, "lastread") or self._eof:

tests/test_video_stream.py

@@ -121,20 +121,17 @@ def get_test_video_params() -> ty.List[VideoParameters]:
     ]
 
 
+_VS_TYPES: list = [vs for vs in (VideoStreamCv2, VideoStreamAv) if vs is not None]
+if VideoStreamMoviePy is not None:
+    _VS_TYPES.append(
+        pytest.param(
+            VideoStreamMoviePy,
+            marks=pytest.mark.flaky(reruns=3, reruns_delay=2, only_rerun=["OSError"]),
+        )
+    )
+
 pytestmark = [
-    pytest.mark.parametrize(
-        "vs_type",
-        list(
-            filter(
-                lambda x: x is not None,
-                [
-                    VideoStreamCv2,
-                    VideoStreamAv,
-                    VideoStreamMoviePy,
-                ],
-            )
-        ),
-    ),
+    pytest.mark.parametrize("vs_type", _VS_TYPES),
     pytest.mark.filterwarnings(MOVIEPY_WARNING_FILTER),
 ]
 

website/pages/changelog.md

@@ -679,6 +679,7 @@ Although there have been minimal changes to most API examples, there are several
 - [feature] New `save-xml` command supports saving scenes in Final Cut Pro formats [#156](https://github.com/Breakthrough/PySceneDetect/issues/156)
 - [feature] `--min-scene-len`/`-m` and `save-images --frame-margin`/`-m` now accept seconds (e.g. `0.6s`) and timecodes (e.g. `00:00:00.600`) in addition to a frame count [#531](https://github.com/Breakthrough/PySceneDetect/issues/531)
 - [bugfix] Fix floating-point precision error in `save-otio` output where frame values near integer boundaries (e.g. `90.00000000000001`) were serialized with spurious precision
+- [bugfix] Add mitigation for transient `OSError` in the MoviePy backend as it is susceptible to subprocess pipe races on slow or heavily loaded systems [#496](https://github.com/Breakthrough/PySceneDetect/issues/496)
 - [refactor] Remove deprecated `-d`/`--min-delta-hsv` option from `detect-adaptive` command
 
 ### API Changes