Skip to content

Commit

Permalink
[cli] Ensure load-scenes can only be specified once
Browse files Browse the repository at this point in the history
Disallow combining `load-scenes` with detectors to avoid confusion. #347
  • Loading branch information
Breakthrough committed Mar 6, 2024
1 parent 4b78450 commit bce23e3
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 8 deletions.
14 changes: 6 additions & 8 deletions scenedetect/_cli/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,8 @@ def get_detect_threshold_params(
else:
min_scene_len = self.config.get_value("detect-threshold", "min-scene-len")
min_scene_len = parse_timecode(min_scene_len, self.video_stream.frame_rate).frame_num

# TODO(v1.0): add_last_scene cannot be disabled right now.
return {
# TODO(v1.0): add_last_scene cannot be disabled right now.
'add_final_scene':
add_last_scene or self.config.get_value("detect-threshold", "add-last-scene"),
'fade_bias':
Expand All @@ -434,6 +433,10 @@ def get_detect_threshold_params(
def handle_load_scenes(self, input: AnyStr, start_col_name: Optional[str]):
"""Handle `load-scenes` command options."""
self._ensure_input_open()
if self.scene_manager._detector_list:
raise click.ClickException(
"The load-scenes command cannot be used with other detectors, and may only be "
"specified once.")
start_col_name = self.config.get_value("load-scenes", "start-col-name", start_col_name)
self.add_detector(
SceneLoader(
Expand Down Expand Up @@ -735,12 +738,7 @@ def _initialize_logging(
def add_detector(self, detector):
""" Add Detector: Adds a detection algorithm to the CliContext's SceneManager. """
self._ensure_input_open()
try:
self.scene_manager.add_detector(detector)
except scenedetect.stats_manager.FrameMetricRegistered as ex:
raise click.BadParameter(
message='Cannot specify detection algorithm twice.',
param_hint=detector.cli_name) from ex
self.scene_manager.add_detector(detector)

def _ensure_input_open(self) -> None:
"""Ensure self.video_stream was initialized (i.e. -i/--input was specified),
Expand Down
10 changes: 10 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,17 @@ def test_cli_load_scenes():
"""Ensure we can load scenes both with and without the cut row."""
assert invoke_scenedetect('-i {VIDEO} time {TIME} {DETECTOR} list-scenes') == 0
assert invoke_scenedetect('-i {VIDEO} time {TIME} load-scenes -i {VIDEO_NAME}-Scenes.csv') == 0
# Specifying a detector with load-scenes should be disallowed.
assert invoke_scenedetect(
'-i {VIDEO} time {TIME} {DETECTOR} load-scenes -i {VIDEO_NAME}-Scenes.csv')
# Specifying load-scenes several times should be disallowed.
assert invoke_scenedetect(
'-i {VIDEO} time {TIME} load-scenes -i {VIDEO_NAME}-Scenes.csv load-scenes -i {VIDEO_NAME}-Scenes.csv'
)
# If `-s`/`--skip-cuts` is specified, the resulting scene list should still be compatible with
# the `load-scenes` command.
assert invoke_scenedetect('-i {VIDEO} time {TIME} {DETECTOR} list-scenes -s') == 0
assert invoke_scenedetect('-i {VIDEO} time {TIME} load-scenes -i {VIDEO_NAME}-Scenes.csv') == 0


def test_cli_load_scenes_with_time_frames():
Expand Down
1 change: 1 addition & 0 deletions website/pages/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ This release focuses on bugfixes and quality of life improvements. This has help
- Valid values: `frames`, `timecode`, `seconds`
- [general] Increase progress bar indent to improve visibility and visual alignment
- [improvement] The `s` suffix for setting timecode values in seconds is no longer required (values without decimal places are still interpreted as frame numbers)
- [general] The `load-scenes` command may only be specified once, and is now disallowed with other `detect-*` commands [#347](https://github.com/Breakthrough/PySceneDetect/issues/347)

**API Changes:**

Expand Down

0 comments on commit bce23e3

Please sign in to comment.