diff --git a/mapillary_tools/geotag/factory.py b/mapillary_tools/geotag/factory.py index 87c0016d..e11984ec 100644 --- a/mapillary_tools/geotag/factory.py +++ b/mapillary_tools/geotag/factory.py @@ -289,7 +289,8 @@ def process( rejected_gps_paths: set[Path] = set() # Paths whose GPX track missed them, with the source that rejected them, to - # warn about if a later source geotags them instead + # warn about if a later source geotags them instead, and to report if none + # does gpx_rejections: dict[Path, tuple[SourceOption, types.ErrorMetadata]] = {} # Videos let through to sources that cannot geotag videos (e.g. exif), with @@ -399,6 +400,14 @@ def process( if not reprocessable_paths: break + # Of a file that a GPX track missed, and that no later source geotagged, + # report the miss: why the track given for it was not used says more than + # the later sources failing too, such as "GPS is too noisy" for a video + # whose GPS the track was given to replace + for path, (_, gpx_miss) in gpx_rejections.items(): + if path in pending_errors: + pending_errors[path] = gpx_miss + final_metadatas.extend(pending_errors.values()) unread_paths = [ diff --git a/tests/unit/test_gpx_over_noisy_gps.py b/tests/unit/test_gpx_over_noisy_gps.py index f8c49411..ac586847 100644 --- a/tests/unit/test_gpx_over_noisy_gps.py +++ b/tests/unit/test_gpx_over_noisy_gps.py @@ -374,6 +374,13 @@ def build_video_geotag(option: SourceOption): GPX_NOT_FOUND = exceptions.MapillaryVideoGPSNotFoundError( "GPX file not found for video" ) +GPX_MISS = exceptions.MapillaryOutsideGPXTrackError( + "The GPX track misses the video by 105.1 days", + image_time="2026_09_11_15_39_41_000", + gpx_start_time="2026_05_29_12_47_06_000", + gpx_end_time="2026_05_29_12_53_00_000", +) +XML_NOT_FOUND = exceptions.MapillaryExifToolXMLNotFoundError("No XML file found") DEFAULT_CHAIN = [SourceType(source) for source in DEFAULT_GEOTAG_SOURCE_OPTIONS] @@ -505,6 +512,60 @@ def test_gpx_that_fails_replaces_the_rejection(self, monkeypatch): assert isinstance(result, types.ErrorMetadata) assert result.error is mismatch + def test_gpx_that_misses_is_reported_over_rejected_gps(self, monkeypatch): + """ + The chain of the Desktop Uploader, with a GPX that misses a video whose + GPS is too noisy: the miss says why the GPX was not used, while "GPS + is too noisy" is what the GPX was given to work around. + """ + results, called = _run_chain( + monkeypatch, + [ + SourceType.GPX, + SourceType.EXIFTOOL_XML, + SourceType.NATIVE, + SourceType.EXIFTOOL_RUNTIME, + ], + { + SourceType.GPX: GPX_MISS, + SourceType.EXIFTOOL_XML: XML_NOT_FOUND, + SourceType.NATIVE: NOISE, + SourceType.EXIFTOOL_RUNTIME: None, + }, + ) + result = _only_result(results) + assert isinstance(result, types.ErrorMetadata) + assert result.error is GPX_MISS + assert called == [SourceType.GPX, SourceType.EXIFTOOL_XML, SourceType.NATIVE] + + @pytest.mark.parametrize( + "later_error", + [NOISE, EMPTY, NOT_FOUND, exceptions.MapillaryExiftoolNotFoundError("no")], + ) + def test_gpx_that_misses_is_reported_over_later_failures( + self, monkeypatch, later_error + ): + results, _ = _run_chain( + monkeypatch, + [SourceType.GPX, SourceType.NATIVE], + {SourceType.GPX: GPX_MISS, SourceType.NATIVE: later_error}, + ) + result = _only_result(results) + assert isinstance(result, types.ErrorMetadata) + assert result.error is GPX_MISS + + def test_gpx_that_misses_does_not_hide_a_final_error(self, monkeypatch): + """An error that ends the chain says something about the file itself""" + stationary = exceptions.MapillaryStationaryVideoError("Stationary") + results, _ = _run_chain( + monkeypatch, + [SourceType.GPX, SourceType.NATIVE], + {SourceType.GPX: GPX_MISS, SourceType.NATIVE: stationary}, + ) + result = _only_result(results) + assert isinstance(result, types.ErrorMetadata) + assert result.error is stationary + def test_not_found_still_falls_through_to_exiftool(self, monkeypatch): """Finding nothing is a verdict on the reader, so retrying is fair.""" results, called = _run_chain( diff --git a/tests/unit/test_gpx_video_coverage.py b/tests/unit/test_gpx_video_coverage.py index 8fa98c19..2e657e30 100644 --- a/tests/unit/test_gpx_video_coverage.py +++ b/tests/unit/test_gpx_video_coverage.py @@ -527,6 +527,30 @@ def test_video_is_geotagged_by_the_next_source_with_a_warning( assert "misses the video" in warning and "by 2.0 hours" in warning assert warning.endswith("Geotagged it from the native source instead") + def test_the_miss_is_reported_if_no_later_source_geotags_the_video(self, tmp_path): + video_path = _write_camm_mp4(tmp_path / "video.mp4", _video_track()) + gpx_path = _write_gpx(tmp_path / "track.gpx", A_UNIX_TIME + 2 * 3600, 10) + options = [ + SourceOption( + SourceType.GPX, + num_processes=0, + source_path=SourcePathOption(source_path=gpx_path), + ), + # Finds no GPX file to read + SourceOption( + SourceType.GPX, + num_processes=0, + source_path=SourcePathOption(source_path=tmp_path / "none.gpx"), + ), + ] + + [metadata] = factory.process([video_path], options) + + assert isinstance(metadata, types.ErrorMetadata) + assert isinstance(metadata.error, exceptions.MapillaryOutsideGPXTrackError) + assert "misses the video" in str(metadata.error) + assert "by 2.0 hours" in str(metadata.error) + @pytest.mark.parametrize( "gpx_start, source", [(A_UNIX_TIME, SourceType.GPX), (A_UNIX_TIME + 2 * 3600, SourceType.NATIVE)],