Keep cuts exactly min_scene_len frames apart at 29.97 and 59.94 fps

scenedetect/detector.py

@@ -124,9 +124,9 @@ def __init__(self, mode: Mode, length: TimecodeLike):
         """
         self._mode = mode
         # Frame count (int) and seconds (float) representations of `length`. Exactly one is
-        # populated up front; the other is computed on the first frame once the framerate is
-        # known. Temporal inputs (float/non-digit str / Timecode / FrameTimecode) populate
-        # `_filter_secs`; integer inputs (int/digit str) populate `_filter_length`.
+        # populated. Temporal inputs (float/non-digit str / Timecode / FrameTimecode) populate
+        # `_filter_secs`; integer inputs (int/digit str) populate `_filter_length`, which is
+        # compared as a frame count so no conversion to seconds is needed (#577).
         self._filter_length: int = 0
         self._filter_secs: float | None = None
         if isinstance(length, float):
@@ -168,17 +168,26 @@ def filter(self, timecode: FrameTimecode, above_threshold: bool) -> list[FrameTi
             return self._filter_suppress(timecode=timecode, above_threshold=above_threshold)
         raise RuntimeError("Unhandled FlashFilter mode.")
 
+    def _min_length_met(self, from_tc: FrameTimecode, to_tc: FrameTimecode) -> bool:
+        """Whether `to_tc` is at least the minimum scene length past `from_tc`.
+
+        When the length was given in frames, compare frame counts directly so an
+        exact N-frame gap is not lost to float conversion at NTSC rates (#577).
+        `FrameTimecode.__ge__` compares an int against `frame_num` exactly, so
+        this avoids the one-ulp error of `length / float(frame_rate)`.
+        """
+        if self._filter_length > 0:
+            return (to_tc - from_tc) >= self._filter_length
+        assert self._filter_secs is not None
+        return (to_tc - from_tc) >= self._filter_secs
+
     def _filter_suppress(
         self, timecode: FrameTimecode, above_threshold: bool
     ) -> list[FrameTimecode]:
         frame_rate = timecode.frame_rate
         assert frame_rate is not None and frame_rate >= 0
         assert self._last_above is not None
-        # Compute the threshold in seconds once from the first frame's framerate. This avoids
-        # using an incorrect average fps (e.g. OpenCV on VFR video) on subsequent frames.
-        if self._filter_secs is None:
-            self._filter_secs = self._filter_length / float(frame_rate)
-        min_length_met: bool = (timecode - self._last_above) >= self._filter_secs
+        min_length_met: bool = self._min_length_met(self._last_above, timecode)
         if not (above_threshold and min_length_met):
             return []
         # Both length and threshold requirements were satisfied. Emit the cut, and wait until both
@@ -190,10 +199,7 @@ def _filter_merge(self, timecode: FrameTimecode, above_threshold: bool) -> list[
         frame_rate = timecode.frame_rate
         assert frame_rate is not None and frame_rate >= 0
         assert self._last_above is not None
-        # Compute the threshold in seconds once from the first frame's framerate.
-        if self._filter_secs is None:
-            self._filter_secs = self._filter_length / float(frame_rate)
-        min_length_met: bool = (timecode - self._last_above) >= self._filter_secs
+        min_length_met: bool = self._min_length_met(self._last_above, timecode)
         # Ensure last frame is always advanced to the most recent one that was above the threshold.
         if above_threshold:
             self._last_above = timecode
@@ -203,7 +209,7 @@ def _filter_merge(self, timecode: FrameTimecode, above_threshold: bool) -> list[
             if (
                 min_length_met
                 and not above_threshold
-                and (self._last_above - self._merge_start) >= self._filter_secs
+                and self._min_length_met(self._merge_start, self._last_above)
             ):
                 self._merge_triggered = False
                 return [self._last_above]

tests/test_timecode.py

@@ -28,6 +28,7 @@
 
 # Standard Library Imports
 from scenedetect.common import MAX_FPS_DELTA, FrameTimecode, Timecode, framerate_to_fraction
+from scenedetect.detector import FlashFilter
 
 
 def test_framerate():
@@ -611,6 +612,68 @@ def test_min_scene_len_accepts_timecode_like():
     ContentDetector(min_scene_len=Timecode(pts=500, time_base=Fraction(1, 1000)))
 
 
+# FlashFilter NTSC regression tests (#577). An integer `length` (frames) was converted to seconds
+# with `length / float(frame_rate)`, which can land one ulp above the true value at NTSC rates, so
+# a gap of exactly N frames failed the `>=` check and the cut was dropped. These tests feed
+# FlashFilter the exact positions a backend reports for an NTSC stream (no video file needed).
+NTSC_RATES = (
+    Fraction(30000, 1001),
+    Fraction(60000, 1001),
+)
+
+
+def _ntsc_pts_position(frame: int, fps: Fraction) -> FrameTimecode:
+    """Position of `frame` as a PTS-exact backend (e.g. PyAV) reports it.
+
+    For an NTSC rate ``num/1001`` with ``time_base = 1/num`` each frame advances exactly ``1001``
+    PTS units.
+    """
+    return FrameTimecode(Timecode(pts=frame * 1001, time_base=Fraction(1, fps.numerator)), fps)
+
+
+def _ntsc_microsecond_position(frame: int, fps: Fraction) -> FrameTimecode:
+    """Position of `frame` as the OpenCV backend reports it, rounded to whole microseconds."""
+    us = round(frame * 1_000_000 / fps)
+    return FrameTimecode(Timecode(pts=us, time_base=Fraction(1, 1_000_000)), fps)
+
+
+@pytest.mark.parametrize("fps", NTSC_RATES)
+@pytest.mark.parametrize("mode", [FlashFilter.Mode.MERGE, FlashFilter.Mode.SUPPRESS])
+def test_flashfilter_emits_cut_every_n_frames_at_ntsc(fps: Fraction, mode: FlashFilter.Mode):
+    """A cut exactly `length` frames after the previous one is kept at NTSC rates."""
+    flash_filter = FlashFilter(mode=mode, length=15)
+    emitted = []
+    for frame in range(300):
+        emitted += flash_filter.filter(
+            _ntsc_pts_position(frame, fps), above_threshold=frame > 0 and frame % 15 == 0
+        )
+    assert [t.frame_num for t in emitted] == list(range(15, 300, 15))
+
+
+@pytest.mark.parametrize("fps", NTSC_RATES)
+def test_flashfilter_keeps_exact_gap_at_ntsc(fps: Fraction):
+    """The exact N-frame gap must satisfy the minimum-length check at NTSC rates."""
+    gap = _ntsc_pts_position(115, fps) - _ntsc_pts_position(100, fps)
+    assert gap.frame_num == 15
+    assert gap >= 15
+
+
+@pytest.mark.parametrize("fps", NTSC_RATES)
+@pytest.mark.parametrize("mode", [FlashFilter.Mode.MERGE, FlashFilter.Mode.SUPPRESS])
+def test_flashfilter_keeps_cuts_on_microsecond_positions(fps: Fraction, mode: FlashFilter.Mode):
+    """OpenCV rounds positions to microseconds, so an N-frame gap can be 1 us short of N / fps.
+
+    With min_scene_len=17 on 29.97 fps (the example in #577) cuts were dropped.
+    """
+    flash_filter = FlashFilter(mode=mode, length=17)
+    emitted = []
+    for frame in range(17 * 20):
+        emitted += flash_filter.filter(
+            _ntsc_microsecond_position(frame, fps), above_threshold=frame > 0 and frame % 17 == 0
+        )
+    assert [t.frame_num for t in emitted] == list(range(17, 17 * 20, 17))
+
+
 def test_get_framerate():
     """`get_framerate()` emits one warning and preserves its legacy float return value."""
     tc = FrameTimecode(timecode=0, fps=30.0)

website/pages/changelog.md

@@ -799,3 +799,4 @@ Development
  - [bugfix] Fix `TypeError` when opening videos with the `pyav` backend on PyAV 19.0 or newer
  - [bugfix] Fix `pyav` backend duration calculation fallback
  - [bugfix] `ThresholdDetector`/`detect-threshold` with method `CEILING` now classifies the first frame with the `CEILING` rule instead of the `FLOOR` rule, anchoring a leading fade one frame late and resulting in the possibility of a spurious first-frame cut (thanks @MuhammadBilalKhan267)
+ - [bugfix] Fix `min_scene_len` given in frames dropping cuts that are exactly `min_scene_len` frames apart at NTSC framerates (e.g. 29.97 fps), by comparing frame counts instead of converting to seconds [#577](https://github.com/Breakthrough/PySceneDetect/issues/577)