Skip to content

Commit c508441

Browse files
committed
[cli] Strengthen type guards
1 parent 268e259 commit c508441

4 files changed

Lines changed: 86 additions & 114 deletions

File tree

scenedetect/_cli/__init__.py

Lines changed: 34 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,15 @@
2727

2828
import click
2929

30-
import scenedetect
30+
import scenedetect as scenedetect_pkg
3131
import scenedetect._cli.commands as cli_commands
3232
from scenedetect._cli.config import (
3333
CHOICE_MAP,
3434
CONFIG_FILE_PATH,
3535
CONFIG_MAP,
3636
DEFAULT_JPG_QUALITY,
3737
DEFAULT_WEBP_QUALITY,
38+
RangeValue,
3839
)
3940
from scenedetect._cli.context import USER_CONFIG, CliContext, check_split_video_requirements
4041
from scenedetect.backends import AVAILABLE_BACKENDS
@@ -47,13 +48,24 @@
4748
)
4849
from scenedetect.platform import get_cv2_imwrite_params, get_system_version_info
4950

50-
PROGRAM_VERSION = scenedetect.__version__
51+
PROGRAM_VERSION = scenedetect_pkg.__version__
5152
"""Used to avoid name conflict with named `scenedetect` command below."""
5253

5354
logger = logging.getLogger("pyscenedetect")
5455

5556
LINE_SEPARATOR = "-" * 72
5657

58+
59+
def _click_range(section: str, key: str) -> "click.IntRange | click.FloatRange":
60+
"""Return a `click` parameter type matching the `RangeValue` at `CONFIG_MAP[section][key]`.
61+
62+
Used in `@click.option(... type=...)` decorators so each option's bounds and value type are
63+
sourced from the canonical `CONFIG_MAP` entry.
64+
"""
65+
val = CONFIG_MAP[section][key]
66+
assert isinstance(val, RangeValue), f"Expected RangeValue at {section}/{key}, got {type(val)}"
67+
return val.click_range
68+
5769
# About & copyright message string shown for the 'about' CLI command (scenedetect about).
5870
ABOUT_STRING = """
5971
Site: http://scenedetect.com/
@@ -375,7 +387,8 @@ def add_hidden_alias(command: click.Command, alias: str):
375387
def help_command(ctx: click.Context, command_name: str):
376388
"""Print full help reference."""
377389
# TODO: Other commands still seem to run if this is specified.
378-
assert isinstance(ctx.parent.command, click.MultiCommand)
390+
assert ctx.parent is not None
391+
assert isinstance(ctx.parent.command, click.Group)
379392
parent_command = ctx.parent.command
380393
all_commands = set(parent_command.list_commands(ctx))
381394
if command_name is not None:
@@ -386,11 +399,15 @@ def help_command(ctx: click.Context, command_name: str):
386399
]
387400
raise click.BadParameter("\n".join(error_strs), param_hint="command")
388401
click.echo("")
389-
print_command_help(ctx, parent_command.get_command(ctx, command_name))
402+
target = parent_command.get_command(ctx, command_name)
403+
assert target is not None
404+
print_command_help(ctx, target)
390405
else:
391406
click.echo(ctx.parent.get_help())
392407
for command in sorted(all_commands):
393-
print_command_help(ctx, parent_command.get_command(ctx, command))
408+
target = parent_command.get_command(ctx, command)
409+
assert target is not None
410+
print_command_help(ctx, target)
394411
ctx.exit()
395412

396413

@@ -509,10 +526,7 @@ def time_command(
509526
"--threshold",
510527
"-t",
511528
metavar="VAL",
512-
type=click.FloatRange(
513-
CONFIG_MAP["detect-content"]["threshold"].min_val,
514-
CONFIG_MAP["detect-content"]["threshold"].max_val,
515-
),
529+
type=_click_range("detect-content", "threshold"),
516530
default=None,
517531
help='The max difference (0.0 to 255.0) that adjacent frames score must exceed to trigger a cut. Lower values are more sensitive to shot changes. Refers to "content_val" in stats file.{}'.format(
518532
USER_CONFIG.get_help_string("detect-content", "threshold")
@@ -720,10 +734,7 @@ def detect_adaptive_command(
720734
"--threshold",
721735
"-t",
722736
metavar="VAL",
723-
type=click.FloatRange(
724-
CONFIG_MAP["detect-threshold"]["threshold"].min_val,
725-
CONFIG_MAP["detect-threshold"]["threshold"].max_val,
726-
),
737+
type=_click_range("detect-threshold", "threshold"),
727738
default=None,
728739
help='Threshold (integer) that frame score must exceed to start a new scene. Refers to "delta_rgb" in stats file.{}'.format(
729740
USER_CONFIG.get_help_string("detect-threshold", "threshold")
@@ -733,10 +744,7 @@ def detect_adaptive_command(
733744
"--fade-bias",
734745
"-f",
735746
metavar="PERCENT",
736-
type=click.FloatRange(
737-
CONFIG_MAP["detect-threshold"]["fade-bias"].min_val,
738-
CONFIG_MAP["detect-threshold"]["fade-bias"].max_val,
739-
),
747+
type=_click_range("detect-threshold", "fade-bias"),
740748
default=None,
741749
help="Percent (%) from -100 to 100 of timecode skew of cut placement. -100 indicates the start frame, +100 indicates the end frame, and 0 is the middle of both.{}".format(
742750
USER_CONFIG.get_help_string("detect-threshold", "fade-bias")
@@ -802,10 +810,7 @@ def detect_threshold_command(
802810
"--threshold",
803811
"-t",
804812
metavar="VAL",
805-
type=click.FloatRange(
806-
CONFIG_MAP["detect-hist"]["threshold"].min_val,
807-
CONFIG_MAP["detect-hist"]["threshold"].max_val,
808-
),
813+
type=_click_range("detect-hist", "threshold"),
809814
default=None,
810815
help="Max difference (0.0 to 1.0) between histograms of adjacent frames. Lower "
811816
"values are more sensitive to changes.{}".format(
@@ -816,9 +821,7 @@ def detect_threshold_command(
816821
"--bins",
817822
"-b",
818823
metavar="NUM",
819-
type=click.IntRange(
820-
CONFIG_MAP["detect-hist"]["bins"].min_val, CONFIG_MAP["detect-hist"]["bins"].max_val
821-
),
824+
type=_click_range("detect-hist", "bins"),
822825
default=None,
823826
help="The number of bins to use for the histogram calculation.{}".format(
824827
USER_CONFIG.get_help_string("detect-hist", "bins")
@@ -873,10 +876,7 @@ def detect_hist_command(
873876
"--threshold",
874877
"-t",
875878
metavar="VAL",
876-
type=click.FloatRange(
877-
CONFIG_MAP["detect-hash"]["threshold"].min_val,
878-
CONFIG_MAP["detect-hash"]["threshold"].max_val,
879-
),
879+
type=_click_range("detect-hash", "threshold"),
880880
default=None,
881881
help=(
882882
"Max distance between hash values (0.0 to 1.0) of adjacent frames. Lower values are "
@@ -889,9 +889,7 @@ def detect_hist_command(
889889
"--size",
890890
"-s",
891891
metavar="SIZE",
892-
type=click.IntRange(
893-
CONFIG_MAP["detect-hash"]["size"].min_val, CONFIG_MAP["detect-hash"]["size"].max_val
894-
),
892+
type=_click_range("detect-hash", "size"),
895893
default=None,
896894
help="Size of square of low frequency data to include from the discrete cosine transform.{}".format(
897895
USER_CONFIG.get_help_string("detect-hash", "size")
@@ -901,9 +899,7 @@ def detect_hist_command(
901899
"--lowpass",
902900
"-l",
903901
metavar="FRAC",
904-
type=click.IntRange(
905-
CONFIG_MAP["detect-hash"]["lowpass"].min_val, CONFIG_MAP["detect-hash"]["lowpass"].max_val
906-
),
902+
type=_click_range("detect-hash", "lowpass"),
907903
default=None,
908904
help=(
909905
"How much high frequency information to filter from the DCT. 2 means keep lower 1/2 of "
@@ -984,6 +980,8 @@ def load_scenes_command(ctx: click.Context, input: str | None, start_col_name: s
984980
raise click.ClickException("The load-scenes command cannot be used with detectors.")
985981
if ctx.load_scenes_input:
986982
raise click.ClickException("The load-scenes command must only be specified once.")
983+
if input is None:
984+
raise click.BadParameter("Input file is required.", param_hint="-i/--input")
987985
input = os.path.abspath(input)
988986
if not os.path.exists(input):
989987
raise click.BadParameter(
@@ -1066,6 +1064,7 @@ def save_html_command(
10661064
# to include images.
10671065
include_images = not ctx.config.get_value("save-html", "no-images", no_images)
10681066
if include_images and not ctx.save_images:
1067+
assert save_images_command.callback is not None
10691068
save_images_command.callback()
10701069
save_html_args = {
10711070
"filename": ctx.config.get_value("save-html", "filename", filename),
@@ -1239,10 +1238,7 @@ def list_scenes_command(
12391238
"-crf",
12401239
metavar="RATE",
12411240
default=None,
1242-
type=click.IntRange(
1243-
CONFIG_MAP["split-video"]["rate-factor"].min_val,
1244-
CONFIG_MAP["split-video"]["rate-factor"].max_val,
1245-
),
1241+
type=_click_range("split-video", "rate-factor"),
12461242
help="Video encoding quality (x264 constant rate factor), from 0-100, where lower is higher quality (larger output). 0 indicates lossless.{}".format(
12471243
USER_CONFIG.get_help_string("split-video", "rate-factor")
12481244
),

scenedetect/_cli/config.py

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from configparser import Error as ConfigParserError
2424
from enum import Enum
2525

26+
import click
2627
from platformdirs import user_config_dir
2728

2829
from scenedetect.common import FrameTimecode
@@ -124,6 +125,13 @@ def max_val(self) -> int | float:
124125
"""Maximum value of the range."""
125126
return self._max_val
126127

128+
@property
129+
def click_range(self) -> "click.IntRange | click.FloatRange":
130+
"""A `click` parameter type matching this range's bounds and value type."""
131+
if isinstance(self._value, int):
132+
return click.IntRange(int(self._min_val), int(self._max_val))
133+
return click.FloatRange(float(self._min_val), float(self._max_val))
134+
127135
@staticmethod
128136
def from_config(config_value: str, default: "RangeValue") -> "RangeValue":
129137
try:
@@ -757,9 +765,14 @@ def get_value(
757765
self,
758766
command: str,
759767
option: str,
760-
override: ConfigValue | None = None,
761-
) -> ConfigValue:
762-
"""Get the current setting or default value of the specified command option."""
768+
override: ty.Any = None,
769+
) -> ty.Any:
770+
"""Get the current setting or default value of the specified command option.
771+
772+
Returns ``ty.Any`` because each (command, option) pair has a known concrete type at
773+
the call site, but the union across all options is too wide to be useful as a return
774+
annotation. Callers should know the expected type for the option they are reading.
775+
"""
763776
assert command in CONFIG_MAP and option in CONFIG_MAP[command]
764777
if override is not None:
765778
value = override

scenedetect/_cli/context.py

Lines changed: 26 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,22 @@ def ensure_detector(self):
133133
(detector_type, detector_args) = self.default_detector
134134
self.add_detector(detector_type, detector_args)
135135

136+
def _resolve_min_scene_len(self, command: str, override: str | None) -> int:
137+
"""Resolve the minimum scene length (in frames) for a `detect-*` command, honoring
138+
the `--drop-short-scenes` flag, command-specific config, and global default."""
139+
if self.drop_short_scenes:
140+
return 0
141+
if override is not None:
142+
parsed = self.parse_timecode(override)
143+
assert parsed is not None
144+
return parsed.frame_num
145+
if self.config.is_default(command, "min-scene-len"):
146+
assert self.min_scene_len is not None
147+
return self.min_scene_len.frame_num
148+
parsed = self.parse_timecode(self.config.get_value(command, "min-scene-len"))
149+
assert parsed is not None
150+
return parsed.frame_num
151+
136152
def parse_timecode(self, value: str | None, correct_pts: bool = False) -> FrameTimecode | None:
137153
"""Parses a user input string into a FrameTimecode assuming the given framerate. If `value`
138154
is None it will be passed through without processing.
@@ -328,18 +344,7 @@ def get_detect_content_params(
328344
filter_mode: str | None = None,
329345
) -> dict[str, ty.Any]:
330346
"""Get a dict containing user options to construct a ContentDetector with."""
331-
if self.drop_short_scenes:
332-
min_scene_len = 0
333-
else:
334-
if min_scene_len is None:
335-
if self.config.is_default("detect-content", "min-scene-len"):
336-
assert self.min_scene_len is not None
337-
min_scene_len = self.min_scene_len.frame_num
338-
else:
339-
min_scene_len = self.config.get_value("detect-content", "min-scene-len")
340-
parsed = self.parse_timecode(min_scene_len)
341-
assert parsed is not None
342-
min_scene_len = parsed.frame_num
347+
min_scene_len_frames = self._resolve_min_scene_len("detect-content", min_scene_len)
343348

344349
if weights is not None:
345350
try:
@@ -354,7 +359,7 @@ def get_detect_content_params(
354359
"weights": self.config.get_value("detect-content", "weights", weights),
355360
"kernel_size": self.config.get_value("detect-content", "kernel-size", kernel_size),
356361
"luma_only": luma_only or self.config.get_value("detect-content", "luma-only"),
357-
"min_scene_len": min_scene_len,
362+
"min_scene_len": min_scene_len_frames,
358363
"threshold": self.config.get_value("detect-content", "threshold", threshold),
359364
"filter_mode": self.config.get_value("detect-content", "filter-mode", filter_mode),
360365
}
@@ -371,18 +376,7 @@ def get_detect_adaptive_params(
371376
) -> dict[str, ty.Any]:
372377
"""Handle detect-adaptive command options and return args to construct one with."""
373378

374-
if self.drop_short_scenes:
375-
min_scene_len = 0
376-
else:
377-
if min_scene_len is None:
378-
if self.config.is_default("detect-adaptive", "min-scene-len"):
379-
assert self.min_scene_len is not None
380-
min_scene_len = self.min_scene_len.frame_num
381-
else:
382-
min_scene_len = self.config.get_value("detect-adaptive", "min-scene-len")
383-
parsed = self.parse_timecode(min_scene_len)
384-
assert parsed is not None
385-
min_scene_len = parsed.frame_num
379+
min_scene_len_frames = self._resolve_min_scene_len("detect-adaptive", min_scene_len)
386380

387381
if weights is not None:
388382
try:
@@ -400,7 +394,7 @@ def get_detect_adaptive_params(
400394
"min_content_val": self.config.get_value(
401395
"detect-adaptive", "min-content-val", min_content_val
402396
),
403-
"min_scene_len": min_scene_len,
397+
"min_scene_len": min_scene_len_frames,
404398
"window_width": self.config.get_value("detect-adaptive", "frame-window", frame_window),
405399
}
406400

@@ -413,24 +407,13 @@ def get_detect_threshold_params(
413407
) -> dict[str, ty.Any]:
414408
"""Handle detect-threshold command options and return args to construct one with."""
415409

416-
if self.drop_short_scenes:
417-
min_scene_len = 0
418-
else:
419-
if min_scene_len is None:
420-
if self.config.is_default("detect-threshold", "min-scene-len"):
421-
assert self.min_scene_len is not None
422-
min_scene_len = self.min_scene_len.frame_num
423-
else:
424-
min_scene_len = self.config.get_value("detect-threshold", "min-scene-len")
425-
parsed = self.parse_timecode(min_scene_len)
426-
assert parsed is not None
427-
min_scene_len = parsed.frame_num
410+
min_scene_len_frames = self._resolve_min_scene_len("detect-threshold", min_scene_len)
428411
# TODO(v1.0): add_last_scene cannot be disabled right now.
429412
return {
430413
"add_final_scene": add_last_scene
431414
or self.config.get_value("detect-threshold", "add-last-scene"),
432415
"fade_bias": self.config.get_value("detect-threshold", "fade-bias", fade_bias),
433-
"min_scene_len": min_scene_len,
416+
"min_scene_len": min_scene_len_frames,
434417
"threshold": self.config.get_value("detect-threshold", "threshold", threshold),
435418
}
436419

@@ -442,21 +425,10 @@ def get_detect_hist_params(
442425
) -> dict[str, ty.Any]:
443426
"""Handle detect-hist command options and return args to construct one with."""
444427

445-
if self.drop_short_scenes:
446-
min_scene_len = 0
447-
else:
448-
if min_scene_len is None:
449-
if self.config.is_default("detect-hist", "min-scene-len"):
450-
assert self.min_scene_len is not None
451-
min_scene_len = self.min_scene_len.frame_num
452-
else:
453-
min_scene_len = self.config.get_value("detect-hist", "min-scene-len")
454-
parsed = self.parse_timecode(min_scene_len)
455-
assert parsed is not None
456-
min_scene_len = parsed.frame_num
428+
min_scene_len_frames = self._resolve_min_scene_len("detect-hist", min_scene_len)
457429
return {
458430
"bins": self.config.get_value("detect-hist", "bins", bins),
459-
"min_scene_len": min_scene_len,
431+
"min_scene_len": min_scene_len_frames,
460432
"threshold": self.config.get_value("detect-hist", "threshold", threshold),
461433
}
462434

@@ -469,21 +441,10 @@ def get_detect_hash_params(
469441
) -> dict[str, ty.Any]:
470442
"""Handle detect-hash command options and return args to construct one with."""
471443

472-
if self.drop_short_scenes:
473-
min_scene_len = 0
474-
else:
475-
if min_scene_len is None:
476-
if self.config.is_default("detect-hash", "min-scene-len"):
477-
assert self.min_scene_len is not None
478-
min_scene_len = self.min_scene_len.frame_num
479-
else:
480-
min_scene_len = self.config.get_value("detect-hash", "min-scene-len")
481-
parsed = self.parse_timecode(min_scene_len)
482-
assert parsed is not None
483-
min_scene_len = parsed.frame_num
444+
min_scene_len_frames = self._resolve_min_scene_len("detect-hash", min_scene_len)
484445
return {
485446
"lowpass": self.config.get_value("detect-hash", "lowpass", lowpass),
486-
"min_scene_len": min_scene_len,
447+
"min_scene_len": min_scene_len_frames,
487448
"size": self.config.get_value("detect-hash", "size", size),
488449
"threshold": self.config.get_value("detect-hash", "threshold", threshold),
489450
}

0 commit comments

Comments
 (0)