diff --git a/docs/cli/backends.rst b/docs/cli/backends.rst index 2e28102b..7c235915 100644 --- a/docs/cli/backends.rst +++ b/docs/cli/backends.rst @@ -19,7 +19,7 @@ The `OpenCV `_ backend (usually `opencv-python `. """ +import warnings from logging import getLogger # OpenCV is a required package, but we don't have it as an explicit dependency since we @@ -115,8 +116,13 @@ def open_video( :class:`VideoOpenFailure`: Constructing the VideoStream fails. If multiple backends have been attempted, the error from the first backend will be returned. """ - # TODO(https://scenedetect.com/issue/548): emit DeprecationWarning when `framerate=` is - # used, once internal callers and downstream users have had a release to migrate. + if framerate is not None: + warnings.warn( + "`framerate` is deprecated and scheduled for removal in v0.9; " + "use `frame_rate` instead.", + DeprecationWarning, + stacklevel=2, + ) if frame_rate is None: frame_rate = framerate # A list of paths is opened as a single concatenated stream. VideoStreamConcat handles diff --git a/scenedetect/_cli/__init__.py b/scenedetect/_cli/__init__.py index dd21768c..ac506dff 100644 --- a/scenedetect/_cli/__init__.py +++ b/scenedetect/_cli/__init__.py @@ -22,6 +22,7 @@ import logging import os import os.path +import warnings from copy import copy import click @@ -361,8 +362,13 @@ def scenedetect( ctx = ctx.obj assert isinstance(ctx, CliContext) - # TODO(https://scenedetect.com/issue/548): emit DeprecationWarning when `--framerate` - # is used, once downstream users have had a release to migrate to `--frame-rate`. + if framerate_legacy is not None: + warnings.warn( + "`--framerate` is deprecated; use `--frame-rate` instead.", + DeprecationWarning, + stacklevel=2, + ) + if frame_rate is None: frame_rate = framerate_legacy elif framerate_legacy is not None: diff --git a/scenedetect/backends/moviepy.py b/scenedetect/backends/moviepy.py index aef9844d..d3167b26 100644 --- a/scenedetect/backends/moviepy.py +++ b/scenedetect/backends/moviepy.py @@ -19,6 +19,7 @@ import os import time import typing as ty +import warnings from fractions import Fraction from logging import getLogger @@ -94,8 +95,13 @@ def __init__( """ super().__init__() - # TODO(https://scenedetect.com/issue/548): emit DeprecationWarning when `framerate=` is - # used, once internal callers and downstream users have had a release to migrate. + if framerate is not None: + warnings.warn( + "`framerate` is deprecated and scheduled for removal in v0.9; " + "use `frame_rate` instead.", + DeprecationWarning, + stacklevel=2, + ) if frame_rate is None: frame_rate = framerate # TODO: Investigate how MoviePy handles ffmpeg not being on PATH. diff --git a/scenedetect/backends/opencv.py b/scenedetect/backends/opencv.py index 12294664..83cd60a8 100644 --- a/scenedetect/backends/opencv.py +++ b/scenedetect/backends/opencv.py @@ -101,8 +101,13 @@ def __init__( ValueError: specified frame rate is invalid """ super().__init__() - # TODO(https://scenedetect.com/issue/548): emit DeprecationWarning when `framerate=` is - # used, once internal callers and downstream users have had a release to migrate. + if framerate is not None: + warnings.warn( + "`framerate` is deprecated and scheduled for removal in v0.9; " + "use `frame_rate` instead.", + DeprecationWarning, + stacklevel=2, + ) if frame_rate is None: frame_rate = framerate if path_or_device is not None: @@ -395,8 +400,13 @@ def __init__( """ super().__init__() - # TODO(https://scenedetect.com/issue/548): emit DeprecationWarning when `framerate=` is - # used, once internal callers and downstream users have had a release to migrate. + if framerate is not None: + warnings.warn( + "`framerate` is deprecated and scheduled for removal in v0.9; " + "use `frame_rate` instead.", + DeprecationWarning, + stacklevel=2, + ) if frame_rate is None: frame_rate = framerate if frame_rate is not None and frame_rate < MAX_FPS_DELTA: diff --git a/scenedetect/backends/pyav.py b/scenedetect/backends/pyav.py index 0933c547..4fd60ea6 100644 --- a/scenedetect/backends/pyav.py +++ b/scenedetect/backends/pyav.py @@ -13,6 +13,7 @@ import os import typing as ty +import warnings from fractions import Fraction from logging import getLogger @@ -89,8 +90,13 @@ def __init__( # refinement for frames FFmpeg flags as corrupt but still decodes. super().__init__() - # TODO(https://scenedetect.com/issue/548): emit DeprecationWarning when `framerate=` is - # used, once internal callers and downstream users have had a release to migrate. + if framerate is not None: + warnings.warn( + "`framerate` is deprecated and scheduled for removal in v0.9; " + "use `frame_rate` instead.", + DeprecationWarning, + stacklevel=2, + ) if frame_rate is None: frame_rate = framerate # Ensure specified frame rate is valid if set. diff --git a/scenedetect/common.py b/scenedetect/common.py index 653c5cf7..81086f60 100644 --- a/scenedetect/common.py +++ b/scenedetect/common.py @@ -296,8 +296,12 @@ def framerate(self) -> float | None: property returns an exact :class:`fractions.Fraction` and matches the naming used by :attr:`scenedetect.video_stream.VideoStream.frame_rate`. """ - # TODO(https://scenedetect.com/issue/548): emit DeprecationWarning here once internal - # callers and downstream users have had a release to migrate to `frame_rate`. + warnings.warn( + "`framerate` is deprecated and scheduled for removal in v0.9; " + "use `frame_rate` instead.", + DeprecationWarning, + stacklevel=2, + ) if self._rate is None: return None return float(self._rate) @@ -335,16 +339,18 @@ def get_frames(self) -> int: def get_framerate(self) -> float | None: """[DEPRECATED] Get Framerate: Returns the framerate used by the FrameTimecode object. - Use the `framerate` property instead. + Use the `frame_rate` property instead. :meta private: """ warnings.warn( - "get_framerate() is deprecated, use the `framerate` property instead.", + "get_framerate() is deprecated, use the `frame_rate` property instead.", DeprecationWarning, stacklevel=2, ) - return self.framerate + if self.frame_rate is None: + return None + return float(self.frame_rate) def equal_frame_rate(self, other: "float | Fraction | FrameTimecode") -> bool: """Determine whether the passed frame rate equals this object's frame rate. @@ -368,8 +374,12 @@ def equal_frame_rate(self, other: "float | Fraction | FrameTimecode") -> bool: def equal_framerate(self, fps) -> bool: """[DEPRECATED] Use :meth:`equal_frame_rate` instead.""" - # TODO(https://scenedetect.com/issue/548): emit DeprecationWarning here once internal - # callers and downstream users have had a release to migrate to `equal_frame_rate`. + warnings.warn( + "`equal_framerate()` is deprecated and scheduled for removal in v0.9; " + "use `equal_frame_rate()` instead.", + DeprecationWarning, + stacklevel=2, + ) return self.equal_frame_rate(fps) @property diff --git a/scenedetect/video_stream.py b/scenedetect/video_stream.py index 48f25087..71af9950 100644 --- a/scenedetect/video_stream.py +++ b/scenedetect/video_stream.py @@ -66,7 +66,7 @@ class FrameRateUnavailable(VideoOpenFailure): def __init__(self): super().__init__( - "Unable to obtain video framerate! Specify `framerate` manually, or" + "Unable to obtain video framerate! Specify `frame_rate` manually, or" " re-encode/re-mux the video and try again." ) diff --git a/tests/test_api.py b/tests/test_api.py index 1ec880ec..5bde6cfd 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -13,6 +13,8 @@ These tests demonstrate common workflow patterns used when integrating the PySceneDetect API.""" +import pytest + def test_api_detect(test_video_file: str): """Demonstrate usage of the `detect()` function to process a complete video.""" @@ -73,15 +75,17 @@ def test_api_scene_manager_start_end_time(test_video_file: str): def test_api_open_video_framerate_legacy_alias(test_video_file: str): - """`open_video(framerate=...)` is the soft-deprecated alias for `frame_rate=` (issue #548). + """`open_video(framerate=...)` is the deprecated alias for `frame_rate=` (issue #548). Both forms must produce equivalent streams; when both are provided, `frame_rate` wins.""" from scenedetect import open_video - legacy = open_video(test_video_file, framerate=30.0) + with pytest.warns(DeprecationWarning, match="frame_rate"): + legacy = open_video(test_video_file, framerate=30.0) canonical = open_video(test_video_file, frame_rate=30.0) assert legacy.frame_rate == canonical.frame_rate # `frame_rate` takes precedence over `framerate` when both are provided. - both = open_video(test_video_file, frame_rate=30.0, framerate=24.0) + with pytest.warns(DeprecationWarning, match="frame_rate"): + both = open_video(test_video_file, frame_rate=30.0, framerate=24.0) assert both.frame_rate == canonical.frame_rate diff --git a/tests/test_backend_opencv.py b/tests/test_backend_opencv.py index 1f8c03be..9aefca78 100644 --- a/tests/test_backend_opencv.py +++ b/tests/test_backend_opencv.py @@ -18,6 +18,7 @@ """ import cv2 +import pytest from scenedetect import ContentDetector, SceneManager from scenedetect.backends.opencv import VideoCaptureAdapter, VideoStreamCv2 @@ -28,7 +29,7 @@ def test_open_image_sequence(test_image_sequence: str): """Test opening an image sequence. Currently, only VideoStreamCv2 supports this.""" - sequence = VideoStreamCv2(test_image_sequence, framerate=25.0) + sequence = VideoStreamCv2(test_image_sequence, frame_rate=25.0) assert sequence.is_seekable assert sequence.frame_size[0] > 0 and sequence.frame_size[1] > 0 assert sequence.duration is not None @@ -53,6 +54,25 @@ def test_capture_adapter(test_movie_clip: str): assert [start.frame_num for (start, _) in scenes] == GROUND_TRUTH_CAPTURE_ADAPTER_TEST +def test_capture_adapter_framerate_legacy_alias(test_movie_clip: str): + """`framerate=` is the deprecated alias for `frame_rate=` on VideoCaptureAdapter.""" + cap = cv2.VideoCapture(test_movie_clip) + assert cap.isOpened() + with pytest.warns(DeprecationWarning, match="frame_rate"): + legacy = VideoCaptureAdapter(cap, framerate=30.0) + + cap = cv2.VideoCapture(test_movie_clip) + assert cap.isOpened() + canonical = VideoCaptureAdapter(cap, frame_rate=30.0) + assert canonical.frame_rate == legacy.frame_rate + + cap = cv2.VideoCapture(test_movie_clip) + assert cap.isOpened() + with pytest.warns(DeprecationWarning, match="frame_rate"): + both = VideoCaptureAdapter(cap, frame_rate=30.0, framerate=24.0) + assert both.frame_rate == canonical.frame_rate + + def test_decode_failures_exposed(corrupt_video_file: str): """The private decode failure counters must be surfaced by the public property on both VideoStreamCv2 and VideoCaptureAdapter.""" diff --git a/tests/test_cli.py b/tests/test_cli.py index a807973e..5a220cb0 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -336,7 +336,7 @@ def test_cli_detector_with_stats(tmp_path, detector_command: str): def test_cli_framerate_legacy_alias(): - """`--framerate` is the soft-deprecated hidden alias for `-f/--frame-rate` (issue #548). + """`--framerate` is the deprecated hidden alias for `-f/--frame-rate` (issue #548). Both forms must be accepted; passing both should not error.""" # Canonical form. exit_code, _ = invoke_cli( @@ -344,26 +344,28 @@ def test_cli_framerate_legacy_alias(): ) assert exit_code == 0 # Legacy form. - exit_code, _ = invoke_cli( - ["-i", DEFAULT_VIDEO_PATH, "--framerate", "30.0", "time", "-s", "2s", "-d", "4s"] - ) + with pytest.warns(DeprecationWarning, match="--frame-rate"): + exit_code, _ = invoke_cli( + ["-i", DEFAULT_VIDEO_PATH, "--framerate", "30.0", "time", "-s", "2s", "-d", "4s"] + ) assert exit_code == 0 # Both forms together: `--frame-rate` wins, a warning is logged but no error. - exit_code, _ = invoke_cli( - [ - "-i", - DEFAULT_VIDEO_PATH, - "--frame-rate", - "30.0", - "--framerate", - "24.0", - "time", - "-s", - "2s", - "-d", - "4s", - ] - ) + with pytest.warns(DeprecationWarning, match="--frame-rate"): + exit_code, _ = invoke_cli( + [ + "-i", + DEFAULT_VIDEO_PATH, + "--frame-rate", + "30.0", + "--framerate", + "24.0", + "time", + "-s", + "2s", + "-d", + "4s", + ] + ) assert exit_code == 0 diff --git a/tests/test_timecode.py b/tests/test_timecode.py index 3ea8fbd3..66017bd9 100644 --- a/tests/test_timecode.py +++ b/tests/test_timecode.py @@ -73,12 +73,16 @@ def test_frame_rate_property(): tc = FrameTimecode(timecode=0, fps=30.0) assert tc.frame_rate == Fraction(30, 1) assert isinstance(tc.frame_rate, Fraction) - assert tc.framerate == 30.0 - assert isinstance(tc.framerate, float) + with pytest.warns(DeprecationWarning, match="frame_rate"): + legacy_frame_rate = tc.framerate + + assert legacy_frame_rate == 30.0 + assert isinstance(legacy_frame_rate, float) # Constructed directly from a Fraction (the exact form for NTSC rates). tc = FrameTimecode(timecode=0, fps=Fraction(30000, 1001)) assert tc.frame_rate == Fraction(30000, 1001) - assert tc.framerate == pytest.approx(float(Fraction(30000, 1001))) + with pytest.warns(DeprecationWarning, match="frame_rate"): + assert tc.framerate == pytest.approx(float(Fraction(30000, 1001))) tc = FrameTimecode(timecode=0, fps=Fraction(24000, 1001)) assert tc.frame_rate == Fraction(24000, 1001) # time_base equals 1 / frame_rate for CFR sources. @@ -108,17 +112,21 @@ def test_frame_num_and_frame_rate_are_read_only(): def test_equal_frame_rate_legacy_alias(): - """`equal_framerate()` is the soft-deprecated alias for `equal_frame_rate()` (issue #548). + """`equal_framerate()` is the deprecated alias for `equal_frame_rate()` (issue #548). Both forms should produce identical results for every accepted operand type.""" tc = FrameTimecode(timecode=0, fps=30.0) # float, Fraction, FrameTimecode operands. other_tc = FrameTimecode(timecode=0, fps=30.0) for other in (30.0, Fraction(30, 1), other_tc): - assert tc.equal_frame_rate(other) == tc.equal_framerate(other) - assert tc.equal_frame_rate(other) is True + expected = tc.equal_frame_rate(other) + with pytest.warns(DeprecationWarning, match="equal_frame_rate"): + actual = tc.equal_framerate(other) + assert actual == expected + assert actual is True # Mismatched rate. assert tc.equal_frame_rate(24.0) is False - assert tc.equal_framerate(24.0) is False + with pytest.warns(DeprecationWarning, match="equal_frame_rate"): + assert tc.equal_framerate(24.0) is False def test_timecode_numeric(): @@ -555,3 +563,15 @@ def test_min_scene_len_accepts_timecode_like(): # ContentDetector: same. ContentDetector(min_scene_len=FrameTimecode(timecode=15, fps=30.0)) ContentDetector(min_scene_len=Timecode(pts=500, time_base=Fraction(1, 1000))) + + +def test_get_framerate(): + """`get_framerate()` emits one warning and preserves its legacy float return value.""" + tc = FrameTimecode(timecode=0, fps=30.0) + + with pytest.warns(DeprecationWarning, match="frame_rate") as warning_info: + frame_rate = tc.get_framerate() + + assert len(warning_info) == 1 + assert frame_rate == 30.0 + assert isinstance(frame_rate, float) diff --git a/tests/test_video_stream.py b/tests/test_video_stream.py index d7e90336..856ad1e7 100644 --- a/tests/test_video_stream.py +++ b/tests/test_video_stream.py @@ -359,14 +359,16 @@ def test_invalid_path(vs_type: ty.Callable[..., VideoStream]): def test_framerate_legacy_alias(vs_type: ty.Callable[..., VideoStream], auto_close): - """`framerate=` is the soft-deprecated alias for `frame_rate=` (issue #548). All backends + """`framerate=` is the deprecated alias for `frame_rate=` (issue #548). All backends must accept both forms and produce the same `frame_rate`.""" path = get_absolute_path("resources/goldeneye.mp4") - legacy = auto_close(vs_type(path, framerate=30.0)) + with pytest.warns(DeprecationWarning, match="frame_rate"): + legacy = auto_close(vs_type(path, framerate=30.0)) canonical = auto_close(vs_type(path, frame_rate=30.0)) assert legacy.frame_rate == canonical.frame_rate # When both are provided, `frame_rate` wins (legacy is ignored). - both = auto_close(vs_type(path, frame_rate=30.0, framerate=24.0)) + with pytest.warns(DeprecationWarning, match="frame_rate"): + both = auto_close(vs_type(path, frame_rate=30.0, framerate=24.0)) assert both.frame_rate == canonical.frame_rate