From 95b2e0d177742610c4a0943888320a06d72927b6 Mon Sep 17 00:00:00 2001 From: Glen Beane <356266+gbeane@users.noreply.github.com> Date: Tue, 14 Jul 2026 14:03:56 -0400 Subject: [PATCH 1/6] Defer video/pose frame-count check off project load (KLAUS-505) --- src/jabs/project/parallel_workers.py | 30 +++++++++++++++++-- src/jabs/project/project.py | 8 +++-- src/jabs/project/video_manager.py | 7 +++-- src/jabs/ui/main_window/central_widget.py | 36 +++++++++++++++++++++++ src/jabs/video_reader/utilities.py | 25 ++++++++++++++++ tests/project/test_parallel_workers.py | 25 ++++++++++++++-- tests/project/test_project.py | 19 ++++++++++-- tests/ui/test_central_widget.py | 14 +++++++++ 8 files changed, 153 insertions(+), 11 deletions(-) diff --git a/src/jabs/project/parallel_workers.py b/src/jabs/project/parallel_workers.py index d54ae2c4..3d986457 100644 --- a/src/jabs/project/parallel_workers.py +++ b/src/jabs/project/parallel_workers.py @@ -21,7 +21,7 @@ from jabs.core.enums import CacheFormat from jabs.pose_estimation import open_pose_file from jabs.video_reader import VideoReader -from jabs.video_reader.utilities import get_fps +from jabs.video_reader.utilities import get_fps_and_nframes from .track_labels import TrackLabels from .video_labels import VideoLabels @@ -212,13 +212,39 @@ def _apply_macos_fork_lapack_workaround() -> None: fe.feature_base_class._use_numpy_detrend = True +def _warn_on_frame_count_mismatch( + video: str, video_frame_count: int, pose_frame_count: int +) -> None: + """Log a warning when a video and its pose file disagree on frame count. + + The video/pose frame-count check is deferred from project load. Feature + extraction and predictions are indexed to pose frames, so a mismatch does + not corrupt them; this surfaces the discrepancy for headless/batch runs + where no video is opened interactively. + + Args: + video: Video filename, used in the log message. + video_frame_count: Frame count read from the video file. + pose_frame_count: Frame count from the pose file. + """ + if video_frame_count != pose_frame_count: + logger.warning( + "%s: video frame count (%d) does not match pose file frame count (%d); " + "features and predictions are pose-indexed and unaffected", + video, + video_frame_count, + pose_frame_count, + ) + + def _open_pose_and_labels( job: _BaseFeatureLoadJobSpec, ) -> "tuple[PoseEstimation, VideoLabels | None, float]": """Set up the macOS workaround and open pose + label resources for a job.""" _apply_macos_fork_lapack_workaround() pose_est = open_pose_file(job["pose_path"], job["cache_dir"]) - fps = get_fps(str(job["video_path"])) + fps, video_frame_count = get_fps_and_nframes(str(job["video_path"])) + _warn_on_frame_count_mismatch(job["video"], video_frame_count, pose_est.num_frames) labels_obj = _load_video_labels(job["annotations_path"], pose_est) return pose_est, labels_obj, fps diff --git a/src/jabs/project/project.py b/src/jabs/project/project.py index f25faad5..a9f7699f 100644 --- a/src/jabs/project/project.py +++ b/src/jabs/project/project.py @@ -81,7 +81,11 @@ class Project: project_path: Path to the project directory. process_pool: Optional shared ProcessPoolManager for feature extraction. If None, runs single-threaded. use_cache: Whether to use cached data. - enable_video_check: Whether to check for video file validity. + enable_video_check: Opt-in up-front validation that every video's frame + count matches its pose file. When True, opens every video file at + load time (expensive). When False (the default), this check is + deferred to when a video is opened (GUI) and to the feature-extraction + workers (headless), which warn on a mismatch. enable_session_tracker: Whether to enable session tracking for this project. validate_project_dir: Whether to validate the project directory structure on creation. video_dir: Optional directory containing video files. Defaults to `project_path`. @@ -106,7 +110,7 @@ def __init__( project_path, process_pool: "ProcessPoolManager | None" = None, use_cache=True, - enable_video_check=True, + enable_video_check=False, enable_session_tracker=True, validate_project_dir=True, video_dir: Path | None = None, diff --git a/src/jabs/project/video_manager.py b/src/jabs/project/video_manager.py index aafea1d6..91f86067 100644 --- a/src/jabs/project/video_manager.py +++ b/src/jabs/project/video_manager.py @@ -31,8 +31,9 @@ class VideoManager: Args: paths: Object containing project directory paths. settings_manager: Manages project settings and metadata. - enable_video_check: Whether to validate that video and pose file frame - counts match. Defaults to True. + enable_video_check: Opt-in up-front validation that video and pose file + frame counts match; when True, every video file is opened. Defaults + to False, which defers the check to when a video is opened. scan_results: Per-video metadata collected by :func:`~jabs.project.parallel_workers.scan_video_metadata`, keyed by video filename. Every video in the project directory must have an @@ -47,7 +48,7 @@ def __init__( self, paths: ProjectPaths, settings_manager: SettingsManager, - enable_video_check: bool = True, + enable_video_check: bool = False, *, scan_results: "dict[str, VideoScanResult]", ): diff --git a/src/jabs/ui/main_window/central_widget.py b/src/jabs/ui/main_window/central_widget.py index d0f5168a..cee855cc 100644 --- a/src/jabs/ui/main_window/central_widget.py +++ b/src/jabs/ui/main_window/central_widget.py @@ -368,6 +368,33 @@ def set_project(self, project: Project) -> None: self._search_bar_widget.update_project(project) self._update_timeline_search_results() + @staticmethod + def _frame_count_mismatch_message( + video_name: str, video_frame_count: int, pose_frame_count: int + ) -> str | None: + """Build a warning message when video and pose frame counts differ. + + The project-load frame-count check is deferred, so it runs here when a + video is opened. Labels and features are indexed to pose frames while the + player displays video frames, so a mismatch means the displayed frame may + not line up with labels for this video. + + Args: + video_name: Name of the video being opened. + video_frame_count: Frame count reported by the video player. + pose_frame_count: Frame count from the pose file. + + Returns: + A user-facing warning message, or None when the counts match. + """ + if video_frame_count == pose_frame_count: + return None + return ( + f"{video_name} has {video_frame_count} video frames but its pose file " + f"has {pose_frame_count}. Labels are indexed to pose frames, so the " + f"displayed frame may not align with labels for this video." + ) + def load_video(self, path: Path) -> None: """load a new video file into self._player_widget @@ -398,6 +425,15 @@ def load_video(self, path: Path) -> None: self._player_widget.load_video(path, self._pose_est, self._labels) + # Frame-count check deferred from project load: warn (do not block) + # if the video and its pose file disagree on frame count, since labels + # and features are pose-indexed but the player shows video frames. + mismatch_message = self._frame_count_mismatch_message( + path.name, self._player_widget.num_frames, self._pose_est.num_frames + ) + if mismatch_message is not None: + MessageDialog.warning(self, "Video / pose frame count mismatch", mismatch_message) + # load saved predictions for this video ( self._predictions, diff --git a/src/jabs/video_reader/utilities.py b/src/jabs/video_reader/utilities.py index 045d42d6..37f6c469 100644 --- a/src/jabs/video_reader/utilities.py +++ b/src/jabs/video_reader/utilities.py @@ -29,3 +29,28 @@ def get_fps(video_path: str): raise OSError(f"unable to open {video_path}") return round(stream.get(cv2.CAP_PROP_FPS)) + + +def get_fps_and_nframes(video_path: str) -> tuple[int, int]: + """Get the frames per second and frame count from a video in a single open. + + Reads both properties from one ``cv2.VideoCapture`` handle so callers that + already need the FPS can obtain the frame count without a second file open. + + Args: + video_path: string containing path to video file. + + Returns: + Tuple of ``(fps, num_frames)`` where ``fps`` is rounded to an int. + + Raises: + OSError: if unable to open the specified video. + """ + stream = cv2.VideoCapture(video_path) + if not stream.isOpened(): + raise OSError(f"unable to open {video_path}") + + fps = round(stream.get(cv2.CAP_PROP_FPS)) + num_frames = int(stream.get(cv2.CAP_PROP_FRAME_COUNT)) + stream.release() + return fps, num_frames diff --git a/tests/project/test_parallel_workers.py b/tests/project/test_parallel_workers.py index 41ab6a6b..9b577f1e 100644 --- a/tests/project/test_parallel_workers.py +++ b/tests/project/test_parallel_workers.py @@ -1,5 +1,6 @@ """Tests for parallel_workers — scan_video_metadata and _get_identity_count.""" +import logging import shutil from pathlib import Path from unittest.mock import MagicMock, patch @@ -13,6 +14,7 @@ VideoScanJobSpec, VideoScanResult, _get_identity_count, + _warn_on_frame_count_mismatch, collect_multiclass_labeled_features, scan_video_metadata, ) @@ -26,6 +28,7 @@ class _MockPose: def __init__(self, mask: np.ndarray): self._mask = mask self.identities = [0] + self.num_frames = int(mask.shape[0]) def identity_mask(self, identity: int) -> np.ndarray: assert identity == 0 @@ -64,7 +67,9 @@ def merge_window_features(window_features: dict[str, np.ndarray]) -> dict[str, n return window_features monkeypatch.setattr("jabs.project.parallel_workers.open_pose_file", lambda *_: pose) - monkeypatch.setattr("jabs.project.parallel_workers.get_fps", lambda *_: 30) + monkeypatch.setattr( + "jabs.project.parallel_workers.get_fps_and_nframes", lambda *_: (30, pose.num_frames) + ) monkeypatch.setattr("jabs.project.parallel_workers._load_video_labels", lambda *_: labels) monkeypatch.setattr("jabs.project.parallel_workers.fe.IdentityFeatures", _FakeIdentityFeatures) @@ -134,7 +139,9 @@ def test_collect_labeled_features_multiclass_requires_behavior_names( labels.get_track_labels("0", MULTICLASS_NONE_BEHAVIOR).label_behavior(0, 0) pose = _MockPose(mask=np.array([1, 1], dtype=np.uint8)) monkeypatch.setattr("jabs.project.parallel_workers.open_pose_file", lambda *_: pose) - monkeypatch.setattr("jabs.project.parallel_workers.get_fps", lambda *_: 30) + monkeypatch.setattr( + "jabs.project.parallel_workers.get_fps_and_nframes", lambda *_: (30, pose.num_frames) + ) monkeypatch.setattr("jabs.project.parallel_workers._load_video_labels", lambda *_: labels) with pytest.raises(ValueError, match="behavior_names is required"): @@ -381,6 +388,20 @@ def test_video_manager_scan_results_frame_mismatch_raises(tmp_path): # --------------------------------------------------------------------------- +def test_warn_on_frame_count_mismatch_logs_warning(caplog): + """A video/pose frame-count mismatch emits a warning naming the video.""" + with caplog.at_level(logging.WARNING): + _warn_on_frame_count_mismatch("video1.avi", 100, 90) + assert any("video1.avi" in m and "does not match" in m for m in caplog.messages) + + +def test_warn_on_frame_count_mismatch_silent_when_equal(caplog): + """No warning is emitted when the frame counts match.""" + with caplog.at_level(logging.WARNING): + _warn_on_frame_count_mismatch("video1.avi", 100, 100) + assert not any("does not match" in m for m in caplog.messages) + + def test_feature_manager_with_scan_results_no_hdf5_open(tmp_path): """FeatureManager.__initialize_pose_data skips HDF5 opens when scan_results provided.""" from jabs.project.feature_manager import FeatureManager diff --git a/tests/project/test_project.py b/tests/project/test_project.py index 687dc6ed..d1e2d0ef 100644 --- a/tests/project/test_project.py +++ b/tests/project/test_project.py @@ -227,9 +227,24 @@ def test_no_saved_video_labels(project_with_data): def test_bad_video_file(project_with_data): - """test loading a video file that doesn't exist raises ValueError""" + """Opt-in up-front video check raises IOError when a video can't be opened.""" with pytest.raises(IOError), hide_stderr(): - _ = Project(Path("test_project_with_data"), enable_session_tracker=False) + _ = Project( + Path("test_project_with_data"), + enable_video_check=True, + enable_session_tracker=False, + ) + + +def test_load_defers_video_frame_check_by_default(project_with_data): + """By default the video frame-count check is deferred. + + The fixture's ``.avi`` files are empty stubs that cannot be opened, so the + opt-in path raises IOError (see ``test_bad_video_file``). With the default + (deferred) behavior no video file is opened, so the project loads cleanly. + """ + project = Project(Path("test_project_with_data"), enable_session_tracker=False) + assert set(project.video_manager.videos) == {"test_file_1.avi", "test_file_2.avi"} def test_min_pose_version(project_with_data): diff --git a/tests/ui/test_central_widget.py b/tests/ui/test_central_widget.py index f96c3e4d..bf4d67bc 100644 --- a/tests/ui/test_central_widget.py +++ b/tests/ui/test_central_widget.py @@ -75,3 +75,17 @@ def test_included_project_bout_totals_handles_none_counts(): """No counts yet -> zero totals (no crash).""" stub = _bout_stub_widget(None, set()) assert CentralWidget._included_project_bout_totals(stub) == (0, 0) + + +def test_frame_count_mismatch_message_none_when_equal(): + """Matching video/pose frame counts produce no warning message.""" + assert CentralWidget._frame_count_mismatch_message("v.avi", 100, 100) is None + + +def test_frame_count_mismatch_message_reports_counts(): + """A mismatch yields a message naming the video and both frame counts.""" + msg = CentralWidget._frame_count_mismatch_message("v.avi", 100, 90) + assert msg is not None + assert "v.avi" in msg + assert "100" in msg + assert "90" in msg From 3fdbbafc9634ddf477e01dff78e8c2dadb3843e9 Mon Sep 17 00:00:00 2001 From: Glen Beane <356266+gbeane@users.noreply.github.com> Date: Tue, 14 Jul 2026 16:49:16 -0400 Subject: [PATCH 2/6] Address review comments: defer mismatch dialog, release VideoCapture (KLAUS-505) --- src/jabs/ui/main_window/central_widget.py | 12 ++++++++++-- src/jabs/video_reader/utilities.py | 13 +++++++------ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/jabs/ui/main_window/central_widget.py b/src/jabs/ui/main_window/central_widget.py index cee855cc..9168fca0 100644 --- a/src/jabs/ui/main_window/central_widget.py +++ b/src/jabs/ui/main_window/central_widget.py @@ -427,12 +427,20 @@ def load_video(self, path: Path) -> None: # Frame-count check deferred from project load: warn (do not block) # if the video and its pose file disagree on frame count, since labels - # and features are pose-indexed but the player shows video frames. + # and features are pose-indexed but the player shows video frames. The + # dialog is scheduled on the next event-loop turn so it does not pause + # the rest of load_video; the video finishes loading, then the warning + # appears over it. mismatch_message = self._frame_count_mismatch_message( path.name, self._player_widget.num_frames, self._pose_est.num_frames ) if mismatch_message is not None: - MessageDialog.warning(self, "Video / pose frame count mismatch", mismatch_message) + QtCore.QTimer.singleShot( + 0, + lambda: MessageDialog.warning( + self, "Video / pose frame count mismatch", mismatch_message + ), + ) # load saved predictions for this video ( diff --git a/src/jabs/video_reader/utilities.py b/src/jabs/video_reader/utilities.py index 37f6c469..bfaa5650 100644 --- a/src/jabs/video_reader/utilities.py +++ b/src/jabs/video_reader/utilities.py @@ -47,10 +47,11 @@ def get_fps_and_nframes(video_path: str) -> tuple[int, int]: OSError: if unable to open the specified video. """ stream = cv2.VideoCapture(video_path) - if not stream.isOpened(): - raise OSError(f"unable to open {video_path}") - - fps = round(stream.get(cv2.CAP_PROP_FPS)) - num_frames = int(stream.get(cv2.CAP_PROP_FRAME_COUNT)) - stream.release() + try: + if not stream.isOpened(): + raise OSError(f"unable to open {video_path}") + fps = round(stream.get(cv2.CAP_PROP_FPS)) + num_frames = int(stream.get(cv2.CAP_PROP_FRAME_COUNT)) + finally: + stream.release() return fps, num_frames From 3be3598ba6a736ee36d01b29dbd614c1d4cebae9 Mon Sep 17 00:00:00 2001 From: Glen Beane <356266+gbeane@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:21:13 -0400 Subject: [PATCH 3/6] jabs-init: keep full up-front video/pose frame-count validation (KLAUS-505) --- src/jabs/scripts/initialize_project.py | 7 ++++- tests/project/test_initialize_project.py | 36 ++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/jabs/scripts/initialize_project.py b/src/jabs/scripts/initialize_project.py index d3806e88..a55cb7e7 100755 --- a/src/jabs/scripts/initialize_project.py +++ b/src/jabs/scripts/initialize_project.py @@ -253,7 +253,12 @@ def run_initialize_project( videos = VideoManager.get_videos(project_dir) metadata = _load_metadata(metadata_path) - project = jabs.project.Project(project_dir, enable_session_tracker=False) + # jabs-init performs a full up-front validation of the project, including + # confirming each video and its pose file agree on frame count, so it fails + # fast on an inconsistent project before any features are generated. + project = jabs.project.Project( + project_dir, enable_video_check=True, enable_session_tracker=False + ) distance_unit = project.feature_manager.distance_unit _apply_project_metadata(project, metadata) diff --git a/tests/project/test_initialize_project.py b/tests/project/test_initialize_project.py index 8fe3e464..6d4d6abb 100644 --- a/tests/project/test_initialize_project.py +++ b/tests/project/test_initialize_project.py @@ -1,3 +1,4 @@ +import pytest from click.testing import CliRunner import jabs.scripts.initialize_project as initialize_project @@ -146,3 +147,38 @@ def test_jabs_init_click_rejects_invalid_process_count(tmp_path): result = runner.invoke(initialize_project.main, ["-p", "0", str(project_dir)]) assert result.exit_code == 2 + + +def test_run_initialize_project_enables_video_frame_check(tmp_path, monkeypatch): + """jabs-init builds the Project with the up-front video-frame check enabled. + + Phase 1 (KLAUS-505) made ``enable_video_check`` default to False, so this + guards that batch initialization still validates that video and pose files + agree on frame count. + """ + project_dir = tmp_path / "project" + project_dir.mkdir() + + captured: dict = {} + + class _StopAfterProject(Exception): + pass + + def fake_project(*args, **kwargs): + captured.update(kwargs) + raise _StopAfterProject + + monkeypatch.setattr("jabs.project.Project", fake_project) + + with pytest.raises(_StopAfterProject): + initialize_project.run_initialize_project( + force=False, + processes=1, + window_sizes=(), + force_pixel_distances=False, + metadata_path=None, + skip_feature_generation=True, + project_dir=project_dir, + ) + + assert captured.get("enable_video_check") is True From 5cd1969b6cea91752828f2c58824e146fc7713d5 Mon Sep 17 00:00:00 2001 From: Glen Beane <356266+gbeane@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:22:52 -0400 Subject: [PATCH 4/6] move return value to get rid of IDE warning --- src/jabs/scripts/initialize_project.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/jabs/scripts/initialize_project.py b/src/jabs/scripts/initialize_project.py index a55cb7e7..a6d4ec19 100755 --- a/src/jabs/scripts/initialize_project.py +++ b/src/jabs/scripts/initialize_project.py @@ -177,6 +177,7 @@ def _load_metadata(metadata_path: Path | None) -> dict | None: try: metadata = json.loads(metadata_path.read_text()) validate_metadata(metadata) + return metadata except json.JSONDecodeError as e: _exit_with_message(f"Error reading metadata file {metadata_path}: {e}") except OSError as e: @@ -184,8 +185,6 @@ def _load_metadata(metadata_path: Path | None) -> dict | None: except ValidationError as e: _exit_with_message(f"Metadata file {metadata_path} is not valid: {e.message}") - return metadata - def _apply_project_metadata(project: jabs.project.Project, metadata: dict | None) -> None: """Merge or replace project metadata when requested.""" From 74cc996002b05fd6224404cf06a651b6c56bd591 Mon Sep 17 00:00:00 2001 From: Glen Beane <356266+gbeane@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:25:55 -0400 Subject: [PATCH 5/6] jabs-init: annotate pool with multiprocessing.pool.Pool --- src/jabs/scripts/initialize_project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jabs/scripts/initialize_project.py b/src/jabs/scripts/initialize_project.py index a6d4ec19..f7785c64 100755 --- a/src/jabs/scripts/initialize_project.py +++ b/src/jabs/scripts/initialize_project.py @@ -9,7 +9,7 @@ import json import logging import os -from multiprocessing import Pool +from multiprocessing.pool import Pool from pathlib import Path import click From a9d98e01b116b487763ac14f4c470a26b59b2ae0 Mon Sep 17 00:00:00 2001 From: Glen Beane <356266+gbeane@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:32:56 -0400 Subject: [PATCH 6/6] jabs-init: rename loop var to avoid variable shadowing --- src/jabs/scripts/initialize_project.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/jabs/scripts/initialize_project.py b/src/jabs/scripts/initialize_project.py index f7785c64..f96d4730 100755 --- a/src/jabs/scripts/initialize_project.py +++ b/src/jabs/scripts/initialize_project.py @@ -287,8 +287,8 @@ def run_initialize_project( # pose file and video frame number mismatch, etc def validation_job_producer(): - for video in videos: - yield {"video": video, "project_dir": project_dir} + for video_name in videos: + yield {"video": video_name, "project_dir": project_dir} # do work in parallel (not really necessary for this test, but we already # have the worker pool for generating features)