diff --git a/scenedetect/backends/pyav.py b/scenedetect/backends/pyav.py index 4fd60ea6..ecfe7bc2 100644 --- a/scenedetect/backends/pyav.py +++ b/scenedetect/backends/pyav.py @@ -39,6 +39,12 @@ Isolated corrupt frames are skipped; this bound ensures a truncated file still terminates.""" +def _to_fraction(value) -> Fraction: + """PyAV >= 19.0 returns `av.AVRational` for time bases and frame rates. The backend must + convert to `Fraction` so downstream arithmetic works correctly.""" + return Fraction(value.numerator, value.denominator) + + class VideoStreamAv(VideoStream): """PyAV `av.InputContainer` backend.""" @@ -150,9 +156,10 @@ def __init__( if hasattr(self._video_stream, "guessed_rate") else self._codec_context.framerate ) - if detected_rate is None or detected_rate == 0: - raise FrameRateUnavailable() - if detected_rate < MAX_FPS_DELTA: + # PyAV behavior has changed in version 19: + # PyAV < 19 - unknown rate is returned as None + # PyAV >= 19 - returns AVRational(0, 1) which is falsy + if not detected_rate or detected_rate < MAX_FPS_DELTA: raise FrameRateUnavailable() self._frame_rate: Fraction = framerate_to_fraction(detected_rate) else: @@ -220,18 +227,18 @@ def position(self) -> FrameTimecode: This can be interpreted as presentation time stamp, thus frame 1 corresponds to the presentation time 0. Returns 0 even if `frame_number` is 1.""" - if self._frame is None or self._frame.pts is None or self._frame.time_base is None: + if self._frame is None or self._frame.pts is None or not self._frame.time_base: return self.base_timecode - timecode = Timecode(pts=self._normalized_pts(), time_base=self._frame.time_base) + timecode = Timecode(pts=self._normalized_pts(), time_base=self._frame_time_base) return FrameTimecode(timecode=timecode, fps=self.frame_rate) @property def position_ms(self) -> float: """Current position within stream as a float of the presentation time in milliseconds. The first frame has a PTS of 0.""" - if self._frame is None or self._frame.pts is None or self._frame.time_base is None: + if self._frame is None or self._frame.pts is None or not self._frame.time_base: return 0.0 - return float(self._normalized_pts() * self._frame.time_base) * 1000.0 + return float(self._normalized_pts() * self._frame_time_base) * 1000.0 @property def frame_number(self) -> int: @@ -239,33 +246,31 @@ def frame_number(self) -> int: Will return 0 until the first frame is `read`. For VFR video this is an approximation derived from PTS * framerate; use `position` for accurate PTS-based timing.""" - if self._frame is None or self._frame.pts is None or self._frame.time_base is None: + if self._frame is None or self._frame.pts is None or not self._frame.time_base: return 0 - seconds = float(self._normalized_pts() * self._frame.time_base) + seconds = float(self._normalized_pts() * self._frame_time_base) return round(seconds * float(self.frame_rate)) + 1 @property - def rate(self) -> Fraction: - return self._video_stream.guessed_rate + def rate(self) -> Fraction | None: + """Frame rate reported by the stream (PyAV `guessed_rate`), or None if unavailable.""" + guessed_rate = self._video_stream.guessed_rate + return _to_fraction(guessed_rate) if guessed_rate else None @property def time_base(self) -> Fraction | None: - if self._frame: - return self._frame.time_base + if self._frame is not None and self._frame.time_base: + return self._frame_time_base return None @property def aspect_ratio(self) -> float: """Pixel aspect ratio as a float (1.0 represents square pixels).""" - if ( - not hasattr(self._codec_context, "display_aspect_ratio") - or self._codec_context.display_aspect_ratio is None - ): - return 1.0 - ar_denom = self._codec_context.display_aspect_ratio.denominator - if ar_denom <= 0: + # Unset is None on PyAV < 19 and a falsy AVRational(0, 1) on PyAV 19+. + dar = getattr(self._codec_context, "display_aspect_ratio", None) + if not dar or dar.denominator <= 0: return 1.0 - display_aspect_ratio = self._codec_context.display_aspect_ratio.numerator / ar_denom + display_aspect_ratio = dar.numerator / dar.denominator assert self.frame_size[0] > 0 and self.frame_size[1] > 0 frame_aspect_ratio = self.frame_size[0] / self.frame_size[1] return display_aspect_ratio / frame_aspect_ratio @@ -297,8 +302,8 @@ def seek(self, target: TimecodeLike) -> None: target = self.base_timecode + target if target >= 1: target = target - 1 - target_pts = self._video_stream.start_time + int( - (self.base_timecode + target).seconds / self._video_stream.time_base + target_pts = (self._video_stream.start_time or 0) + int( + (self.base_timecode + target).seconds / _to_fraction(self._video_stream.time_base) ) self._frame = None self._decoder = None @@ -376,6 +381,12 @@ def _codec_context(self): """PyAV `av.codec.context.CodecContext` being used.""" return self._video_stream.codec_context + @property + def _frame_time_base(self) -> Fraction: + """Time base of the current frame as a `Fraction` (see `_to_fraction`).""" + assert self._frame is not None and self._frame.time_base + return _to_fraction(self._frame.time_base) + def _normalized_pts(self) -> int: """PTS of the current frame relative to the start of the stream. Some files have a nonzero stream start_time (e.g. from edit lists); other backends report the first @@ -383,7 +394,9 @@ def _normalized_pts(self) -> int: assert self._frame is not None and self._frame.pts is not None start_time = self._video_stream.start_time or 0 if start_time and self._video_stream.time_base != self._frame.time_base: - start_time = int(start_time * self._video_stream.time_base / self._frame.time_base) + start_time = int( + start_time * _to_fraction(self._video_stream.time_base) / self._frame_time_base + ) return self._frame.pts - start_time def _get_duration(self) -> int: @@ -405,15 +418,14 @@ def _get_duration(self) -> int: if self._video_stream.duration is None: logger.warning("Video duration unavailable.") return 0 - # Streams use stream `time_base` as the time base. + # Streams use stream `time_base` as the time base. Unset is None on PyAV < 19 and + # a falsy AVRational(0, 1) on PyAV 19+. time_base = self._video_stream.time_base - if time_base.denominator == 0: - logger.warning( - "Unable to calculate video duration: time_base (%s) has zero denominator!", - str(time_base), - ) + if not time_base: + logger.warning("Unable to calculate video duration: stream time_base is unset.") return 0 - duration_sec = float(self._video_stream.duration / time_base) + # `duration` is in units of `time_base`, so seconds = duration * time_base. + duration_sec = float(self._video_stream.duration * _to_fraction(time_base)) return round(duration_sec * self.frame_rate) def _handle_eof(self): diff --git a/scenedetect/common.py b/scenedetect/common.py index 81086f60..f3846693 100644 --- a/scenedetect/common.py +++ b/scenedetect/common.py @@ -62,6 +62,7 @@ """ import math +import numbers import typing as ty import warnings from dataclasses import dataclass @@ -123,7 +124,7 @@ ## -def framerate_to_fraction(fps: "FrameRate") -> Fraction: +def framerate_to_fraction(fps: "FrameRate | numbers.Rational") -> Fraction: """Convert a framerate value to an exact rational Fraction. Detects NTSC-derived framerates of the form ``N * 1000/1001`` (e.g. 23.976 -> 24000/1001, @@ -131,7 +132,11 @@ def framerate_to_fraction(fps: "FrameRate") -> Fraction: their exact rational representation. Whole-number framerates are returned as ``Fraction(N, 1)``. Other values fall back to ``limit_denominator(10000)`` for a clean rational approximation. ``Fraction`` inputs are returned directly without conversion. + :class:`numbers.Rational` (e.g. ``av.AVRational`` from PyAV) is converted using numerator and + denominator. """ + if isinstance(fps, numbers.Rational) and not isinstance(fps, Fraction): + fps = Fraction(int(fps.numerator), int(fps.denominator)) if fps <= MAX_FPS_DELTA: raise ValueError("Framerate must be positive and greater than zero.") if isinstance(fps, Fraction): @@ -169,6 +174,15 @@ class Timecode: time_base: Fraction """The base unit in which `pts` is measured.""" + def __post_init__(self): + # Accept any rational (e.g. `av.AVRational` from PyAV 19+) but always store a `Fraction` + # so downstream arithmetic (`round`, `int`, mixed-type operations) behaves consistently. + time_base = self.time_base + if isinstance(time_base, numbers.Rational) and not isinstance(time_base, Fraction): + object.__setattr__( + self, "time_base", Fraction(int(time_base.numerator), int(time_base.denominator)) + ) + @property def seconds(self) -> float: return float(self.time_base * self.pts) @@ -464,17 +478,20 @@ def get_timecode( return f"{hrs:02d}:{mins:02d}:{secs_str}" @staticmethod - def _ensure_fractional(fps: "FrameRate | FrameTimecode") -> Fraction: + def _ensure_fractional(fps: "FrameRate | numbers.Rational | FrameTimecode") -> Fraction: """Validate and convert an `fps` argument into a positive `Fraction`. NTSC-like frame rates are handled via :func:`framerate_to_fraction`.""" if isinstance(fps, FrameTimecode): if fps._rate is None: raise TypeError("FrameTimecode passed as fps must have a known rate.") return fps._rate - if isinstance(fps, (float, Fraction)): + # Accept floats and non-integral rationals (`Fraction`, PyAV 19's `av.AVRational`), but + # keep rejecting `int`/`bool` so an accidental frame count is not treated as a frame rate. + if isinstance(fps, (float, numbers.Rational)) and not isinstance(fps, numbers.Integral): return framerate_to_fraction(fps) raise TypeError( - f"Wrong type for fps: {type(fps)} - expected float, Fraction, or FrameTimecode" + f"Wrong type for fps: {type(fps)} - expected float, Fraction (or another non-integral" + " rational), or FrameTimecode" ) def _seconds_to_frames(self, seconds: float) -> int: diff --git a/tests/test_backend_pyav.py b/tests/test_backend_pyav.py index 17bf0170..14ba5a5c 100644 --- a/tests/test_backend_pyav.py +++ b/tests/test_backend_pyav.py @@ -17,9 +17,13 @@ For VideoStream tests that validate conformance, see test_video_stream.py. """ +from fractions import Fraction +from types import SimpleNamespace + import av +import pytest -from scenedetect.backends.pyav import MAX_CONSECUTIVE_DECODE_FAILURES, VideoStreamAv +from scenedetect.backends.pyav import MAX_CONSECUTIVE_DECODE_FAILURES, VideoStreamAv, _to_fraction def test_video_stream_pyav_bytesio(test_video_file: str, auto_close): @@ -91,3 +95,110 @@ def always_failing_decode(container, *args, **kwargs): # `no_logs_gte_error` fixture doesn't fail the test. assert any("consecutive" in record.message for record in caplog.records) caplog.clear() + + +def test_rationals_are_fractions(test_video_file: str, auto_close): + """PyAV >= 19.0 returns `av.AVRational` for time bases and frame rates. The backend must + convert to `Fraction` so downstream arithmetic (`round`, `int`) works properly.""" + stream = auto_close(VideoStreamAv(test_video_file)) + assert stream.read(decode=False) is not False + assert isinstance(stream.frame_rate, Fraction) + assert stream.frame_rate == Fraction(30000, 1001) + assert isinstance(stream.rate, Fraction) + assert isinstance(stream.time_base, Fraction) + assert isinstance(stream.position.time_base, Fraction) + assert stream.position_ms == 0.0 + assert stream.frame_number == 1 + + +class _StubRational: + """Stands in for `av.AVRational` (PyAV 19+): only `numerator`/`denominator`, no `__int__`.""" + + def __init__(self, numerator: int, denominator: int): + self.numerator = numerator + self.denominator = denominator + + +def test_to_fraction_from_stub_rational(): + result = _to_fraction(_StubRational(30000, 1001)) + assert isinstance(result, Fraction) + assert result == Fraction(30000, 1001) + assert _to_fraction(Fraction(1, 24000)) == Fraction(1, 24000) + + +class _AttrOverridingProxy: + """Forwards attribute access to the wrapped object except for the given overrides.""" + + def __init__(self, wrapped, **overrides): + self._wrapped = wrapped + self._overrides = overrides + + def __getattr__(self, name): + if name in self._overrides: + return self._overrides[name] + return getattr(self._wrapped, name) + + +def test_duration_fallback_from_stream_duration( + test_video_file: str, monkeypatch: pytest.MonkeyPatch, auto_close +): + """When neither the stream frame count nor the container duration is available, the duration + must be derived from `stream.duration * stream.time_base` (duration is in time_base units).""" + stream = auto_close(VideoStreamAv(test_video_file)) + real_stream = stream._video_stream + assert real_stream.frames == 720 + fake_stream = _AttrOverridingProxy( + real_stream, + frames=0, + container=SimpleNamespace(duration=None), + ) + monkeypatch.setattr(VideoStreamAv, "_video_stream", property(lambda self: fake_stream)) + assert stream._get_duration() == 720 + + +@pytest.mark.parametrize("unset_dar", [None, Fraction(0, 1)], ids=["none", "zero"]) +def test_aspect_ratio_unset_dar( + test_video_file: str, unset_dar, monkeypatch: pytest.MonkeyPatch, auto_close +): + """An unset display aspect ratio (None on PyAV < 19, AVRational(0, 1) on PyAV 19+) must be + treated as square pixels rather than producing an aspect ratio of 0.""" + stream = auto_close(VideoStreamAv(test_video_file)) + width, height = stream.frame_size + fake_ctx = SimpleNamespace(display_aspect_ratio=unset_dar, width=width, height=height) + monkeypatch.setattr(VideoStreamAv, "_codec_context", property(lambda self: fake_ctx)) + assert stream.aspect_ratio == 1.0 + + +@pytest.mark.parametrize( + "guessed_rate, expected", + [(_StubRational(30000, 1001), Fraction(30000, 1001)), (None, None)], + ids=["stub-rational", "none"], +) +def test_rate_normalizes_stream_rational( + test_video_file: str, guessed_rate, expected, monkeypatch: pytest.MonkeyPatch, auto_close +): + """`rate` must return a `Fraction` for any rational type the stream reports (PyAV 19+ returns + `av.AVRational`), and None when the stream has no guessed rate (PyAV < 19).""" + stream = auto_close(VideoStreamAv(test_video_file)) + fake_stream = _AttrOverridingProxy(stream._video_stream, guessed_rate=guessed_rate) + monkeypatch.setattr(VideoStreamAv, "_video_stream", property(lambda self: fake_stream)) + assert stream.rate == expected + assert expected is None or isinstance(stream.rate, Fraction) + + +@pytest.mark.parametrize("time_base", [None, Fraction(0, 1)], ids=["none", "zero"]) +def test_duration_fallback_unset_time_base( + test_video_file: str, time_base, monkeypatch: pytest.MonkeyPatch, caplog, auto_close +): + """With no frame count, no container duration, and an unset stream time base (None on + PyAV < 19, AVRational(0, 1) on PyAV 19+), the duration must be reported as 0 with a warning.""" + stream = auto_close(VideoStreamAv(test_video_file)) + fake_stream = _AttrOverridingProxy( + stream._video_stream, + frames=0, + container=SimpleNamespace(duration=None), + time_base=time_base, + ) + monkeypatch.setattr(VideoStreamAv, "_video_stream", property(lambda self: fake_stream)) + assert stream._get_duration() == 0 + assert any("time_base" in record.message for record in caplog.records) diff --git a/tests/test_timecode.py b/tests/test_timecode.py index 66017bd9..15ea5c10 100644 --- a/tests/test_timecode.py +++ b/tests/test_timecode.py @@ -21,6 +21,7 @@ """ # Third-Party Library Imports +import numbers from fractions import Fraction import pytest @@ -429,6 +430,51 @@ def test_framerate_to_fraction_non_ntsc_fallback(): assert framerate_to_fraction(24.5) == Fraction(49, 2) +class _StubRational: + """Minimal `numbers.Rational` that only exposes numerator/denominator, mirroring + `av.AVRational` from PyAV 19+ (which has no `__int__`, `__round__`, or `__floor__`).""" + + def __init__(self, numerator: int, denominator: int): + self.numerator = numerator + self.denominator = denominator + + +numbers.Rational.register(_StubRational) + + +def test_framerate_to_fraction_accepts_rational(): + """Any `numbers.Rational` (not just `Fraction`) must be converted exactly. PyAV >= 19.0 + reports frame rates and time bases as the `av.AVRational` type.""" + for num, den in [(30000, 1001), (24000, 1001), (25, 1)]: + result = framerate_to_fraction(_StubRational(num, den)) # type: ignore[arg-type] + assert isinstance(result, Fraction) + assert result == Fraction(num, den) + with pytest.raises(ValueError): + framerate_to_fraction(_StubRational(0, 1)) # type: ignore[arg-type] + + +def test_frame_timecode_accepts_rational_fps(): + tc = FrameTimecode(0, _StubRational(24000, 1001)) # type: ignore[arg-type] + assert isinstance(tc.frame_rate, Fraction) + assert tc.frame_rate == Fraction(24000, 1001) + + +def test_frame_timecode_rejects_integral_fps(): + """`int`/`bool` are also `numbers.Rational` - these must raise TypeError so a misplaced + frame *count* is never silently used as a frame *rate*.""" + with pytest.raises(TypeError): + FrameTimecode(0, 30) # type: ignore[arg-type] + with pytest.raises(TypeError): + FrameTimecode(0, True) # type: ignore[arg-type] + + +def test_timecode_normalizes_rational_time_base(): + tc = Timecode(pts=1001, time_base=_StubRational(1, 30000)) # type: ignore[arg-type] + assert isinstance(tc.time_base, Fraction) + assert tc.time_base == Fraction(1, 30000) + assert tc.seconds == pytest.approx(1001 / 30000) + + def test_timecode_arithmetic_mixed_time_base(): """Arithmetic with FrameTimecodes using different time_bases should work.""" fps = Fraction(24000, 1001) diff --git a/website/pages/changelog.md b/website/pages/changelog.md index fd4bcdca..f8d15edf 100644 --- a/website/pages/changelog.md +++ b/website/pages/changelog.md @@ -794,5 +794,7 @@ Development - [api] Scene-list output functions for EDL, FCPXML, FCP7 XML, and OTIO now accept an open text file handle, a string path, or a `pathlib.Path`; paths are opened and closed automatically [#567](https://github.com/Breakthrough/PySceneDetect/issues/567) - [feature] Add `--min-out-length`/`min_out_length` to `detect-threshold`/`ThresholdDetector`, which ignores fades that stay below the threshold for less than the given duration [#278](https://github.com/Breakthrough/PySceneDetect/issues/278) - [improvement] `detect-threshold` now ignores fade-outs shorter than `0.1s` by default. Use `--min-out-length 0` to restore the previous behavior. The API default remains `0` [#278](https://github.com/Breakthrough/PySceneDetect/issues/278) -- [feature] Added `save-keyframes` command to export detected cuts using `# keyframe format v1` for Aegisub-compatible tools [#534](https://github.com/Breakthrough/PySceneDetect/issues/534). Frame numbers are currently approximate for VFR input [#569](https://github.com/Breakthrough/PySceneDetect/issues/569) + - [feature] Added `save-keyframes` command to export detected cuts using `# keyframe format v1` for Aegisub-compatible tools [#534](https://github.com/Breakthrough/PySceneDetect/issues/534). Frame numbers are currently approximate for VFR input [#569](https://github.com/Breakthrough/PySceneDetect/issues/569) - [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