Fail the sample instead of silently producing no frames

[caglarpir]

Problem

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, so 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, but nothing about it is specific to noisy GPS — a video with no GPS at all takes the same path. Same clip, same GPX, only the sampling mode differs:

	exit	frames
--video_sample_distance 3	0	0
--video_sample_interval 2	0	3

Fix

Distance sampling needs positions to decide which frames to cut, so failing to read them is a failed sample. It now raises instead of warning and returning, with the error read from the video's metadata as it is (MapillaryGPSNoiseError, MapillaryVideoGPSNotFoundError, ...). Two neighbouring silent paths get the same treatment: a missing video stream (also returned after a warning), and an empty point list (an assert, so it disappeared under python -O, leaving an IndexError further down).

A track that covers none of the video, such as a GPX from another trip, selected no frames and exited 0 with an empty sample directory, the same silent success reached through a readable track. That is a failed sample too (3649f34).

A video that fails no longer stops the others (38ded6a). Raising out of the first failure made the result for a directory depend on which video that was, and left the frames of the videos sampled before it unprocessed. Sampling now does what process does with the files it cannot process:

• It sorts the videos, so that they are sampled and reported in a stable order.
• It carries on past a failed video, logs why it failed, and ends with a sample summary that counts the failures by type.
• It then raises one MapillaryVideoSampleError, exit code 7, naming the flag that skips it.
• 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, and says so.

ERROR   - Failed to sample .../gopro_data/hero8.mp4: GPS is too noisy
INFO    - ==> Sample summary
INFO    - 2 videos found
INFO    -       1 sampled
ERROR   -       1 MapillaryGPSNoiseError
INFO    - ==> Process summary
INFO    -       3 (406.2 kB) ready
ERROR   - MapillaryVideoSampleError: Failed to sample 1 videos. To skip these errors, specify --skip_sample_errors

With --skip_sample_errors the failures are logged as warnings, and the command exits 0. A missing ffmpeg still fails at once, and its message does not offer the flag, which does not suppress it. The sample directory is created as soon as there are videos to sample, so skipping every video no longer ends in "Import file or directory not found".

The README now documents --skip_sample_errors, and that it covers sampling only, not the errors --skip_process_errors skips (8423f1d).

Behaviour change worth flagging (for the release notes)

A directory with a video that cannot be sampled now exits 7 unless --skip_sample_errors is specified, where it used to exit 0 having silently left that video out. The other videos are still sampled and processed, and their descriptions written.

video_process_and_upload then uploads nothing, where it used to upload the other videos. That is deliberate: a run that failed should not pass for one that worked, and process_and_upload does the same with the files it fails to process. Because the last thing logged before the error is that the sample images are ready, it says that nothing was uploaded (7cd3025):

INFO    - ==> Process summary
INFO    -       3 (406.2 kB) ready
ERROR   - Nothing was uploaded, because some videos failed to sample. To upload the sample images of the others, specify --skip_sample_errors
ERROR   - MapillaryVideoSampleError: Failed to sample 1 videos. To skip these errors, specify --skip_sample_errors

With --skip_sample_errors it uploads the videos that did sample and exits 0, as it used to.

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, and new tests pin the loud behaviour, and that the other videos are still processed.

Scope

This does not make --geotag_source reach distance sampling. That is #841, which is stacked on this PR. This one only stops failures from being silent.

Independent of #831 (different file, branches off main).

Verification

Full suite passes (773), with mypy (for Python 3.9, as CI checks, and 3.13), ruff and usort clean.

[caglarpir]

Pushed bb5873c — good catch, the trap is real.

The raise said the sample failed but not how to get past it, and the obvious guess is the wrong one: --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.

The message now reads:

MapillaryVideoError: Unable to sample GS018205.360 by distance: GPS is too noisy. To skip these errors, specify --skip_sample_errors

Applied to all three raises in this PR, wording matching process_geotag_properties.py. Added a test asserting the flag is named, so it cannot quietly drop out of the message later.

760 unit + integration tests pass, mypy clean.

[caglarpir]

Pushed 976c135 — took the polish, thanks.

You were right that half the errors naming the flag is worse than none: a user who lands on a silent one reaches for --skip_process_errors and gets nowhere. All four now name it, alongside the three this PR already covered.

Rather than append the hint at seven call sites, where it can drift out of sync again, they go through a small constructor:

def _sampling_error(message: str) -> exceptions.MapillaryVideoError:
    return exceptions.MapillaryVideoError(f"{message}. {_SKIP_HINT}")

so a raise added later cannot forget it.

One deliberate exception: MapillaryFFmpegNotFoundError keeps its plain message. It is re-raised by its own handler before the skip check, so --skip_sample_errors genuinely does not suppress it and it must not claim otherwise. There is a test pinning that, plus tests for both newly hinted start-time paths — the two frame-count raises remain unexercised, as you noted, but they can no longer be inconsistent by construction.

713 unit + 57 integration pass, mypy and ruff clean.