From 9a5f1d00e6b93b85a7afcacd775911239b3b535f Mon Sep 17 00:00:00 2001 From: Tom Han <103209542+tom21100227@users.noreply.github.com> Date: Wed, 1 Jul 2026 14:11:39 -0700 Subject: [PATCH] fix(gui): omit --frames for entire-video inference The "Entire video" inference option encoded its selection as the range [0, len(video)) and serialized it to a fragile `--frames 0,-N` argument. That form only decodes as a range via a legacy `rstrip(",")` quirk in sleap-nn's `frame_list`, and it pins the endpoint to the GUI's frame count. When that count exceeds the number of frames the inference video reader can decode (off-by-one indexing, or a video whose metadata over-reports its length), inference requests a nonexistent frame and crashes with `IndexError: Failed to read frame index N`. Add `VideoItemForInference.is_entire_video` and omit `--frames` for that case so the inference CLI predicts on all frames using its own frame count (its documented default). Clip and explicit-frame selections are unchanged. Fixes #2808. Reported in #2807. With the help of Claude. --- sleap/gui/learning/runners.py | 49 ++++++++++++++++++--- tests/gui/learning/test_cli_construction.py | 46 +++++++++++++++++++ 2 files changed, 89 insertions(+), 6 deletions(-) diff --git a/sleap/gui/learning/runners.py b/sleap/gui/learning/runners.py index acf963d0f..bfe26d20d 100644 --- a/sleap/gui/learning/runners.py +++ b/sleap/gui/learning/runners.py @@ -431,6 +431,30 @@ def path(self): return os.path.abspath(self.video.filename) return self.video.filename + @property + def is_entire_video(self) -> bool: + """Whether `frames` selects every frame of the video. + + The "entire video" option encodes its selection as the half-open range + ``[0, len(video))`` (i.e. ``frames == [0, -len(video)]`` via the + ``encode_range`` helper in `MainWindow._get_frames_for_prediction`). + Detecting this lets `cli_args` omit ``--frames`` so the inference CLI + predicts on all frames using its own frame count, rather than a count + derived from the GUI (which may differ by one). + """ + if self.frames is None: + return False + frames = list(self.frames) + negatives = [f for f in frames if f < 0] + non_negatives = [f for f in frames if f >= 0] + if len(negatives) != 1 or len(non_negatives) != 1: + return False + try: + video_length = len(self.video) + except (TypeError, AttributeError): + return False + return non_negatives[0] == 0 and -negatives[0] == video_length + @property def cli_args(self): arg_list = list() @@ -453,12 +477,25 @@ def cli_args(self): ): arg_list.extend(("--video_input_format", self.video.backend.input_format)) - # -Y represents endpoint of [X, Y) range but inference cli expects - # [X, Y-1] range (so add 1 since negative). - frame_int_list = list(set([i + 1 if i < 0 else i for i in self.frames])) - frame_int_list.sort(reverse=min(frame_int_list) < 0) # Assumes len of 2 if neg. - - arg_list.extend(("--frames", ",".join(map(str, frame_int_list)))) + # `frames` is either an explicit list of (non-negative) frame indices or + # a half-open range [X, Y) whose exclusive endpoint Y is stored as a + # negative value (see `MainWindow._get_frames_for_prediction`). + # + # For the entire video, omit `--frames` and let the inference CLI default + # to predicting on all frames. This avoids emitting the fragile "0,-N" + # range encoding, and avoids pinning the endpoint to the frame count seen + # by the GUI -- which can be one greater than the number of frames the + # inference video reader actually decodes, causing an out-of-range + # `IndexError` on the final frame. + if not self.is_entire_video: + # -Y represents endpoint of [X, Y) range but inference cli expects + # [X, Y-1] range (so add 1 since negative). + frame_int_list = list(set([i + 1 if i < 0 else i for i in self.frames])) + frame_int_list.sort( + reverse=min(frame_int_list) < 0 + ) # Assumes len of 2 if neg. + + arg_list.extend(("--frames", ",".join(map(str, frame_int_list)))) return arg_list diff --git a/tests/gui/learning/test_cli_construction.py b/tests/gui/learning/test_cli_construction.py index 1a715fee3..4d454a098 100644 --- a/tests/gui/learning/test_cli_construction.py +++ b/tests/gui/learning/test_cli_construction.py @@ -132,6 +132,52 @@ def test_cli_args_frame_range(self, mock_video): # -100 becomes -99 (endpoint adjustment) assert "-99" in cli_args[frames_idx] + def test_cli_args_entire_video_omits_frames(self, mock_video): + """Entire-video selection should omit --frames (predict all frames). + + The entire video is encoded as ``[0, -len(video)]``; emitting an + explicit ``--frames 0,-N`` here pins the endpoint to the GUI's frame + count, which can request a frame past the end of the video and crash + inference with an ``IndexError`` (see discussion #2807). + """ + mock_video.__len__.return_value = 100 + item = VideoItemForInference( + video=mock_video, + frames=[0, -100], # encode_range(0, len(video)) for a 100-frame video + labels_path="/path/to/labels.slp", + video_idx=0, + ) + + assert item.is_entire_video + assert "--frames" not in item.cli_args + + def test_cli_args_clip_not_treated_as_entire_video(self, mock_video): + """A clip shorter than the video should still emit --frames.""" + mock_video.__len__.return_value = 100 + item = VideoItemForInference( + video=mock_video, + frames=[0, -50], # frames 0..49 of a 100-frame video + labels_path="/path/to/labels.slp", + video_idx=0, + ) + + assert not item.is_entire_video + cli_args = item.cli_args + assert "--frames" in cli_args + assert cli_args[cli_args.index("--frames") + 1] == "0,-49" + + def test_cli_args_explicit_frame_list_not_entire_video(self, mock_video): + """An explicit list of frames is never treated as the entire video.""" + mock_video.__len__.return_value = 5 + item = VideoItemForInference( + video=mock_video, + frames=[0, 1, 2, 3, 4], + labels_path="/path/to/labels.slp", + ) + + assert not item.is_entire_video + assert "--frames" in item.cli_args + def test_path_property_with_labels(self, mock_video): """path property should return labels_path when provided.""" item = VideoItemForInference(