Refactor VideoProcessor class (#3358)
* Sort and stringify video list Convert entries in self.files to strings and sort them before use. This ensures a deterministic, predictable ordering of videos (e.g., when files are Path objects or come from an unordered collection) and avoids nondeterministic processing order. * Convert video paths to strings for vp calls Cast video and output path arguments to str before passing them to vp to ensure pathlib.Path or other path-like objects don't cause errors. Updated calls in proc_video, create_video, create_video_with_all_detections (and related CreateVideoSlow usage) to use str(video) and str(outputname/output_path) for improved compatibility with Path inputs. * Revert "Sort and stringify video list" This reverts commit 3e47c96de1325bc4dda58e00a729c81ff3bfb097. * Add tests for VideoProcessorCV Introduce tests/utils/test_video_processor.py which adds comprehensive unit tests for deeplabcut.utils.video_processor.VideoProcessorCV. Includes a helper _make_test_video to produce temporary OpenCV videos and tests for metadata reading, frame loading and EOF behavior, nframes cap and -1 handling, fps override, video writing with default and explicit dimensions, idempotent close, context-manager cleanup, and invalid-input error handling. * Turn VideoProcessor into ABC with abstract methods Make VideoProcessor an abstract base class by importing ABC and abstractmethod and inheriting from ABC. Replace placeholder pass methods with @abstractmethod-decorated methods that raise NotImplementedError for get_video, get_info, create_video, _read_frame, save_frame, and close. Expose internal attributes via @property for height, width, fps, counter, and frame_count. These changes formalize the interface and enforce implementation in subclasses. * Use clip properties for height/width/fps Replace calls to clip.height(), clip.width(), and clip.fps() with attribute access (clip.height, clip.width, clip.fps) in make_labeled_video.py. This updates CreateVideo, CreateVideoSlow, proc_video, and create_video_with_all_detections to match the video object API and avoid method-call errors when these values are provided as properties. * Add properties and docs to video processor Introduce fname/sname properties on VideoProcessor (with _fname/_sname initialization) to ensure string coercion and clearer attribute access. Expand VideoProcessorCV with comprehensive docstrings, clarify RGB<->BGR conversion behavior, add safety checks when reading frames, and make close() null out released OpenCV handles. Update tests: remove the context-manager close test and mark the invalid-input-video test as xfail for backwards compatibility. * Avoid str() casts when calling vp Remove redundant str() conversions when creating vp clips in make_labeled_video.py now that the property coerces. Pass video and outputname/path variables directly (e.g. vp(fname=video), vp(video), vp(fname=video, sname=outputname)) to improve compatibility with pathlib.Path and avoid potential type issues. Updates applied in proc_video, create_video, and create_video_with_all_detections. * Initialize video attrs and switch to logging In deeplabcut/utils/video_processor.py initialize commonly used instance attributes (FPS, vid, svid, sh, sw) in the constructor to avoid missing-attribute errors. Replace a stray print() in the exception handler with a module logger and add logging import/logger setup. Also add a brief note about FPS handling to clarify behavior when overriding video FPS. * Change logging to exception * Test VideoProcessorCV accepts Path and stores str Add a unit test that creates a temporary video file and passes a pathlib.Path to VideoProcessorCV. The test asserts that no exception is raised, that clip.fname is stored as a string, and ensures the clip is closed. This prevents regressions where Path objects might not be handled or converted to str by the processor. * Add type annotations to VideoProcessor.__init__ Introduce Python type hints to the VideoProcessor.__init__ signature: fname: str, sname: str, nframes: int, fps: float, codec: str, sh: int, sw: int. This improves code clarity and static typing. * Address review comments Annotate fps, sh, and sw with None-aware union types and change sh/sw defaults from empty strings to None. Update logic to check for None rather than empty strings, and make fname/sname setters convert None or empty input to an empty string to avoid storing literal 'None'. Also guard frame writes by ensuring svid is set before calling write (prevents attempts to write when output video is not initialized). These changes improve type correctness and robustness when optional values are omitted. * Refactor VideoProcessor API to video_fps/h/w Introduce a clearer VideoProcessor API: replace legacy FPS/width/height/frame_count attributes with video_fps, h, w, and nframes, add setters/getters and legacy compatibility methods (fps(), height(), width(), frame_count(), FPS). Improve docstrings and typing (Literal import), tighten handling of empty output sizes, and ensure video writers use video_fps. Update VideoProcessorCV to set/read video_fps from OpenCV and to warn when writing without an open writer; avoid writing None frames. Update make_labeled_video calls to use clip.video_fps and clip.h/clip.w, and adjust tests accordingly to reflect the new attribute names. * Mark legacy VideoProcessor accessors as deprecated Add deprecation wrappers to legacy VideoProcessor methods and properties (counter, height, width, fps, FPS getter/setter) so callers are warned to migrate. Each decorator provides a replacement hint (i, h, w, video_fps) and a since="3.1" note. Also import the deprecated helper from deeplabcut.utils.deprecation to support these annotations. This preserves backward compatibility while guiding users toward the new attributes. * Disable deprecated decorator on FPS property * Comment on deprecations * Fix incorrect property usage * Rename VideoProcessor.video_fps to fps Replace the legacy video_fps attribute with a slimmer fps property across the codebase. Updated VideoProcessor to expose fps (getter/setter) and changed VideoProcessorCV to initialize and use fps for reading and writing; callers in make_labeled_video now reference clip.fps. Removed several legacy/deprecated compatibility methods related to old accessors (counter, height, width, FPS, frame_count, etc.). Tests were adjusted to assert clip.fps instead of clip.video_fps. This consolidates FPS handling and simplifies the VideoProcessor API. * Rename VideoProcessor h/w to height/width Replace ambiguous 'h' and 'w' properties with explicit 'height' and 'width' on VideoProcessor and its subclasses. Update all internal references (e.g. VideoProcessorCV initialization and make_labeled_video bbox/cropping logic) and adjust tests to assert clip.width and clip.height. Improves readability and consistency of video dimension access across the codebase. * Fix missing renaming of properties in make_labeled_video --------- Co-authored-by: Mackenzie Mathis <mathis@rowland.harvard.edu>
C
Cyril Achard committed
95c630b1e1f94dab2b7861b66a4e716b13958d0e
Parent: 6e4465e
Committed by GitHub <noreply@github.com>
on 6/25/2026, 7:45:49 PM