From a5e49e4979d94a5275d5ea788db7633240d902e6 Mon Sep 17 00:00:00 2001
From: Caglar Pir <caglarp@meta.com>
Date: Mon, 21 Sep 2026 16:37:31 +0200
Subject: [PATCH 1/8] Fail the sample instead of silently producing no frames

Distance sampling warned and returned when it could not read a video's
GPS, so the command exited 0 having written nothing:

  WARNING - GPS is too noisy
  ==> Processing 0 files with source gpx...
  ==> Validating 0 metadatas...
  ==> Process summary

No error, no frames, no non-zero exit -- the sample directory is never
created and the geotag stage then runs over zero files. The user is told
the run succeeded and has nothing to upload. Reported for a GoPro MAX 2
whose embedded GPS is rejected as noise, where attaching a GPX made no
difference, but nothing about it is specific to noisy GPS: a video with
no GPS at all takes the same path, which is every "camera without
embedded GPS, bring your own GPX" workflow.

Distance sampling needs positions to decide which frames to cut, so
failing to read them is a failed sample. Raise MapillaryVideoError, the
same error the rest of this function already raises for an unreadable
start time or a frame count mismatch, and which exits 7 rather than
dumping a traceback. sample_video() already funnels sampling errors
through --skip_sample_errors, so callers who want to tolerate this keep
a supported way to do it and the default stops lying.

Two neighbouring silent paths get the same treatment: a missing video
stream, which also returned after a warning, and an empty point list,
which was an assert and so disappeared under `python -O`, leaving an
IndexError further down instead.

Note this changes batch behaviour. Sampling a directory containing one
unreadable video now aborts unless --skip_sample_errors is passed. That
matches what every other sampling error in this function already does,
and the alternative is continuing to hide the failure, but it is a
behaviour change for callers who relied on the skip.

Three integration tests covered directories containing hero8.mp4, whose
32 embedded points are all dropped by remove_noisy_points(); they were
passing while it contributed no frames at all. They now pass
--skip_sample_errors, which is what they always meant. A new test pins
the loud behaviour, mirroring test_sample_video_without_video_time.

This does not make --geotag_source reach distance sampling; that call
site still hardcodes GeotagVideosFromVideo() and is a separate fix. It
only stops the failure from being silent.
---
 mapillary_tools/sample_video.py         |  24 ++++--
 tests/integration/test_video_process.py |  49 ++++++++++-
 tests/unit/test_sample_video.py         | 104 +++++++++++++++++++++++-
 3 files changed, 170 insertions(+), 7 deletions(-)

diff --git a/mapillary_tools/sample_video.py b/mapillary_tools/sample_video.py
index 1e3e7764..b641e56d 100644
--- a/mapillary_tools/sample_video.py
+++ b/mapillary_tools/sample_video.py
@@ -300,17 +300,31 @@ 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. sample_video()
+    # already funnels these through --skip_sample_errors for callers who do
+    # want to tolerate them.
     if isinstance(video_metadata, types.ErrorMetadata):
-        LOG.warning(str(video_metadata.error))
-        return
-    assert video_metadata.points, "expect non-empty points"
+        raise exceptions.MapillaryVideoError(
+            f"Unable to sample {video_path} by distance: {video_metadata.error}"
+        ) from video_metadata.error
+
+    if not video_metadata.points:
+        raise exceptions.MapillaryVideoError(
+            f"Unable to sample {video_path} by distance: 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"]
diff --git a/tests/integration/test_video_process.py b/tests/integration/test_video_process.py
index a2a2a05e..aff97e68 100644
--- a/tests/integration/test_video_process.py
+++ b/tests/integration/test_video_process.py
@@ -31,13 +31,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 +57,37 @@ 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_sample_video_without_video_time(setup_data: py.path.local):
     pytest_skip_if_not_ffmpeg_installed()
 
@@ -143,6 +185,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 +212,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..0aa609ba 100644
--- a/tests/unit/test_sample_video.py
+++ b/tests/unit/test_sample_video.py
@@ -22,9 +22,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 +335,107 @@ 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):
+        with pytest.raises(exceptions.MapillaryVideoError) as excinfo:
+            self._sample(tmpdir)
+
+        # the underlying reason has to survive into the message the user sees
+        assert "GPS is too noisy" in str(excinfo.value)
+        assert excinfo.value.__cause__ is unreadable_gps
+
+    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_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
 # ---------------------------------------------------------------------------

From bb5873c99dc8a02afe7af4d550587c50443d261e Mon Sep 17 00:00:00 2001
From: Caglar Pir <caglarp@meta.com>
Date: Tue, 22 Sep 2026 15:54:14 +0200
Subject: [PATCH 2/8] Name --skip_sample_errors in the error it suppresses

The raise told users the sample failed but not how to get past it, and
the obvious guess is wrong: --skip_process_errors governs the later
geotagging stage and does not cover sampling, so reaching for it leaves
the run failing with the same message.

Append the hint, matching the existing wording in
process_geotag_properties.py. The message now reads:

  MapillaryVideoError: Unable to sample GS018205.360 by distance:
  GPS is too noisy. To skip these errors, specify --skip_sample_errors
---
 mapillary_tools/sample_video.py | 11 ++++++++---
 tests/unit/test_sample_video.py | 13 +++++++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/mapillary_tools/sample_video.py b/mapillary_tools/sample_video.py
index b641e56d..752fc36d 100644
--- a/mapillary_tools/sample_video.py
+++ b/mapillary_tools/sample_video.py
@@ -22,6 +22,11 @@
 
 LOG = logging.getLogger(__name__)
 
+# Sampling errors are suppressed by --skip_sample_errors, not by
+# --skip_process_errors, which governs the later geotagging stage. Say so in
+# the message: the two flags are easy to reach for the wrong one.
+_SKIP_HINT = "To skip these errors, specify --skip_sample_errors"
+
 
 def _normalize_path(
     video_import_path: Path, skip_subfolders: bool
@@ -309,12 +314,12 @@ def _sample_single_video_by_distance(
     # want to tolerate them.
     if isinstance(video_metadata, types.ErrorMetadata):
         raise exceptions.MapillaryVideoError(
-            f"Unable to sample {video_path} by distance: {video_metadata.error}"
+            f"Unable to sample {video_path} by distance: {video_metadata.error}. {_SKIP_HINT}"
         ) from video_metadata.error
 
     if not video_metadata.points:
         raise exceptions.MapillaryVideoError(
-            f"Unable to sample {video_path} by distance: no GPS points found"
+            f"Unable to sample {video_path} by distance: no GPS points found. {_SKIP_HINT}"
         )
 
     LOG.info("Found total %d GPS points", len(video_metadata.points))
@@ -323,7 +328,7 @@ def _sample_single_video_by_distance(
     video_stream = probe.probe_video_with_max_resolution()
     if not video_stream:
         raise exceptions.MapillaryVideoError(
-            f"No video streams found in {video_path} by ffprobe"
+            f"No video streams found in {video_path} by ffprobe. {_SKIP_HINT}"
         )
 
     LOG.info("Extracting video samples")
diff --git a/tests/unit/test_sample_video.py b/tests/unit/test_sample_video.py
index 0aa609ba..c7ad376d 100644
--- a/tests/unit/test_sample_video.py
+++ b/tests/unit/test_sample_video.py
@@ -379,6 +379,19 @@ def test_it_raises_instead_of_returning(self, tmpdir, setup_mock, unreadable_gps
         assert "GPS is too noisy" in str(excinfo.value)
         assert excinfo.value.__cause__ is unreadable_gps
 
+    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:

From 976c13562eef29f863f2181ea5ad0e026a1f0847 Mon Sep 17 00:00:00 2001
From: Caglar Pir <caglarp@meta.com>
Date: Tue, 22 Sep 2026 16:34:09 +0200
Subject: [PATCH 3/8] Name the flag in every error it suppresses, not just some

Four other raises sit inside the same try block that
--skip_sample_errors guards and said nothing about it:

  _sample_single_video_by_interval  unable to extract video start time
  _sample_single_video_by_distance  unable to extract video start time
  _sample_single_video_by_distance  expect N samples but extracted M
  _sample_single_video_by_distance  expect X to be Nth sample but got M

Half the errors naming the flag is worse than none naming it: a user who
lands on a silent one reaches for --skip_process_errors, which governs
the later geotagging stage and leaves the run failing with the same
message.

Rather than append the hint at seven call sites, where it can drift out
of sync again, construct these errors through _sampling_error(), so a
raise added later cannot forget it.

MapillaryFFmpegNotFoundError keeps its plain message. It is re-raised by
its own handler before the skip check, so --skip_sample_errors really
does not suppress it and it must not say otherwise. There is a test for
that, alongside ones covering both newly hinted start-time paths.
---
 mapillary_tools/sample_video.py | 32 ++++++++++-----
 tests/unit/test_sample_video.py | 71 +++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 11 deletions(-)

diff --git a/mapillary_tools/sample_video.py b/mapillary_tools/sample_video.py
index 752fc36d..b6548685 100644
--- a/mapillary_tools/sample_video.py
+++ b/mapillary_tools/sample_video.py
@@ -28,6 +28,18 @@
 _SKIP_HINT = "To skip these errors, specify --skip_sample_errors"
 
 
+def _sampling_error(message: str) -> exceptions.MapillaryVideoError:
+    """
+    Build an error for a failed sample, naming the flag that skips it.
+
+    Everything raised out of the per-video body of sample_video() is
+    suppressible by --skip_sample_errors, so every one of those messages should
+    say so. Going through here rather than appending the hint at each raise
+    keeps that true of raises added later.
+    """
+    return exceptions.MapillaryVideoError(f"{message}. {_SKIP_HINT}")
+
+
 def _normalize_path(
     video_import_path: Path, skip_subfolders: bool
 ) -> tuple[Path, list[Path]]:
@@ -198,7 +210,7 @@ def _sample_single_video_by_interval(
             ffmpeg.probe_format_and_streams(video_path)
         ).probe_video_start_time()
         if start_time is None:
-            raise exceptions.MapillaryVideoError(
+            raise _sampling_error(
                 f"Unable to extract video start time from {video_path}"
             )
 
@@ -294,7 +306,7 @@ def _sample_single_video_by_distance(
     if start_time is None:
         start_time = probe.probe_video_start_time()
         if start_time is None:
-            raise exceptions.MapillaryVideoError(
+            raise _sampling_error(
                 f"Unable to extract video start time from {video_path}"
             )
 
@@ -313,13 +325,13 @@ def _sample_single_video_by_distance(
     # already funnels these through --skip_sample_errors for callers who do
     # want to tolerate them.
     if isinstance(video_metadata, types.ErrorMetadata):
-        raise exceptions.MapillaryVideoError(
-            f"Unable to sample {video_path} by distance: {video_metadata.error}. {_SKIP_HINT}"
+        raise _sampling_error(
+            f"Unable to sample {video_path} by distance: {video_metadata.error}"
         ) from video_metadata.error
 
     if not video_metadata.points:
-        raise exceptions.MapillaryVideoError(
-            f"Unable to sample {video_path} by distance: no GPS points found. {_SKIP_HINT}"
+        raise _sampling_error(
+            f"Unable to sample {video_path} by distance: no GPS points found"
         )
 
     LOG.info("Found total %d GPS points", len(video_metadata.points))
@@ -327,9 +339,7 @@ def _sample_single_video_by_distance(
     # find the video stream with maximum resolution
     video_stream = probe.probe_video_with_max_resolution()
     if not video_stream:
-        raise exceptions.MapillaryVideoError(
-            f"No video streams found in {video_path} by ffprobe. {_SKIP_HINT}"
-        )
+        raise _sampling_error(f"No video streams found in {video_path} by ffprobe")
 
     LOG.info("Extracting video samples")
     video_stream_idx = video_stream["index"]
@@ -352,7 +362,7 @@ def _sample_single_video_by_distance(
             wip_dir, video_path, selected_stream_specifiers=[str(video_stream_idx)]
         )
         if len(frame_samples) != len(sorted_sample_indices):
-            raise exceptions.MapillaryVideoError(
+            raise _sampling_error(
                 f"Expect {len(sorted_sample_indices)} samples but extracted {len(frame_samples)} samples"
             )
         for idx, (frame_idx_1based, sample_paths) in enumerate(frame_samples):
@@ -360,7 +370,7 @@ def _sample_single_video_by_distance(
                 "Expect 1 sample path at {frame_idx_1based} but got {sample_paths}"
             )
             if idx + 1 != frame_idx_1based:
-                raise exceptions.MapillaryVideoError(
+                raise _sampling_error(
                     f"Expect {sample_paths[0]} to be {idx + 1}th sample but got {frame_idx_1based}"
                 )
 
diff --git a/tests/unit/test_sample_video.py b/tests/unit/test_sample_video.py
index c7ad376d..15dcc6f3 100644
--- a/tests/unit/test_sample_video.py
+++ b/tests/unit/test_sample_video.py
@@ -723,3 +723,74 @@ 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
+
+
+class TestEverySuppressibleErrorNamesTheFlag:
+    """
+    Every error raised out of the per-video body of sample_video() is
+    suppressed by --skip_sample_errors, so every one of those messages has to
+    name it. Half of them naming it is worse than none: a user who hits one of
+    the silent ones reaches for --skip_process_errors, which governs the later
+    geotagging stage and leaves the run failing with the same message.
+    """
+
+    VIDEO = _PWD.joinpath("data/mock_sample_video/videos/hello.mp4")
+
+    @pytest.fixture
+    def no_start_time(self, monkeypatch):
+        monkeypatch.setattr(
+            ffmpeglib.Probe, "probe_video_start_time", lambda _self: None
+        )
+
+    @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_names_the_flag(
+        self, tmpdir, setup_mock, no_start_time, kwargs
+    ):
+        with pytest.raises(exceptions.MapillaryVideoError) as excinfo:
+            sample_video.sample_video(self.VIDEO, Path(tmpdir), rerun=True, **kwargs)
+
+        assert "Unable to extract video start time" in str(excinfo.value)
+        assert "--skip_sample_errors" in str(excinfo.value)
+
+    def test_the_helper_appends_it_once(self):
+        error = sample_video._sampling_error("something went wrong")
+
+        assert str(error) == (
+            "something went wrong. To skip these errors, specify --skip_sample_errors"
+        )
+        assert isinstance(error, exceptions.MapillaryUserError)
+
+    def test_a_missing_ffmpeg_does_not_claim_to_be_skippable(
+        self, tmpdir, setup_mock, monkeypatch
+    ):
+        """
+        FFmpegNotFoundError is re-raised by its own handler before the skip
+        check, so --skip_sample_errors does not suppress it 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)

From 38ded6acab7cd6eea76752b821c46135e234510b Mon Sep 17 00:00:00 2001
From: Caglar Pir <caglarp@meta.com>
Date: Wed, 23 Sep 2026 17:00:10 +0200
Subject: [PATCH 4/8] Sample every video before failing, and report the
 failures together

Raising out of the first video that failed to sample made the result of
a directory depend on which video that was. find_videos() does not sort,
so the videos sampled before the failure depended on the order the
filesystem lists them in. The command exited 7 with their frames written
to the sample directory but never processed, because video_process never
reached the process step.
The same directory exits 0 on main, where the failure was silent.

Do what the process command does with the files it cannot process:

  - Sort the videos, so they are sampled and reported in a stable order.
  - Carry on past a failed video, log why it failed, and log a sample
    summary that counts the failures by type, like the process summary.
  - Then raise a single MapillaryVideoSampleError, still exit code 7:

      Failed to sample 1 videos. To skip these errors, specify
      --skip_sample_errors

  - video_process processes the frames of the videos that did sample
    before re-raising it, as process writes out the descriptions it did
    produce before failing. video_process_and_upload then stops before
    uploading, as process_and_upload does on a process error.

With --skip_sample_errors the failures are logged as warnings and the
command exits 0.

Since the aggregated error names the flag, the per-raise hint and
_sampling_error() are gone. The errors read from the video's metadata
are raised as they are, so the summary counts them by their own type,
MapillaryGPSNoiseError or MapillaryVideoGPSNotFoundError, instead of all
of them as MapillaryVideoError.

Skipping every video used to end in "Import file or directory not
found" for the sample directory, which nothing had created. Create it
once there are videos to sample, so the process step finds it empty. A
directory without videos still fails as before.
---
 mapillary_tools/commands/video_process.py |  13 +-
 mapillary_tools/exceptions.py             |   6 +
 mapillary_tools/sample_video.py           | 116 ++++++++++------
 tests/integration/test_video_process.py   |  52 +++++++
 tests/unit/test_sample_video.py           | 161 +++++++++++++++++-----
 5 files changed, 273 insertions(+), 75 deletions(-)

diff --git a/mapillary_tools/commands/video_process.py b/mapillary_tools/commands/video_process.py
index 83010873..d59b1280 100644
--- a/mapillary_tools/commands/video_process.py
+++ b/mapillary_tools/commands/video_process.py
@@ -5,6 +5,7 @@
 
 import logging
 
+from .. import exceptions
 from ..types import FileType
 from .process import Command as ProcessCommand
 from .sample_video import Command as SampleCommand
@@ -22,7 +23,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 +42,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/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 b6548685..b4b1e501 100644
--- a/mapillary_tools/sample_video.py
+++ b/mapillary_tools/sample_video.py
@@ -22,30 +22,14 @@
 
 LOG = logging.getLogger(__name__)
 
-# Sampling errors are suppressed by --skip_sample_errors, not by
-# --skip_process_errors, which governs the later geotagging stage. Say so in
-# the message: the two flags are easy to reach for the wrong one.
-_SKIP_HINT = "To skip these errors, specify --skip_sample_errors"
-
-
-def _sampling_error(message: str) -> exceptions.MapillaryVideoError:
-    """
-    Build an error for a failed sample, naming the flag that skips it.
-
-    Everything raised out of the per-video body of sample_video() is
-    suppressible by --skip_sample_errors, so every one of those messages should
-    say so. Going through here rather than appending the hint at each raise
-    keeps that true of raises added later.
-    """
-    return exceptions.MapillaryVideoError(f"{message}. {_SKIP_HINT}")
-
 
 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)
@@ -109,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(
@@ -120,6 +113,7 @@ def sample_video(
                 video_path.name,
                 sample_dir,
             )
+            num_already_sampled += 1
             continue
 
         try:
@@ -146,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
@@ -210,7 +245,7 @@ def _sample_single_video_by_interval(
             ffmpeg.probe_format_and_streams(video_path)
         ).probe_video_start_time()
         if start_time is None:
-            raise _sampling_error(
+            raise exceptions.MapillaryVideoError(
                 f"Unable to extract video start time from {video_path}"
             )
 
@@ -306,7 +341,7 @@ def _sample_single_video_by_distance(
     if start_time is None:
         start_time = probe.probe_video_start_time()
         if start_time is None:
-            raise _sampling_error(
+            raise exceptions.MapillaryVideoError(
                 f"Unable to extract video start time from {video_path}"
             )
 
@@ -321,25 +356,22 @@ def _sample_single_video_by_distance(
     # 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. sample_video()
-    # already funnels these through --skip_sample_errors for callers who do
-    # want to tolerate them.
+    # (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):
-        raise _sampling_error(
-            f"Unable to sample {video_path} by distance: {video_metadata.error}"
-        ) from video_metadata.error
+        raise video_metadata.error
 
     if not video_metadata.points:
-        raise _sampling_error(
-            f"Unable to sample {video_path} by distance: no GPS points found"
-        )
+        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:
-        raise _sampling_error(f"No video streams found in {video_path} by ffprobe")
+        raise exceptions.MapillaryVideoError(
+            f"No video streams found in {video_path} by ffprobe"
+        )
 
     LOG.info("Extracting video samples")
     video_stream_idx = video_stream["index"]
@@ -362,7 +394,7 @@ def _sample_single_video_by_distance(
             wip_dir, video_path, selected_stream_specifiers=[str(video_stream_idx)]
         )
         if len(frame_samples) != len(sorted_sample_indices):
-            raise _sampling_error(
+            raise exceptions.MapillaryVideoError(
                 f"Expect {len(sorted_sample_indices)} samples but extracted {len(frame_samples)} samples"
             )
         for idx, (frame_idx_1based, sample_paths) in enumerate(frame_samples):
@@ -370,7 +402,7 @@ def _sample_single_video_by_distance(
                 "Expect 1 sample path at {frame_idx_1based} but got {sample_paths}"
             )
             if idx + 1 != frame_idx_1based:
-                raise _sampling_error(
+                raise exceptions.MapillaryVideoError(
                     f"Expect {sample_paths[0]} to be {idx + 1}th sample but got {frame_idx_1based}"
                 )
 
diff --git a/tests/integration/test_video_process.py b/tests/integration/test_video_process.py
index aff97e68..1252bec8 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
@@ -88,6 +89,57 @@ def test_sample_video_by_distance_without_usable_gps(setup_data: py.path.local):
         )
 
 
+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_sample_video_without_video_time(setup_data: py.path.local):
     pytest_skip_if_not_ffmpeg_installed()
 
diff --git a/tests/unit/test_sample_video.py b/tests/unit/test_sample_video.py
index 15dcc6f3..f46819ab 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
@@ -371,13 +372,14 @@ def _sample(self, tmpdir: py.path.local, **kwargs):
             self.VIDEO, Path(tmpdir), video_sample_distance=2, rerun=True, **kwargs
         )
 
-    def test_it_raises_instead_of_returning(self, tmpdir, setup_mock, unreadable_gps):
-        with pytest.raises(exceptions.MapillaryVideoError) as excinfo:
+    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 survive into the message the user sees
-        assert "GPS is too noisy" in str(excinfo.value)
-        assert excinfo.value.__cause__ is unreadable_gps
+        # 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
@@ -411,6 +413,14 @@ def test_skip_sample_errors_still_tolerates_it(
         """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(
@@ -725,22 +735,117 @@ def test_exif_lat_lon_written(self, tmp_path: Path) -> None:
         assert abs(lon - (-74.0)) < 0.01
 
 
-class TestEverySuppressibleErrorNamesTheFlag:
+class TestFailuresAreReportedTogether:
     """
-    Every error raised out of the per-video body of sample_video() is
-    suppressed by --skip_sample_errors, so every one of those messages has to
-    name it. Half of them naming it is worse than none: a user who hits one of
-    the silent ones reaches for --skip_process_errors, which governs the later
-    geotagging stage and leaves the run failing with the same message.
+    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 no_start_time(self, monkeypatch):
+    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(
-            ffmpeglib.Probe, "probe_video_start_time", lambda _self: None
+            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",
@@ -752,30 +857,22 @@ def no_start_time(self, monkeypatch):
             ),
         ],
     )
-    def test_unreadable_start_time_names_the_flag(
-        self, tmpdir, setup_mock, no_start_time, kwargs
+    def test_unreadable_start_time(
+        self, tmpdir, setup_mock, monkeypatch, kwargs, caplog
     ):
-        with pytest.raises(exceptions.MapillaryVideoError) as excinfo:
-            sample_video.sample_video(self.VIDEO, Path(tmpdir), rerun=True, **kwargs)
-
-        assert "Unable to extract video start time" in str(excinfo.value)
-        assert "--skip_sample_errors" in str(excinfo.value)
+        monkeypatch.setattr(
+            ffmpeglib.Probe, "probe_video_start_time", lambda _self: None
+        )
 
-    def test_the_helper_appends_it_once(self):
-        error = sample_video._sampling_error("something went wrong")
+        with pytest.raises(exceptions.MapillaryVideoSampleError):
+            sample_video.sample_video(self.VIDEO, Path(tmpdir), rerun=True, **kwargs)
 
-        assert str(error) == (
-            "something went wrong. To skip these errors, specify --skip_sample_errors"
-        )
-        assert isinstance(error, exceptions.MapillaryUserError)
+        assert "Unable to extract video start time" in caplog.text
 
-    def test_a_missing_ffmpeg_does_not_claim_to_be_skippable(
-        self, tmpdir, setup_mock, monkeypatch
-    ):
+    def test_a_missing_ffmpeg_is_still_fatal(self, tmpdir, setup_mock, monkeypatch):
         """
-        FFmpegNotFoundError is re-raised by its own handler before the skip
-        check, so --skip_sample_errors does not suppress it and its message
-        must not offer the flag.
+        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):

From 3649f345c180beab95af7694f308ead8d1775103 Mon Sep 17 00:00:00 2001
From: Caglar Pir <caglarp@meta.com>
Date: Wed, 23 Sep 2026 17:00:10 +0200
Subject: [PATCH 5/8] Fail distance sampling that selects no frames

A GPS track that covers none of the video, for example a GPX from
another trip, puts no frame within the track, so distance sampling
selected nothing. The command exited 0 with an empty sample directory:
the same silent success as the unreadable GPS fixed earlier, reached
through a readable track instead.

Treat it as a failed sample, so the summary counts it and the command
fails unless --skip_sample_errors is specified:

  No video frames found within the GPS track from 100.000s to 109.000s
---
 mapillary_tools/sample_video.py |  5 +++++
 tests/unit/test_sample_video.py | 29 +++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/mapillary_tools/sample_video.py b/mapillary_tools/sample_video.py
index b4b1e501..b0d32652 100644
--- a/mapillary_tools/sample_video.py
+++ b/mapillary_tools/sample_video.py
@@ -380,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/unit/test_sample_video.py b/tests/unit/test_sample_video.py
index f46819ab..9f1b9299 100644
--- a/tests/unit/test_sample_video.py
+++ b/tests/unit/test_sample_video.py
@@ -734,6 +734,35 @@ def test_exif_lat_lon_written(self, tmp_path: Path) -> None:
         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:
     """

From 8423f1d7fad0aa6d9b38473f2cfe47cd7c7b4143 Mon Sep 17 00:00:00 2001
From: Caglar Pir <caglarp@meta.com>
Date: Wed, 23 Sep 2026 17:00:10 +0200
Subject: [PATCH 6/8] Document --skip_sample_errors

The flag was missing from the README. Say what happens without it when a
video cannot be sampled, and that it does not cover the errors that
--skip_process_errors skips.
---
 README.md                                | 10 ++++++++++
 mapillary_tools/commands/sample_video.py |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

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,

From bc5b69446e2e0336ba6f5fc0f96b22988f8a6683 Mon Sep 17 00:00:00 2001
From: Caglar Pir <caglarp@meta.com>
Date: Mon, 19 Oct 2026 16:39:54 +0200
Subject: [PATCH 7/8] Postpone annotations in video_process for mypy on Python
 3.9

CI type-checks with mypy on Python 3.9, which rejects the
`MapillaryVideoSampleError | None` annotation added to video_process
unless annotations are postponed, and so failed every build. Add
`from __future__ import annotations`, as the other modules do. The
annotation is on a local variable, which Python does not evaluate, so the
command itself was not affected.
---
 mapillary_tools/commands/video_process.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mapillary_tools/commands/video_process.py b/mapillary_tools/commands/video_process.py
index d59b1280..d127b9b9 100644
--- a/mapillary_tools/commands/video_process.py
+++ b/mapillary_tools/commands/video_process.py
@@ -3,6 +3,8 @@
 # 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 7cd30258cc3c167805be4074ae079f47923b249f Mon Sep 17 00:00:00 2001
From: Caglar Pir <caglarp@meta.com>
Date: Tue, 20 Oct 2026 11:34:45 +0200
Subject: [PATCH 8/8] Say that video_process_and_upload uploaded nothing

When a video fails to sample, video_process_and_upload exits 7 without
uploading the videos that did sample, as process_and_upload does with
the files it fails to process. But the last thing it logged before the
error was the process summary, with their sample images "ready", so it
read as if they were about to be uploaded:

  ==> Process summary
          3 (406.2 kB) ready
  MapillaryVideoSampleError: Failed to sample 1 videos. To skip these
  errors, specify --skip_sample_errors

Log that nothing was uploaded, and that --skip_sample_errors uploads the
others, before failing.
---
 .../commands/video_process_and_upload.py      | 19 ++++++++-
 tests/integration/test_video_process.py       | 40 +++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)

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/tests/integration/test_video_process.py b/tests/integration/test_video_process.py
index 1252bec8..055ad9be 100644
--- a/tests/integration/test_video_process.py
+++ b/tests/integration/test_video_process.py
@@ -18,7 +18,10 @@
     pytest_skip_if_not_ffmpeg_installed,
     run_command,
     run_process_for_descs,
+    setup_config,
     setup_data,
+    setup_upload,
+    USERNAME,
 )
 
 
@@ -140,6 +143,43 @@ def test_video_process_skips_the_only_video(setup_data: py.path.local):
     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()