Skip to content

Commit 006fe37

Browse files
committed
[api] Strengthen type hints for detectors
1 parent 23fc9c5 commit 006fe37

19 files changed

Lines changed: 132 additions & 56 deletions

docs/api/migration_guide.rst

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,37 @@ All backends now return presentation timestamp (PTS) backed values from ``VideoS
160160
``StatsManager`` Changes
161161
=======================================================================
162162

163-
The ``StatsManager`` methods ``get_metrics()``, ``set_metrics()``, and ``metrics_exist()`` now take a ``FrameTimecode`` instead of ``int`` for the frame identifier, matching the detector interface change.
163+
The ``StatsManager`` methods ``get_metrics()``, ``set_metrics()``, and ``metrics_exist()`` now formally accept either a ``FrameTimecode`` or a plain ``int`` frame number for the timecode argument. Passing a ``FrameTimecode`` is preferred and matches the detector interface; the ``int`` form is retained for compatibility with the deprecated ``load_from_csv()`` path, which keys metrics by integer frame number.
164+
165+
``StatsManager.load_from_csv()`` also accepts ``os.PathLike`` (e.g. ``pathlib.Path``) in addition to ``str`` / ``bytes`` / file handles.
166+
167+
168+
=======================================================================
169+
``SceneDetector`` Annotation Fixes
170+
=======================================================================
171+
172+
``SceneDetector.post_process()`` now declares its parameter as ``timecode: FrameTimecode`` (previously typed as ``int``). The method already received a ``FrameTimecode`` at runtime and concrete detectors (e.g. ``ThresholdDetector``, ``ContentDetector``) already used the ``FrameTimecode`` type — only the abstract-base-class annotation was inconsistent. No call-site changes are needed; this just brings the signature into agreement with the documented and actual behavior.
173+
174+
175+
=======================================================================
176+
``SceneManager.detect_scenes()`` Time Arguments
177+
=======================================================================
178+
179+
The ``duration`` and ``end_time`` arguments now formally accept ``int`` (frames), ``float`` (seconds), ``str`` (timecode string, e.g. ``"00:00:05.000"``), or ``FrameTimecode``. The internal code already validated these forms; the annotation was previously narrower than the documented behavior.
180+
181+
.. code:: python
182+
183+
# All of these were always supported at runtime; now they type-check too:
184+
scene_manager.detect_scenes(video=video, end_time=15.0) # seconds
185+
scene_manager.detect_scenes(video=video, end_time=1500) # frames
186+
scene_manager.detect_scenes(video=video, end_time="00:01:00") # timecode
187+
188+
189+
=======================================================================
190+
``save_images()`` Path Handling
191+
=======================================================================
192+
193+
The ``output_dir`` argument of :func:`scenedetect.output.save_images` now accepts ``os.PathLike`` (e.g. ``pathlib.Path``) in addition to ``str``. No changes are required for existing string-based callers.
164194

165195

166196
=======================================================================

pyproject.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ unfixable = []
6161

6262
[tool.pyright]
6363
include = ["scenedetect", "tests"]
64+
# Vendored third-party code: don't lint upstream source (mirrors the ruff
65+
# per-file-ignores convention).
66+
exclude = ["scenedetect/_thirdparty"]
6467
# Run pyright from inside an activated venv (or pass `--pythonpath` /
6568
# configure your editor's interpreter) so cv2 / av / numpy / moviepy
6669
# resolve. Without this, pyright uses its bundled Python and the report

scenedetect/_cli/context.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ def handle_options(
206206

207207
# The `scenedetect` command was just started, let's initialize logging and try to load any
208208
# config files that were specified.
209+
init_log: list = []
209210
try:
210211
init_failure = not self.config.initialized
211212
init_log = self.config.get_init_log()
@@ -305,9 +306,9 @@ def handle_options(
305306
scene_manager.auto_downscale = True
306307
else:
307308
scene_manager.auto_downscale = False
308-
downscale = self.config.get_value("global", "downscale", downscale)
309+
downscale_value: int = self.config.get_value("global", "downscale", downscale)
309310
try:
310-
scene_manager.downscale = downscale
311+
scene_manager.downscale = downscale_value
311312
except ValueError as ex:
312313
logger.debug(str(ex))
313314
raise click.BadParameter(str(ex), param_hint="downscale factor") from ex
@@ -532,11 +533,13 @@ def _open_video_stream(
532533
framerate=framerate,
533534
backend=backend,
534535
)
536+
duration = self.video_stream.duration
537+
duration_str = f"{duration} ({duration.frame_num} frames)" if duration else "unknown"
535538
logger.debug(f"""Video information:
536539
Backend: {type(self.video_stream).__name__}
537540
Resolution: {self.video_stream.frame_size}
538541
Framerate: {self.video_stream.frame_rate}
539-
Duration: {self.video_stream.duration} ({self.video_stream.duration.frame_num} frames)""")
542+
Duration: {duration_str}""")
540543

541544
except FrameRateUnavailable as ex:
542545
if __debug__:

scenedetect/detector.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ def process_frame(
6161

6262
# Optional Methods
6363

64-
def post_process(self, timecode: int) -> list[FrameTimecode]:
64+
def post_process(self, timecode: FrameTimecode) -> list[FrameTimecode]:
6565
"""Called after there are no more frames to process.
6666
6767
Args:

scenedetect/detectors/adaptive_detector.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ def get_metrics(self) -> list[str]:
100100
def process_frame(self, timecode: FrameTimecode, frame_img: np.ndarray) -> list[FrameTimecode]:
101101
super().process_frame(timecode=timecode, frame_img=frame_img)
102102

103+
# If the parent could not calculate a frame score, there's nothing to buffer.
104+
if self._frame_score is None:
105+
return []
106+
103107
# Initialize last scene cut point at the beginning of the frames of interest.
104108
if self._last_cut is None:
105109
self._last_cut = timecode

scenedetect/detectors/content_detector.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,9 @@ def _calculate_frame_score(self, timecode: FrameTimecode, frame_img: numpy.ndarr
169169
delta_sat=_mean_pixel_distance(sat, self._last_frame.sat),
170170
delta_lum=_mean_pixel_distance(lum, self._last_frame.lum),
171171
delta_edges=(
172-
0.0 if edges is None else _mean_pixel_distance(edges, self._last_frame.edges)
172+
0.0
173+
if edges is None or self._last_frame.edges is None
174+
else _mean_pixel_distance(edges, self._last_frame.edges)
173175
),
174176
)
175177

scenedetect/detectors/transnet_v2.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,8 @@ def push(self, pixels: np.ndarray, time: np.ndarray):
107107
),
108108
)
109109
else:
110+
# `self.time` is set in lockstep with `self.pixels` above, so it is non-None here.
111+
assert self.time is not None
110112
c1 = self.pixels
111113
c2 = pixels
112114

scenedetect/output/image.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
Interpolation,
2929
SceneList,
3030
)
31-
from scenedetect.platform import get_and_create_path, get_cv2_imwrite_params, tqdm
31+
from scenedetect.platform import StrPath, get_and_create_path, get_cv2_imwrite_params, tqdm
3232
from scenedetect.video_stream import VideoStream
3333

3434
logger = logging.getLogger("pyscenedetect")
@@ -162,7 +162,7 @@ def run(
162162
self,
163163
video: VideoStream,
164164
scene_list: SceneList,
165-
output_dir: str | None = None,
165+
output_dir: StrPath | None = None,
166166
show_progress=False,
167167
) -> dict[int, list[str]]:
168168
"""Run image extraction on `video` using the current parameters. Thread-safe.
@@ -355,7 +355,7 @@ def save_images(
355355
image_extension: str = "jpg",
356356
encoder_param: int = 95,
357357
image_name_template: str = "$VIDEO_NAME-Scene-$SCENE_NUMBER-$IMAGE_NUMBER",
358-
output_dir: str | None = None,
358+
output_dir: StrPath | None = None,
359359
show_progress: bool | None = False,
360360
scale: float | None = None,
361361
height: int | None = None,

scenedetect/scene_manager.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,8 +419,8 @@ def stop(self) -> None:
419419
def detect_scenes(
420420
self,
421421
video: VideoStream | None = None,
422-
duration: FrameTimecode | None = None,
423-
end_time: FrameTimecode | None = None,
422+
duration: "int | float | str | FrameTimecode | None" = None,
423+
end_time: "int | float | str | FrameTimecode | None" = None,
424424
frame_skip: int = 0,
425425
show_progress: bool = False,
426426
callback: ty.Callable[[np.ndarray, FrameTimecode], None] | None = None,

scenedetect/stats_manager.py

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -96,19 +96,22 @@ class StatsManager:
9696
Only metrics consisting of `float` or `int` should be used currently.
9797
"""
9898

99-
def __init__(self, base_timecode: FrameTimecode | None = None):
99+
def __init__(self, base_timecode: int | FrameTimecode | None = None):
100100
"""Initialize a new StatsManager.
101101
102102
Arguments:
103103
base_timecode: Timecode associated with this object. Must not be None (default value
104104
will be removed in a future release).
105105
"""
106-
# Frame metrics is a dict of frame (int): metric_dict (Dict[str, float])
107-
# of each frame metric key and the value it represents (usually float).
108-
self._frame_metrics: dict[FrameTimecode, dict[str, float]] = dict()
106+
# Frame metrics keyed by either an `int` frame number or a `FrameTimecode`. Both forms
107+
# hash/compare to the same dict slot (`FrameTimecode.__hash__` returns `frame_num`), so
108+
# public methods accept both interchangeably for the same frame.
109+
self._frame_metrics: dict[int | FrameTimecode, dict[str, float]] = dict()
109110
self._metric_keys: set[str] = set()
110111
self._metrics_updated: bool = False # Flag indicating if metrics require saving.
111-
self._base_timecode: FrameTimecode | None = base_timecode # Used for timing calculations.
112+
self._base_timecode: int | FrameTimecode | None = (
113+
base_timecode # Used for timing calculations.
114+
)
112115

113116
@property
114117
def metric_keys(self) -> ty.Iterable[str]:
@@ -120,7 +123,9 @@ def register_metrics(self, metric_keys: ty.Iterable[str]) -> None:
120123

121124
# TODO(https://scenedetect.com/issues/507): We should support the dictionary protocol instead
122125
# of using this bespoke interface. It would be useful for Pandas compatibility as well.
123-
def get_metrics(self, timecode: FrameTimecode, metric_keys: ty.Iterable[str]) -> list[ty.Any]:
126+
def get_metrics(
127+
self, timecode: int | FrameTimecode, metric_keys: ty.Iterable[str]
128+
) -> list[ty.Any]:
124129
"""Return the requested statistics/metrics for a given timecode.
125130
126131
Returns:
@@ -129,7 +134,7 @@ def get_metrics(self, timecode: FrameTimecode, metric_keys: ty.Iterable[str]) ->
129134
"""
130135
return [self._get_metric(timecode, metric_key) for metric_key in metric_keys]
131136

132-
def set_metrics(self, timecode: FrameTimecode, metric_kv_dict: dict[str, ty.Any]) -> None:
137+
def set_metrics(self, timecode: int | FrameTimecode, metric_kv_dict: dict[str, ty.Any]) -> None:
133138
"""Set Metrics: Sets the provided statistics/metrics for a given frame.
134139
135140
Arguments:
@@ -139,7 +144,7 @@ def set_metrics(self, timecode: FrameTimecode, metric_kv_dict: dict[str, ty.Any]
139144
for metric_key in metric_kv_dict:
140145
self._set_metric(timecode, metric_key, metric_kv_dict[metric_key])
141146

142-
def metrics_exist(self, timecode: FrameTimecode, metric_keys: ty.Iterable[str]) -> bool:
147+
def metrics_exist(self, timecode: int | FrameTimecode, metric_keys: ty.Iterable[str]) -> bool:
143148
"""Metrics Exist: Checks if the given metrics/stats exist for the given frame.
144149
145150
Returns:
@@ -188,6 +193,10 @@ def save_to_csv(
188193
frame_keys = sorted(self._frame_metrics.keys())
189194
logger.info("Writing %d frames to CSV...", len(frame_keys))
190195
for frame_key in frame_keys:
196+
# `frame_key` may be a bare `int` if the deprecated `load_from_csv` populated the dict.
197+
# Skip such rows since we cannot recover a timecode without a base framerate.
198+
if not isinstance(frame_key, FrameTimecode):
199+
continue
191200
csv_writer.writerow(
192201
[frame_key.frame_num + 1, frame_key.get_timecode()]
193202
+ [str(metric) for metric in self.get_metrics(frame_key, metric_keys)]
@@ -209,7 +218,7 @@ def valid_header(row: list[str]) -> bool:
209218

210219
# TODO(v1.0): Create a replacement for a calculation cache that functions like load_from_csv
211220
# did, but is better integrated with detectors for cached calculations instead of statistics.
212-
def load_from_csv(self, csv_file: str | bytes | ty.TextIO) -> int | None:
221+
def load_from_csv(self, csv_file: StrPath | bytes | ty.TextIO) -> int | None:
213222
"""[DEPRECATED] DO NOT USE
214223
215224
Load all metrics stored in a CSV file into the StatsManager instance. Will be removed in a
@@ -233,7 +242,7 @@ def load_from_csv(self, csv_file: str | bytes | ty.TextIO) -> int | None:
233242

234243
# If we get a path instead of an open file handle, check that it exists, and if so,
235244
# recursively call ourselves again but with file set instead of path.
236-
if isinstance(csv_file, (str, bytes, Path)):
245+
if isinstance(csv_file, (str, bytes, os.PathLike)):
237246
if os.path.exists(csv_file):
238247
with open(csv_file) as file:
239248
return self.load_from_csv(csv_file=file)
@@ -288,16 +297,18 @@ def load_from_csv(self, csv_file: str | bytes | ty.TextIO) -> int | None:
288297

289298
# TODO: Get rid of these functions and simplify the implementation of this class.
290299

291-
def _get_metric(self, timecode: FrameTimecode, metric_key: str) -> ty.Any | None:
300+
def _get_metric(self, timecode: int | FrameTimecode, metric_key: str) -> ty.Any | None:
292301
if self._metric_exists(timecode, metric_key):
293302
return self._frame_metrics[timecode][metric_key]
294303
return None
295304

296-
def _set_metric(self, timecode: FrameTimecode, metric_key: str, metric_value: ty.Any) -> None:
305+
def _set_metric(
306+
self, timecode: int | FrameTimecode, metric_key: str, metric_value: ty.Any
307+
) -> None:
297308
self._metrics_updated = True
298309
if timecode not in self._frame_metrics:
299310
self._frame_metrics[timecode] = dict()
300311
self._frame_metrics[timecode][metric_key] = metric_value
301312

302-
def _metric_exists(self, timecode: FrameTimecode, metric_key: str) -> bool:
313+
def _metric_exists(self, timecode: int | FrameTimecode, metric_key: str) -> bool:
303314
return timecode in self._frame_metrics and metric_key in self._frame_metrics[timecode]

0 commit comments

Comments
 (0)