diff --git a/README.md b/README.md index f7929f31..f5612762 100644 --- a/README.md +++ b/README.md @@ -303,6 +303,16 @@ mapillary_tools sample_video MY_VIDEO_DIR MY_SAMPLES --video_sample_distance 5 mapillary_tools process MY_SAMPLES ``` +If a video can not be sampled, for example because it has no GPS track to locate the frames along, +mapillary_tools still samples the other videos and processes their sample images, then exits with an error. +To skip such videos instead, specify `--skip_sample_errors`. It covers sampling only: errors from processing +the sample images are skipped with `--skip_process_errors`. + +```sh +# Sample and process the videos that can be sampled, and skip the others +mapillary_tools video_process MY_VIDEO_DIR MY_SAMPLES --skip_sample_errors +``` + To process and upload the sample images consecutively, run: ```sh diff --git a/mapillary_tools/commands/sample_video.py b/mapillary_tools/commands/sample_video.py index 2573f800..c7dc3e92 100644 --- a/mapillary_tools/commands/sample_video.py +++ b/mapillary_tools/commands/sample_video.py @@ -61,7 +61,7 @@ def add_basic_arguments(self, parser: argparse.ArgumentParser): ) group.add_argument( "--skip_sample_errors", - help="Skip errors from the video sampling.", + help="Skip the videos that fail to sample. Otherwise the command fails after sampling the other videos.", action="store_true", default=False, required=False, diff --git a/mapillary_tools/commands/video_process.py b/mapillary_tools/commands/video_process.py index 83010873..d127b9b9 100644 --- a/mapillary_tools/commands/video_process.py +++ b/mapillary_tools/commands/video_process.py @@ -3,8 +3,11 @@ # This source code is licensed under the BSD license found in the # LICENSE file in the root directory of this source tree. +from __future__ import annotations + import logging +from .. import exceptions from ..types import FileType from .process import Command as ProcessCommand from .sample_video import Command as SampleCommand @@ -22,7 +25,14 @@ def add_basic_arguments(self, parser): ProcessCommand().add_basic_arguments(parser) def run(self, args: dict): - SampleCommand().run(args) + sample_error: exceptions.MapillaryVideoSampleError | None = None + try: + SampleCommand().run(args) + except exceptions.MapillaryVideoSampleError as ex: + # Like the process command, which writes out the files it did + # process before failing: process the frames of the videos that + # did sample rather than leave them behind, then fail + sample_error = ex option = "filetypes" if args[option] != {FileType.IMAGE}: @@ -34,3 +44,6 @@ def run(self, args: dict): args[option] = {FileType.IMAGE} ProcessCommand().run(args) + + if sample_error is not None: + raise sample_error diff --git a/mapillary_tools/commands/video_process_and_upload.py b/mapillary_tools/commands/video_process_and_upload.py index 5831fef3..3f3b145f 100644 --- a/mapillary_tools/commands/video_process_and_upload.py +++ b/mapillary_tools/commands/video_process_and_upload.py @@ -4,12 +4,17 @@ # LICENSE file in the root directory of this source tree. import inspect +import logging +from .. import exceptions from ..authenticate import fetch_user_items from .upload import Command as UploadCommand from .video_process import Command as VideoProcessCommand +LOG = logging.getLogger(__name__) + + class Command: name = "video_process_and_upload" help = "sample video into images, process the images and upload to Mapillary" @@ -33,5 +38,17 @@ def run(self, vars_args: dict): } ) - VideoProcessCommand().run(vars_args) + try: + VideoProcessCommand().run(vars_args) + except exceptions.MapillaryVideoSampleError: + # Upload only some of the videos when asked to, as process_and_upload + # does with the files it fails to process. But say so: the last thing + # logged before this is the process summary of the sample images that + # were ready to upload + LOG.error( + "Nothing was uploaded, because some videos failed to sample. " + "To upload the sample images of the others, specify --skip_sample_errors" + ) + raise + UploadCommand().run(vars_args) diff --git a/mapillary_tools/exceptions.py b/mapillary_tools/exceptions.py index 5474fd1b..1c97f6dc 100644 --- a/mapillary_tools/exceptions.py +++ b/mapillary_tools/exceptions.py @@ -40,6 +40,12 @@ class MapillaryVideoError(MapillaryUserError): exit_code = 7 +class MapillaryVideoSampleError(MapillaryVideoError): + """ + Raised after sampling every video if any of them failed + """ + + class MapillaryFFmpegNotFoundError(MapillaryUserError): exit_code = 8 diff --git a/mapillary_tools/sample_video.py b/mapillary_tools/sample_video.py index 1e3e7764..b0d32652 100644 --- a/mapillary_tools/sample_video.py +++ b/mapillary_tools/sample_video.py @@ -27,8 +27,9 @@ def _normalize_path( video_import_path: Path, skip_subfolders: bool ) -> tuple[Path, list[Path]]: if video_import_path.is_dir(): - video_list = utils.find_videos( - [video_import_path], skip_subfolders=skip_subfolders + # Sorted so that videos are sampled and reported in a stable order + video_list = sorted( + utils.find_videos([video_import_path], skip_subfolders=skip_subfolders) ) video_dir = video_import_path.resolve() LOG.debug("Found %d videos in %s", len(video_list), video_dir) @@ -92,6 +93,15 @@ def sample_video( elif sample_dir.is_file(): os.remove(sample_dir) + if video_list: + # Created even if no video samples, so that the process command that + # follows in video_process finds an empty import path rather than none + os.makedirs(import_path, exist_ok=True) + + num_sampled = 0 + num_already_sampled = 0 + failures: list[tuple[Path, Exception]] = [] + for video_path in video_list: # need to resolve video_path because video_dir might be absolute sample_dir = Path(import_path).joinpath( @@ -103,6 +113,7 @@ def sample_video( video_path.name, sample_dir, ) + num_already_sampled += 1 continue try: @@ -129,15 +140,56 @@ def sample_video( raise exceptions.MapillaryFFmpegNotFoundError(str(ex)) from ex except Exception as ex: - if skip_sample_errors: - LOG.warning( - "Skipping the error sampling %s: %s", - video_path, - str(ex), - exc_info=LOG.isEnabledFor(logging.DEBUG), - ) - else: - raise + # Carry on with the other videos, as the process command does with + # the other files, and fail after the summary. Log the reason now, + # because unlike process, sampling writes no description file + (LOG.warning if skip_sample_errors else LOG.error)( + "Failed to sample %s: %s", + video_path, + str(ex), + exc_info=LOG.isEnabledFor(logging.DEBUG), + ) + failures.append((video_path, ex)) + + else: + num_sampled += 1 + + _show_stats( + len(video_list), + num_sampled=num_sampled, + num_already_sampled=num_already_sampled, + failures=failures, + skip_sample_errors=skip_sample_errors, + ) + + +def _show_stats( + num_videos: int, + num_sampled: int, + num_already_sampled: int, + failures: T.Sequence[tuple[Path, Exception]], + skip_sample_errors: bool, +) -> None: + LOG.info("==> Sample summary") + LOG.info("%d videos found", num_videos) + if num_sampled: + LOG.info("\t%d sampled", num_sampled) + if num_already_sampled: + LOG.info("\t%d already sampled", num_already_sampled) + + failures_by_type: dict[type[Exception], int] = {} + for _, ex in failures: + failures_by_type[type(ex)] = failures_by_type.get(type(ex), 0) + 1 + + for error_type, count in failures_by_type.items(): + (LOG.warning if skip_sample_errors else LOG.error)( + "\t%d %s", count, error_type.__name__ + ) + + if failures and not skip_sample_errors: + raise exceptions.MapillaryVideoSampleError( + f"Failed to sample {len(failures)} videos. To skip these errors, specify --skip_sample_errors" + ) @contextmanager @@ -300,17 +352,26 @@ def _sample_single_video_by_distance( ) assert len(video_metadatas) == 1, "expect 1 video metadata" video_metadata = video_metadatas[0] + + # Distance sampling needs positions to decide which frames to cut, so + # failing to read them is a failed sample, not something to carry on past. + # Warning and returning left the caller with a success exit code, an empty + # (or missing) sample directory and nothing to upload. Raised as is, so + # that the sample summary counts it by its own type if isinstance(video_metadata, types.ErrorMetadata): - LOG.warning(str(video_metadata.error)) - return - assert video_metadata.points, "expect non-empty points" + raise video_metadata.error + + if not video_metadata.points: + raise exceptions.MapillaryVideoGPSNotFoundError("No GPS points found") + LOG.info("Found total %d GPS points", len(video_metadata.points)) # find the video stream with maximum resolution video_stream = probe.probe_video_with_max_resolution() if not video_stream: - LOG.warning("no video streams found from ffprobe") - return + raise exceptions.MapillaryVideoError( + f"No video streams found in {video_path} by ffprobe" + ) LOG.info("Extracting video samples") video_stream_idx = video_stream["index"] @@ -319,6 +380,11 @@ def _sample_single_video_by_distance( sample_points_by_frame_idx = _sample_video_stream_by_distance( video_metadata.points, video_track_parser, sample_distance ) + if not sample_points_by_frame_idx: + # A track that covers none of the video, e.g. a GPX from another trip + raise exceptions.MapillaryVideoError( + f"No video frames found within the GPS track from {video_metadata.points[0].time:.3f}s to {video_metadata.points[-1].time:.3f}s" + ) sorted_sample_indices = sorted(sample_points_by_frame_idx.keys()) with wip_dir_context(wip_sample_dir(sample_dir), sample_dir) as wip_dir: diff --git a/tests/integration/test_video_process.py b/tests/integration/test_video_process.py index a2a2a05e..055ad9be 100644 --- a/tests/integration/test_video_process.py +++ b/tests/integration/test_video_process.py @@ -4,6 +4,7 @@ # LICENSE file in the root directory of this source tree. import functools +import json import subprocess import tempfile from pathlib import Path @@ -17,7 +18,10 @@ pytest_skip_if_not_ffmpeg_installed, run_command, run_process_for_descs, + setup_config, setup_data, + setup_upload, + USERNAME, ) @@ -31,13 +35,24 @@ def test_sample_video_relpath(): pytest_skip_if_not_ffmpeg_installed() + # hero8.mp4's embedded GPS is filtered out entirely as noise, so it cannot + # be sampled by distance. This test is about relative paths, not about that, + # hence --skip_sample_errors. with tempfile.TemporaryDirectory() as dir: - run_sample_video(["--rerun", "tests/data/gopro_data/hero8.mp4", str(dir)]) + run_sample_video( + [ + "--rerun", + "--skip_sample_errors", + "tests/data/gopro_data/hero8.mp4", + str(dir), + ] + ) with tempfile.TemporaryDirectory() as dir: run_sample_video( [ "--rerun", + "--skip_sample_errors", "--video_start_time", "2021_10_10_10_10_10_123", "tests/data", @@ -46,6 +61,125 @@ def test_sample_video_relpath(): ) +def test_sample_video_by_distance_without_usable_gps(setup_data: py.path.local): + """ + A video whose GPS cannot be read is not silently sampled into nothing. + + hero8.mp4 has 32 embedded GPS points and remove_noisy_points() drops all of + them, so distance sampling has no positions to choose frames with. It used + to log a warning, write no frames and still exit 0, which left the caller a + success code and an empty import directory. + """ + pytest_skip_if_not_ffmpeg_installed() + + video_path = setup_data.join("gopro_data").join("hero8.mp4") + + with tempfile.TemporaryDirectory() as dir: + with pytest.raises(subprocess.CalledProcessError) as ex: + run_sample_video(["--video_sample_distance=6", "--rerun", video_path, dir]) + assert 7 == ex.value.returncode, ex.value.stderr + assert not list(Path(dir).iterdir()) + + with tempfile.TemporaryDirectory() as dir: + run_sample_video( + [ + "--video_sample_distance=6", + "--skip_sample_errors", + "--rerun", + video_path, + dir, + ] + ) + + +def test_video_process_processes_the_videos_that_sampled(setup_data: py.path.local): + """ + hero8.mp4's GPS is all noise, so it cannot be sampled by distance. That + fails the command, but only after max-360mode.mp4 beside it is sampled and + its samples processed, rather than leaving them behind unprocessed. + """ + pytest_skip_if_not_ffmpeg_installed() + + video_dir = setup_data.join("gopro_data") + desc_path = Path(setup_data, "descs.json") + + with pytest.raises(subprocess.CalledProcessError) as ex: + run_command( + [ + "--video_sample_distance=6", + *["--desc_path", str(desc_path)], + str(video_dir), + str(video_dir.join("my_samples")), + ], + "video_process", + capture_output=True, + text=True, + ) + assert 7 == ex.value.returncode, ex.value.stderr + assert "Failed to sample 1 videos" in ex.value.stderr + assert "--skip_sample_errors" in ex.value.stderr + + descs = json.loads(desc_path.read_text()) + assert descs + for desc in descs: + assert "error" not in desc, desc + assert Path(desc["filename"]).parent.name == "max-360mode.mp4" + + +def test_video_process_skips_the_only_video(setup_data: py.path.local): + """ + Skipping every video leaves nothing to process, which is not an error: + it used to fail with "Import file or directory not found" + """ + pytest_skip_if_not_ffmpeg_installed() + + descs = run_video_process_for_descs( + [ + "--video_sample_distance=6", + "--skip_sample_errors", + str(setup_data.join("gopro_data").join("hero8.mp4")), + ] + ) + assert [] == descs + + +def test_video_process_and_upload_uploads_nothing_if_a_video_fails( + setup_data: py.path.local, setup_config, setup_upload +): + """ + hero8.mp4 cannot be sampled by distance. The command fails without + uploading max-360mode.mp4 beside it either, and says so, because the last + thing it logs before failing is that its sample images are ready. + """ + pytest_skip_if_not_ffmpeg_installed() + + video_dir = setup_data.join("gopro_data") + upload_args = ["--dry_run", *["--user_name", USERNAME], "--video_sample_distance=6"] + + with pytest.raises(subprocess.CalledProcessError) as ex: + run_command( + [*upload_args, str(video_dir), str(setup_data.join("samples"))], + command="video_process_and_upload", + capture_output=True, + text=True, + ) + assert 7 == ex.value.returncode, ex.value.stderr + assert "Nothing was uploaded" in ex.value.stderr + assert "--skip_sample_errors" in ex.value.stderr + assert not setup_upload.listdir() + + run_command( + [ + *upload_args, + "--skip_sample_errors", + str(video_dir), + str(setup_data.join("samples_skipped")), + ], + command="video_process_and_upload", + ) + assert setup_upload.listdir() + + def test_sample_video_without_video_time(setup_data: py.path.local): pytest_skip_if_not_ffmpeg_installed() @@ -143,6 +277,9 @@ def test_video_process_sample_with_multiple_distances(setup_data: py.path.local) [ "--video_sample_distance", str(distance), + # gopro_data also holds hero8.mp4, whose GPS is all noise and so + # cannot be distance-sampled; this test is about max-360mode.mp4 + "--skip_sample_errors", "--rerun", str(video_dir), str(video_dir.join("my_samples")), @@ -167,6 +304,8 @@ def test_video_process_sample_with_distance(setup_data: py.path.local): descs = run_video_process_for_descs( [ *options, + # see test_video_process_sample_with_multiple_distances + "--skip_sample_errors", str(video_dir), str(video_dir.join("my_samples")), ] diff --git a/tests/unit/test_sample_video.py b/tests/unit/test_sample_video.py index 0743eeb4..9f1b9299 100644 --- a/tests/unit/test_sample_video.py +++ b/tests/unit/test_sample_video.py @@ -7,6 +7,7 @@ import datetime import json +import logging import os import shutil import typing as T @@ -22,9 +23,10 @@ geo, sample_video, ) +from mapillary_tools.geotag import geotag_videos_from_video from mapillary_tools.mp4 import mp4_sample_parser from mapillary_tools.serializer import description -from mapillary_tools.types import FileType, VideoMetadata +from mapillary_tools.types import describe_error_metadata, FileType, VideoMetadata _PWD = Path(os.path.dirname(os.path.abspath(__file__))) @@ -334,6 +336,129 @@ def test_empty_samples(self) -> None: assert len(result) == 0 +# --------------------------------------------------------------------------- +# A failed distance sample must not look like a successful one +# --------------------------------------------------------------------------- + + +class TestDistanceSamplingFailsLoudly: + """ + Distance sampling used to warn and return when it could not read the GPS, + which left the caller a success exit code, no sample directory and nothing + to upload -- `video_process` went on to geotag zero files and printed an + empty summary. Reported for a GoPro whose embedded GPS is all noise, where + an attached GPX made no difference. + """ + + VIDEO = _PWD.joinpath("data/mock_sample_video/videos/hello.mp4") + + @pytest.fixture + def unreadable_gps(self, monkeypatch): + """Every GPS read fails the way a fully filtered noisy track does.""" + error = exceptions.MapillaryGPSNoiseError("GPS is too noisy") + monkeypatch.setattr( + geotag_videos_from_video.GeotagVideosFromVideo, + "to_description", + lambda _self, paths: [ + describe_error_metadata( + error, filename=paths[0], filetype=FileType.GOPRO + ) + ], + ) + return error + + def _sample(self, tmpdir: py.path.local, **kwargs): + return sample_video.sample_video( + self.VIDEO, Path(tmpdir), video_sample_distance=2, rerun=True, **kwargs + ) + + def test_it_raises_instead_of_returning( + self, tmpdir, setup_mock, unreadable_gps, caplog + ): + with pytest.raises(exceptions.MapillaryVideoSampleError): + self._sample(tmpdir) + + # the underlying reason has to reach the user, and is logged per video + assert "GPS is too noisy" in caplog.text + + def test_it_names_the_flag_that_suppresses_it( + self, tmpdir, setup_mock, unreadable_gps + ): + """ + --skip_process_errors governs the later geotagging stage and does not + cover sampling, so the message has to name --skip_sample_errors or + users reach for the wrong flag. + """ + with pytest.raises(exceptions.MapillaryVideoError) as excinfo: + self._sample(tmpdir) + + assert "--skip_sample_errors" in str(excinfo.value) + + def test_the_exit_code_is_a_clean_one(self, tmpdir, setup_mock, unreadable_gps): + """Not a MapillaryUserError means a traceback instead of an exit code.""" + with pytest.raises(exceptions.MapillaryUserError) as excinfo: + self._sample(tmpdir) + + assert excinfo.value.exit_code == 7 + + def test_nothing_is_left_behind(self, tmpdir, setup_mock, unreadable_gps): + with pytest.raises(exceptions.MapillaryVideoError): + self._sample(tmpdir) + + assert not Path(tmpdir).joinpath(self.VIDEO.name).exists() + + def test_skip_sample_errors_still_tolerates_it( + self, tmpdir, setup_mock, unreadable_gps + ): + """The opt-out that several existing callers rely on.""" + self._sample(tmpdir, skip_sample_errors=True) + + def test_the_summary_counts_it_by_its_own_type( + self, tmpdir, setup_mock, unreadable_gps, caplog + ): + with pytest.raises(exceptions.MapillaryVideoSampleError): + self._sample(tmpdir) + + assert "1 MapillaryGPSNoiseError" in caplog.text + + def test_an_empty_track_is_also_an_error(self, tmpdir, setup_mock, monkeypatch): + """Previously an assert, so it vanished under `python -O`.""" + monkeypatch.setattr( + geotag_videos_from_video.GeotagVideosFromVideo, + "to_description", + lambda _self, paths: [ + VideoMetadata( + filename=paths[0], filesize=0, filetype=FileType.GOPRO, points=[] + ) + ], + ) + + with pytest.raises(exceptions.MapillaryVideoError): + self._sample(tmpdir) + + def test_a_missing_video_stream_is_also_an_error( + self, tmpdir, setup_mock, monkeypatch + ): + monkeypatch.setattr( + geotag_videos_from_video.GeotagVideosFromVideo, + "to_description", + lambda _self, paths: [ + VideoMetadata( + filename=paths[0], + filesize=0, + filetype=FileType.GOPRO, + points=_make_gps_points(3, time_step=1.0), + ) + ], + ) + monkeypatch.setattr( + ffmpeglib.Probe, "probe_video_with_max_resolution", lambda _self: None + ) + + with pytest.raises(exceptions.MapillaryVideoError): + self._sample(tmpdir) + + # --------------------------------------------------------------------------- # sample_video() parameter validation & rerun # --------------------------------------------------------------------------- @@ -608,3 +733,190 @@ def test_exif_lat_lon_written(self, tmp_path: Path) -> None: # First GPS point is at (40.0, -74.0) assert abs(lat - 40.0) < 0.01 assert abs(lon - (-74.0)) < 0.01 + + def test_no_frame_within_the_track(self, tmp_path: Path) -> None: + """ + A track that covers none of the video selects no frames, which is a + failed sample, not an empty sample directory and a success + """ + video_file = tmp_path / "test.mp4" + video_file.touch() + sample_dir = tmp_path / "output" / "test.mp4" + + mocks = self._setup_mocks(tmp_path, video_file) + # The frames are at 0s to 9s + for point in mocks["gps_points"]: + point.time += 100 + + with ( + mocks["patches"]["ffmpeg_cls"], + mocks["patches"]["geotag_cls"], + mocks["patches"]["moov_parse"], + ): + with pytest.raises( + exceptions.MapillaryVideoError, + match="No video frames found within the GPS track from 100.000s to 109.000s", + ): + sample_video._sample_single_video_by_distance( + video_file, sample_dir, sample_distance=0.0 + ) + + assert not sample_dir.exists() + + +class TestFailuresAreReportedTogether: + """ + A video that fails to sample does not stop the others. They are sampled, + the failures are summarized by type, and one error naming + --skip_sample_errors is raised at the end, as the process command does + with --skip_process_errors. Raising on the first failure left the samples + of the videos before it unprocessed, and which videos those were depended + on the order the filesystem listed them in. + """ + + VIDEO = _PWD.joinpath("data/mock_sample_video/videos/hello.mp4") + + @pytest.fixture + def video_dir(self, tmp_path: Path, monkeypatch) -> Path: + video_dir = tmp_path / "videos" + video_dir.mkdir() + videos = [video_dir / name for name in ["c.mp4", "a.mp4", "b.mp4"]] + for video in videos: + video.touch() + # Listed out of order, as a filesystem may list them + monkeypatch.setattr( + sample_video.utils, "find_videos", lambda *args, **kwargs: videos + ) + return video_dir + + @pytest.fixture + def attempted(self, monkeypatch) -> list[str]: + """ + Sample a video by creating its sample directory, unless it is named + a.mp4 (no GPS) or b.mp4 (unreadable) + """ + attempted: list[str] = [] + + def fake_sample(video_path: Path, sample_dir: Path, **kwargs) -> None: + attempted.append(video_path.name) + if video_path.name == "a.mp4": + raise exceptions.MapillaryVideoGPSNotFoundError("No GPS data found") + if video_path.name == "b.mp4": + raise exceptions.MapillaryVideoError("Unreadable") + sample_dir.mkdir(parents=True) + + monkeypatch.setattr( + sample_video, "_sample_single_video_by_distance", fake_sample + ) + return attempted + + def test_every_video_is_sampled_in_order( + self, tmp_path: Path, video_dir: Path, attempted: list[str] + ): + with pytest.raises(exceptions.MapillaryVideoSampleError): + sample_video.sample_video(video_dir, tmp_path / "output") + + assert attempted == ["a.mp4", "b.mp4", "c.mp4"] + assert (tmp_path / "output" / "c.mp4").is_dir() + + def test_one_error_names_the_flag( + self, tmp_path: Path, video_dir: Path, attempted: list[str] + ): + """ + --skip_process_errors governs the later geotagging stage and does not + cover sampling, so the message has to name --skip_sample_errors or + users reach for the wrong flag. + """ + with pytest.raises(exceptions.MapillaryVideoSampleError) as excinfo: + sample_video.sample_video(video_dir, tmp_path / "output") + + assert str(excinfo.value) == ( + "Failed to sample 2 videos. To skip these errors, specify --skip_sample_errors" + ) + + def test_the_summary_counts_failures_by_type( + self, tmp_path: Path, video_dir: Path, attempted: list[str], caplog + ): + caplog.set_level(logging.INFO) + with pytest.raises(exceptions.MapillaryVideoSampleError): + sample_video.sample_video(video_dir, tmp_path / "output") + + errors = [r.getMessage() for r in caplog.records if r.levelname == "ERROR"] + assert f"Failed to sample {video_dir / 'a.mp4'}: No GPS data found" in errors + assert f"Failed to sample {video_dir / 'b.mp4'}: Unreadable" in errors + assert "\t1 MapillaryVideoGPSNotFoundError" in errors + assert "\t1 MapillaryVideoError" in errors + assert "3 videos found" in caplog.text + assert "\t1 sampled" in caplog.text + + def test_skip_sample_errors_downgrades_them_to_warnings( + self, tmp_path: Path, video_dir: Path, attempted: list[str], caplog + ): + sample_video.sample_video( + video_dir, tmp_path / "output", skip_sample_errors=True + ) + + assert attempted == ["a.mp4", "b.mp4", "c.mp4"] + assert not [r for r in caplog.records if r.levelname == "ERROR"] + assert "\t1 MapillaryVideoGPSNotFoundError" in caplog.text + + def test_the_import_path_exists_even_if_nothing_samples( + self, tmp_path: Path, attempted: list[str] + ): + """ + video_process runs the process command on the import path next, which + fails on a missing one: skipping every video used to end in "Import + file or directory not found" + """ + video = tmp_path / "a.mp4" + video.touch() + + sample_video.sample_video(video, tmp_path / "output", skip_sample_errors=True) + + assert (tmp_path / "output").is_dir() + assert not list((tmp_path / "output").iterdir()) + + @pytest.mark.parametrize( + "kwargs", + [ + pytest.param({"video_sample_distance": 2}, id="by_distance"), + pytest.param( + {"video_sample_distance": -1, "video_sample_interval": 2}, + id="by_interval", + ), + ], + ) + def test_unreadable_start_time( + self, tmpdir, setup_mock, monkeypatch, kwargs, caplog + ): + monkeypatch.setattr( + ffmpeglib.Probe, "probe_video_start_time", lambda _self: None + ) + + with pytest.raises(exceptions.MapillaryVideoSampleError): + sample_video.sample_video(self.VIDEO, Path(tmpdir), rerun=True, **kwargs) + + assert "Unable to extract video start time" in caplog.text + + def test_a_missing_ffmpeg_is_still_fatal(self, tmpdir, setup_mock, monkeypatch): + """ + FFmpegNotFoundError fails every video alike, so it is raised at once, + --skip_sample_errors or not, and its message must not offer the flag + """ + + def boom(*args, **kwargs): + raise ffmpeglib.FFmpegNotFoundError("ffmpeg not found") + + monkeypatch.setattr(MOCK_FFMPEG, "extract_frames_by_interval", boom) + + with pytest.raises(exceptions.MapillaryFFmpegNotFoundError) as excinfo: + sample_video.sample_video( + self.VIDEO, + Path(tmpdir), + video_sample_distance=-1, + video_sample_interval=2, + rerun=True, + skip_sample_errors=True, + ) + + assert "--skip_sample_errors" not in str(excinfo.value)