-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Fix frame extraction failure with pathlib video selections #3449
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
ba536eb
819fecc
012bf98
23b601e
06dba4a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,28 @@ | |
| from pathlib import Path | ||
|
|
||
|
|
||
| def normalize_video_path(video: str | Path) -> Path: | ||
| return Path(video) | ||
|
|
||
|
|
||
| def _filter_config_videos( | ||
| configured_videos, | ||
| selected_videos, | ||
| ) -> list: | ||
|
Comment on lines
+19
to
+22
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would be helpful to add type hints. Also, let's add/adjust the type hints of the |
||
| """Return config video keys matching the selected video paths. | ||
|
|
||
| The original config keys are returned so they remain valid for subsequent | ||
| config dictionary lookups. | ||
| """ | ||
| configured_videos = list(configured_videos) | ||
|
|
||
| if selected_videos is None: | ||
| return configured_videos | ||
|
|
||
| selected = {normalize_video_path(video) for video in selected_videos} | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer normalizing |
||
| return [video for video in configured_videos if normalize_video_path(video) in selected] | ||
|
|
||
|
|
||
| def select_cropping_area(config: str | Path, videos=None): | ||
| """Interactively select the cropping area of all videos in the config. A user | ||
| interface pops up with a frame to select the cropping parameters. Use the left click | ||
|
|
@@ -234,10 +256,14 @@ def extract_frames( | |
| cfg = auxiliaryfunctions.read_config(config_file) | ||
| print("Config file read successfully.") | ||
|
|
||
| if videos_list is None: | ||
| videos = list(cfg.get("video_sets_original") or cfg["video_sets"]) | ||
| else: # filter video_list by the ones in the config file | ||
| videos = [v for v in cfg["video_sets"] if v in videos_list] | ||
| configured_videos = list(cfg.get("video_sets_original") or cfg["video_sets"]) | ||
| videos = _filter_config_videos(configured_videos, videos_list) | ||
|
|
||
| if videos_list is not None and not videos: | ||
| raise ValueError( | ||
| "None of the selected videos matched the videos in the project " | ||
| "configuration. Selected videos may use a different path representation." | ||
| ) | ||
|
|
||
| if mode == "manual": | ||
| from deeplabcut.gui.widgets import launch_napari | ||
|
|
@@ -407,7 +433,11 @@ def extract_frames( | |
| else: # NO! | ||
| has_failed.append(False) | ||
|
|
||
| if all(has_failed): | ||
| if not has_failed: | ||
| raise RuntimeError( | ||
| "No videos were processed. Check that the selected video paths match the entries in config.yaml" | ||
| ) | ||
| elif all(has_failed): | ||
| print("Frame extraction failed. Video files must be corrupted.") | ||
| return has_failed | ||
| elif any(has_failed): | ||
|
|
@@ -427,9 +457,14 @@ def extract_frames( | |
| config_file = Path(config) | ||
| cfg = auxiliaryfunctions.read_config(config_file) | ||
| print("Config file read successfully.") | ||
| videos = sorted(cfg["video_sets"].keys()) | ||
| if videos_list is not None: # filter video_list by the ones in the config file | ||
| videos = [v for v in videos if v in videos_list] | ||
|
|
||
| videos = _filter_config_videos(sorted(cfg["video_sets"]), videos_list) | ||
| if videos_list is not None and not videos: | ||
| raise ValueError( | ||
| "None of the selected videos matched the videos in the project " | ||
| "configuration. Selected videos may use a different path representation." | ||
| ) | ||
|
|
||
| project_path = Path(config).parents[0] | ||
| labels_path = project_path / "labeled-data" | ||
| try: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| import os | ||
| from pathlib import Path | ||
|
|
||
| import numpy as np | ||
| import pytest | ||
| from skimage import io | ||
|
|
||
| from deeplabcut.generate_training_dataset.frame_extraction import _filter_config_videos, extract_frames | ||
| from deeplabcut.utils import auxfun_videos, auxiliaryfunctions, frameselectiontools | ||
|
|
||
|
|
||
| def test_extract_frames_accepts_path_videos_list(tmp_path, monkeypatch): | ||
| video = tmp_path / "videos" / "video.mp4" | ||
| video.parent.mkdir() | ||
| video.touch() | ||
|
|
||
| cfg = { | ||
| "video_sets": { | ||
| str(video): {"crop": "0, 100, 0, 100"}, | ||
| }, | ||
| "numframes2pick": 1, | ||
| "start": 0.0, | ||
| "stop": 1.0, | ||
| } | ||
|
|
||
| monkeypatch.setattr( | ||
| auxiliaryfunctions, | ||
| "read_config", | ||
| lambda _: cfg, | ||
| ) | ||
|
|
||
| processed = [] | ||
|
|
||
| class FakeVideoWriter: | ||
| def __init__(self, path): | ||
| processed.append(path) | ||
|
|
||
| def __len__(self): | ||
| return 10 | ||
|
|
||
| def set_to_frame(self, index): | ||
| pass | ||
|
|
||
| def read_frame(self, crop=True): | ||
| return np.zeros((100, 100, 3), dtype=np.uint8) | ||
|
|
||
| def close(self): | ||
| pass | ||
|
|
||
| monkeypatch.setattr( | ||
| auxfun_videos, | ||
| "VideoWriter", | ||
| FakeVideoWriter, | ||
| ) | ||
|
|
||
| # Isolate path filtering from frame-selection behavior. | ||
| monkeypatch.setattr( | ||
| frameselectiontools, | ||
| "UniformFramescv2", | ||
| lambda *args, **kwargs: [0], | ||
| ) | ||
|
|
||
| # Avoid writing an actual PNG. | ||
| monkeypatch.setattr(io, "imsave", lambda *args, **kwargs: None) | ||
|
|
||
| result = extract_frames( | ||
| tmp_path / "config.yaml", | ||
| mode="automatic", | ||
| algo="uniform", | ||
| videos_list=[video], | ||
| userfeedback=False, | ||
| ) | ||
|
|
||
| assert processed == [str(video)] | ||
| assert result == [False] | ||
|
|
||
|
|
||
| class TestFilterConfigVideos: | ||
| def test_filter_config_videos_matches_path_to_string(self): | ||
| configured = [r"C:\project\videos\video.mp4"] | ||
| selected = [Path(r"C:\project\videos\video.mp4")] | ||
|
|
||
| result = _filter_config_videos(configured, selected) | ||
|
|
||
| assert result == configured | ||
| assert isinstance(result[0], str) | ||
|
|
||
| def test_filter_config_videos_matches_string_to_path(self): | ||
| configured = [Path(r"C:\project\videos\video.mp4")] | ||
| selected = [r"C:\project\videos\video.mp4"] | ||
|
|
||
| result = _filter_config_videos(configured, selected) | ||
|
|
||
| assert result == configured | ||
| assert isinstance(result[0], Path) | ||
|
|
||
| def test_filter_config_videos_preserves_original_config_key(self): | ||
| configured = [r"C:\project\videos\video.mp4"] | ||
| selected = [Path(r"C:\project\videos\video.mp4")] | ||
|
|
||
| result = _filter_config_videos(configured, selected) | ||
|
|
||
| assert result[0] is configured[0] | ||
|
|
||
| def test_filter_config_videos_returns_all_when_selection_is_none(self): | ||
| configured = ["video-a.mp4", "video-b.mp4"] | ||
|
|
||
| assert _filter_config_videos(configured, None) == configured | ||
|
|
||
| def test_filter_config_videos_returns_empty_for_nonmatching_selection(self): | ||
| configured = ["video-a.mp4"] | ||
| selected = [Path("video-b.mp4")] | ||
|
|
||
| assert _filter_config_videos(configured, selected) == [] | ||
|
|
||
| @pytest.mark.skipif(os.name != "nt", reason="Windows path semantics") | ||
| def test_filter_config_videos_is_case_insensitive_on_windows(self): | ||
| configured = [r"C:\Project\Videos\VIDEO.MP4"] | ||
| selected = [Path(r"c:\project\videos\video.mp4")] | ||
|
|
||
| assert _filter_config_videos(configured, selected) == configured |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me, this helper feels like an unnecessary abstraction of inline
Pathwith is perfectly readable (even shorter).