From b3770ce83c114994854f602448a369117926ec5d Mon Sep 17 00:00:00 2001 From: theluckystrike <51033404+theluckystrike@users.noreply.github.com> Date: Fri, 9 Oct 2026 21:50:24 +0700 Subject: [PATCH 1/2] [detector] Compare frame counts for min_scene_len given in frames (#577) --- scenedetect/detector.py | 18 +++++-- tests/test_flashfilter_ntsc.py | 93 ++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 tests/test_flashfilter_ntsc.py diff --git a/scenedetect/detector.py b/scenedetect/detector.py index e06e1440..10def2c0 100644 --- a/scenedetect/detector.py +++ b/scenedetect/detector.py @@ -168,6 +168,18 @@ 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 + return (to_tc - from_tc) >= self._filter_secs + def _filter_suppress( self, timecode: FrameTimecode, above_threshold: bool ) -> list[FrameTimecode]: @@ -178,7 +190,7 @@ def _filter_suppress( # 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 @@ -193,7 +205,7 @@ def _filter_merge(self, timecode: FrameTimecode, above_threshold: bool) -> list[ # 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 +215,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] diff --git a/tests/test_flashfilter_ntsc.py b/tests/test_flashfilter_ntsc.py new file mode 100644 index 00000000..2fb85403 --- /dev/null +++ b/tests/test_flashfilter_ntsc.py @@ -0,0 +1,93 @@ +# +# PySceneDetect: Python-Based Video Scene Detector +# ------------------------------------------------------------------- +# [ Site: https://scenedetect.com ] +# [ Docs: https://scenedetect.com/docs/ ] +# [ Github: https://github.com/Breakthrough/PySceneDetect/ ] +# +# Copyright (C) 2026 PySceneDetect Contributors. +# PySceneDetect is licensed under the BSD 3-Clause License; see the +# included LICENSE file, or visit one of the above pages for details. +# +"""FlashFilter NTSC regression tests (#577) + +ContentDetector (and any detector that routes through FlashFilter) must emit a +cut exactly ``min_scene_len`` frames after the previous one, even at NTSC rates +(30000/1001 and 60000/1001 fps) where ``float(frame_rate)`` is not exact. The +regression was introduced in 0.7 when FlashFilter moved to time units: an +integer frame length was converted to seconds with ``length / float(frame_rate)``, +which can land one ulp above the true value, so a gap of exactly N frames failed +the ``>=`` comparison and the cut was dropped. + +These tests feed FlashFilter the exact PTS positions a backend reports for an +NTSC stream (no video file needed) and assert that both MERGE and SUPPRESS modes +emit a cut every 15 frames. +""" + +from fractions import Fraction + +import pytest + +from scenedetect.common import FrameTimecode, Timecode +from scenedetect.detector import FlashFilter + +# NTSC rates whose frame duration is not exactly representable as a float. +NTSC_RATES = ( + Fraction(30000, 1001), + Fraction(60000, 1001), +) + + +def _at(frame: int, fps: Fraction) -> FrameTimecode: + """Position of `frame` as a PTS-exact backend (e.g. PyAV) reports it. + + For an NTSC rate ``num/1001`` the frame duration is ``1001/num`` seconds, so + 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) + + +@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( + _at(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 = _at(115, fps) - _at(100, fps) + assert gap.frame_num == 15 + # The float conversion the bug used is one ulp high, so the exact comparison + # must not go through it. + assert gap >= 15 + + +def _at_microseconds(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_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; comparing frame + counts keeps them. + """ + flash_filter = FlashFilter(mode=mode, length=17) + emitted = [] + for frame in range(17 * 20): + emitted += flash_filter.filter( + _at_microseconds(frame, fps), above_threshold=frame > 0 and frame % 17 == 0 + ) + assert [t.frame_num for t in emitted] == list(range(17, 17 * 20, 17)) From 4de35718ab46295036e8f90d0445cb01f1fb070e Mon Sep 17 00:00:00 2001 From: theluckystrike <51033404+theluckystrike@users.noreply.github.com> Date: Sat, 10 Oct 2026 09:15:45 +0700 Subject: [PATCH 2/2] [detector] Address review feedback for #577 Skip the unused seconds conversion when min_scene_len is given in frames, move the FlashFilter NTSC tests into test_timecode.py, and add a changelog entry. --- scenedetect/detector.py | 14 ++--- tests/test_flashfilter_ntsc.py | 93 ---------------------------------- tests/test_timecode.py | 63 +++++++++++++++++++++++ website/pages/changelog.md | 1 + 4 files changed, 68 insertions(+), 103 deletions(-) delete mode 100644 tests/test_flashfilter_ntsc.py diff --git a/scenedetect/detector.py b/scenedetect/detector.py index 10def2c0..f3c16d06 100644 --- a/scenedetect/detector.py +++ b/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): @@ -178,6 +178,7 @@ def _min_length_met(self, from_tc: FrameTimecode, to_tc: FrameTimecode) -> bool: """ 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( @@ -186,10 +187,6 @@ def _filter_suppress( 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 = self._min_length_met(self._last_above, timecode) if not (above_threshold and min_length_met): return [] @@ -202,9 +199,6 @@ 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 = 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: diff --git a/tests/test_flashfilter_ntsc.py b/tests/test_flashfilter_ntsc.py deleted file mode 100644 index 2fb85403..00000000 --- a/tests/test_flashfilter_ntsc.py +++ /dev/null @@ -1,93 +0,0 @@ -# -# PySceneDetect: Python-Based Video Scene Detector -# ------------------------------------------------------------------- -# [ Site: https://scenedetect.com ] -# [ Docs: https://scenedetect.com/docs/ ] -# [ Github: https://github.com/Breakthrough/PySceneDetect/ ] -# -# Copyright (C) 2026 PySceneDetect Contributors. -# PySceneDetect is licensed under the BSD 3-Clause License; see the -# included LICENSE file, or visit one of the above pages for details. -# -"""FlashFilter NTSC regression tests (#577) - -ContentDetector (and any detector that routes through FlashFilter) must emit a -cut exactly ``min_scene_len`` frames after the previous one, even at NTSC rates -(30000/1001 and 60000/1001 fps) where ``float(frame_rate)`` is not exact. The -regression was introduced in 0.7 when FlashFilter moved to time units: an -integer frame length was converted to seconds with ``length / float(frame_rate)``, -which can land one ulp above the true value, so a gap of exactly N frames failed -the ``>=`` comparison and the cut was dropped. - -These tests feed FlashFilter the exact PTS positions a backend reports for an -NTSC stream (no video file needed) and assert that both MERGE and SUPPRESS modes -emit a cut every 15 frames. -""" - -from fractions import Fraction - -import pytest - -from scenedetect.common import FrameTimecode, Timecode -from scenedetect.detector import FlashFilter - -# NTSC rates whose frame duration is not exactly representable as a float. -NTSC_RATES = ( - Fraction(30000, 1001), - Fraction(60000, 1001), -) - - -def _at(frame: int, fps: Fraction) -> FrameTimecode: - """Position of `frame` as a PTS-exact backend (e.g. PyAV) reports it. - - For an NTSC rate ``num/1001`` the frame duration is ``1001/num`` seconds, so - 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) - - -@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( - _at(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 = _at(115, fps) - _at(100, fps) - assert gap.frame_num == 15 - # The float conversion the bug used is one ulp high, so the exact comparison - # must not go through it. - assert gap >= 15 - - -def _at_microseconds(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_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; comparing frame - counts keeps them. - """ - flash_filter = FlashFilter(mode=mode, length=17) - emitted = [] - for frame in range(17 * 20): - emitted += flash_filter.filter( - _at_microseconds(frame, fps), above_threshold=frame > 0 and frame % 17 == 0 - ) - assert [t.frame_num for t in emitted] == list(range(17, 17 * 20, 17)) diff --git a/tests/test_timecode.py b/tests/test_timecode.py index 15ea5c10..863e9fe3 100644 --- a/tests/test_timecode.py +++ b/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) diff --git a/website/pages/changelog.md b/website/pages/changelog.md index f8d15edf..23893f60 100644 --- a/website/pages/changelog.md +++ b/website/pages/changelog.md @@ -798,3 +798,4 @@ Development - [bugfix] Fix intermittent segfault on exit. Isolated to Windows builds with OpenCV 5.x when opening PNG image sequences with the `VideoStreamCv2` backend. [#575](https://github.com/Breakthrough/PySceneDetect/issues/575) - [bugfix] Fix `TypeError` when opening videos with the `pyav` backend on PyAV 19.0 or newer - [bugfix] Fix `pyav` backend duration calculation fallback + - [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)