Let a GPX override a video whose own GPS is unusable

[caglarpir]

Problem

Attaching a GPX is the documented escape hatch for a video with bad embedded GPS, but for a GoPro whose GPS is rejected as noise the upload still fails with

MapillaryGPSNoiseError: GPS is too noisy

no matter what GPX is supplied.

Reported against a GoPro MAX2 .360 recorded with no GPS fix: all 32 GPMF points carry fix=NO_FIX and a DoP of 2139 (the limit is 1000), so remove_noisy_points() drops every one.

Cause

This used to work. Before the geotag refactor, NativeVideoExtractor returned an ErrorMetadata value and GPXVideoExtractor fell back to the GPX on any failure. It now raises instead, and only one of the three "no usable GPS" errors was being caught:

except exceptions.MapillaryVideoGPSNotFoundError as ex:

MapillaryGPSNoiseError and MapillaryGPXEmptyError are siblings of that class rather than subclasses, so they escape the handler and fail the whole video.

That also explains the reporter's observation that the same GPX works on a video with no embedded GPS at all: that path raises MapillaryVideoGPSNotFoundError, which is caught.

The same bug had a second instance in factory._is_reprocessable(), which also listed only MapillaryVideoGPSNotFoundError. Chaining sources, as in --geotag_source native --geotag_source gpx, failed at the native stage and never reached the GPX.

Fix

Rather than only widening the except clause, stop filtering in the first place when the caller is the GPX path. The noise filter is a quality gate on the track we are about to publish; once a GPX replaces that track, the video's own GPS is just a source of make/model and of a clock to sync against, and neither is improved by discarding points.

Widening the except clause alone would work, but it drops the video into the bare-VIDEO fallback and loses filetype=gopro, make/model, and the sync anchor — the GPX would be rebased to 0 instead of to its real +2.0s offset. The timestamps are still good with no fix, so they still sync. The except clause is widened as well, for the genuinely empty case where there is no clock to recover.

_is_reprocessable() now treats unusable GPS as reprocessable, since that is exactly what a later source is there to replace.

The gate itself is unchanged: a noisy video with no GPX supplied is still rejected with "GPS is too noisy".

Verification

Verified end to end on the reported file. Processing now reports 1 gopro read / ready, and a dry-run upload produces an mp4 whose CAMM track carries the GPX coordinates at t=2.0, 4.0, ... with GoPro/MAX2 preserved.

Of the 11 new tests in tests/unit/test_gpx_over_noisy_gps.py, 7 fail without this change and the 4 guard tests pass either way. Full suite: 696 unit + 52 integration tests pass, mypy clean.

[caglarpir]

Pushed 898988b to fix a regression in the first commit — thanks to @caglarpir for catching it.

Making MapillaryGPSNoiseError reprocessable was too broad. The default chain is native, exiftool_runtime, so a video the native parser had just rejected as noise fell through to exiftool, and mapillary_tools process with no flags at all started accepting the very file this PR is about.

The two readers disagree because they do not see the same fields:

reader	fix	DoP (limit 1000)	after remove_noisy_points()
native GPMF	24×FIX_3D, 8×NO_FIX	~2100–2139	0 / 32 → rejected
exiftool	24×FIX_3D, 8×NO_FIX	None for all 32	24 / 32 → accepted

exiftool losing GPSP is a pre-existing bug — --geotag_source exiftool_runtime already accepts this file on main, independently of this PR. What changed here was only that the default chain started reaching it. I've left that one alone as out of scope; happy to file it separately.

The fix: unusable GPS is a verdict on the data, not on the reader that reported it, so only a source supplying GPS from outside the video can overturn it. The fall-through is now gated on the remaining sources — GPX and NMEA can rescue the file, another reader of the same embedded telemetry cannot. MapillaryVideoGPSNotFoundError is unaffected, since "could not read it" really is a verdict on the reader.

Verified on the reported file:

process (default)                    -> GPS is too noisy
process --geotag_source native       -> GPS is too noisy
process --geotag_source gpx          -> 1 gopro ready
process --geotag_source native,gpx   -> 1 gopro ready

Tests are up to 20 (from 11), including one pinning the default chain specifically. 705 unit + 52 integration pass, mypy clean.