From b5b61c369938606b7506f768b471addf119f23f4 Mon Sep 17 00:00:00 2001 From: Caglar Pir Date: Mon, 2 Nov 2026 13:56:51 +0100 Subject: [PATCH] Refuse again a GPX track far longer than a video without a GPS clock A video without a GPS clock of its own gives nothing to place a GPX track by, and the track is assumed to start with it. A track that runs on past the end of the video for longer than the whole video was not recorded for the video alone: the video could start anywhere along it. Sampling refused such a track from #841 until #944, which only warns that the video start time needs aligning with the track. Since then, sampling places the video at the start of whatever the track recorded, such as the first trip of a log of several. Sampling refuses it again, by distance and by interval alike, with the MapillaryGPXVideoStartUnknownError and the message it gave before, which for a track of a day or more says that the track covers more than the video. Nothing is refused when --video_start_time places the video along the track, when --interpolation_offset_time looks the video up along it, or when the video spans the track at --video_duration_ratio. A track shorter than the video, or longer by less than the video lasts, keeps the warning. process, which takes neither --video_start_time nor --video_duration_ratio, still uploads such a track whole, with the warning, as the track of a timelapse runs on past the end of its video too. It refuses only a track of a day or more, a log of several trips whose start is that of the first, unless the video lasts more than half as long. The creation time of the video still places nothing. --- .../geotag/video_extractors/gpx.py | 84 +++++++-- mapillary_tools/sample_video.py | 11 +- tests/integration/test_process.py | 33 ++++ tests/integration/test_video_process.py | 56 +++++- tests/unit/test_gpx_video_alignment.py | 176 ++++++++++++++++- tests/unit/test_gpx_video_sync.py | 178 +++++++++++++++--- tests/unit/test_nmea_video.py | 13 +- tests/unit/test_sample_video.py | 67 +++++++ 8 files changed, 558 insertions(+), 60 deletions(-) diff --git a/mapillary_tools/geotag/video_extractors/gpx.py b/mapillary_tools/geotag/video_extractors/gpx.py index d5b0bbde..eb1a4792 100644 --- a/mapillary_tools/geotag/video_extractors/gpx.py +++ b/mapillary_tools/geotag/video_extractors/gpx.py @@ -441,10 +441,10 @@ def _place_without_video_gps( Place the GPX track on the timeline of a video that has no GPS clock to sync it with: by the start time of the video, when sampling was given one, and otherwise by assuming that the track starts with the video, - with a warning when their lengths differ (see _check_length). No other - time of the video, such as its creation time, which the camera's own - clock stamps, places the track. It runs at --video_duration_ratio GPS - seconds per second of video. + which is refused or warned of when their lengths differ (see + _check_length). No other time of the video, such as its creation time, + which the camera's own clock stamps, places the track. It runs at + --video_duration_ratio GPS seconds per second of video. Return the clock of the video the track was placed by: the start time, or None if the track was assumed to start with the video. @@ -732,16 +732,19 @@ def _check_coverage( def _check_length(self, gpx_points: T.Sequence[geo.Point], ratio: float) -> None: """ - Warn when the GPX track, assumed to start with the video because the - video has no GPS clock to place it by, lasts longer or shorter than the - video by more than _GPX_LENGTH_TOLERANCE_SECONDS: the start of the video - then needs aligning with the track. A longer track warns only if it also + Check the GPX track, assumed to start with the video because the video + has no GPS clock to place it by, against the length of the video, which + spans **ratio** GPS seconds per second. A track that runs on for longer + than the whole video may be refused (see _check_assumed_start). + + Otherwise, warn when the track lasts longer or shorter than the video + by more than _GPX_LENGTH_TOLERANCE_SECONDS: the start of the video then + needs aligning with the track. A longer track warns only if it also lasts more than _MIN_LONG_GPX_RATIO times as long as the video, and may - be that of a timelapse. The video spans **ratio** GPS seconds per second. + be that of a timelapse. - The track is taken to be right, so neither is refused. --interpolation_offset_time places the start of the video along the - track, which leaves nothing to warn of. + track, which leaves nothing to refuse or warn of. """ if self.interpolation is not None and self.interpolation.offset_time: return @@ -751,6 +754,8 @@ def _check_length(self, gpx_points: T.Sequence[geo.Point], ratio: float) -> None return video_span, gpx_duration = durations + self._check_assumed_start(gpx_points, video_span, gpx_duration, ratio) + longer = ( _GPX_LENGTH_TOLERANCE_SECONDS < gpx_duration - video_span and _MIN_LONG_GPX_RATIO * video_span < gpx_duration @@ -801,6 +806,63 @@ def _check_length(self, gpx_points: T.Sequence[geo.Point], ratio: float) -> None LOG.warning(message) self.warnings.append(warning) + def _check_assumed_start( + self, + gpx_points: T.Sequence[geo.Point], + video_span: float, + gpx_duration: float, + ratio: float, + ) -> None: + """ + Refuse a GPX track assumed to start with the video that runs on for + longer than the whole video, which spans **video_span** GPS seconds at + **ratio** GPS seconds per second: it was not recorded for the video + alone, and the video could start anywhere along it. + + Sampling refuses it: --video_start_time places the video along the + track, and --video_duration_ratio spans it at the pace of a timelapse. + process takes neither, and refuses only a track that lasts longer than + _MAX_RECORDING_SECONDS, a log of several trips whose start is that of + the first. A shorter one may be the track of a timelapse, which runs on + past the end of the video by design: process uploads it whole, with a + warning to align them (see _check_length). + """ + excess = gpx_duration - video_span + if excess <= video_span: + return + if self.video_sync is None and gpx_duration < _MAX_RECORDING_SECONDS: + return + + message = ( + f"The {self.file_format} track in {self.gpx_path} lasts {_format_duration(gpx_duration)} " + f"and {self._describe_video_span(video_span, ratio)}, so the video " + "could start anywhere along the track. " + ) + if self.video_sync is None: + # process, which has no --video_start_time + message += ( + "A track that long covers more than this video: trim the " + f"{self.file_format} to the video, or sample the video with " + "video_process and --video_start_time" + ) + # Not --interpolation_use_gpx_start_time, which starts the track with + # the video too, and refuses it for the same reason + elif gpx_duration < _MAX_RECORDING_SECONDS: + message += ( + "If the video is a timelapse, specify --video_duration_ratio. If " + "the GPS logger was started with the video and left running, " + "specify --video_start_time " + f"{build_capture_time(min(p.time for p in gpx_points))}, when the " + "track starts. If the track covers more than this video, specify " + "the time the video started as --video_start_time instead" + ) + else: + message += ( + "A track that long covers more than this video: specify the time " + "the video started as --video_start_time" + ) + raise exceptions.MapillaryGPXVideoStartUnknownError(message) + def _check_given_start( self, gpx_points: T.Sequence[geo.Point], ratio: float ) -> None: diff --git a/mapillary_tools/sample_video.py b/mapillary_tools/sample_video.py index 553ae531..d0ec612b 100644 --- a/mapillary_tools/sample_video.py +++ b/mapillary_tools/sample_video.py @@ -718,10 +718,11 @@ def _track_file_start_time( of the first source that geotags images, which the geotagging stage places the interval samples along by their capture times: the start of the track, which is assumed to start with the video, as GPX sync places a track on - such a video (see GPXVideoExtractor._place_without_video_gps), with a - warning when their lengths differ. None without such a track, or when - --interpolation_use_gpx_start_time starts the track with the video, which - the geotagging stage does. + such a video (see GPXVideoExtractor._place_without_video_gps): refused + if it runs on for longer than the whole video, and warned of if their + lengths differ (see GPXVideoExtractor._check_length). None without such a + track, or when --interpolation_use_gpx_start_time starts the track with + the video, which the geotagging stage does. """ for option in geotag_options: extractor_class = _TRACK_FILE_EXTRACTORS.get(option.source) @@ -751,6 +752,8 @@ def _track_file_start_time( return None if not gpx_points: return None + # Refused or warned of as in distance sampling, which places the track + # by GPX sync extractor._check_length( gpx_points, 1.0 if video_sync is None else video_sync.duration_ratio ) diff --git a/tests/integration/test_process.py b/tests/integration/test_process.py index 0a1a0364..f35a1be1 100644 --- a/tests/integration/test_process.py +++ b/tests/integration/test_process.py @@ -1715,6 +1715,39 @@ def test_process_video_with_a_gpx_far_longer_than_it_warns( assert desc["MAPGPSTrack"][0][0] == 0 +def test_process_video_with_a_gpx_longer_than_a_day_fails( + setup_data: py.path.local, +): + """ + A track that lasts more than a day is a log of several trips, whose start + is that of the first: process does not assume that the video starts with + it, as sampling does not for any track that runs on past the video for + longer than the video lasts + """ + video_path = setup_data.join("videos").join("sample-5s.mp4") + start = datetime.datetime(2025, 3, 14, 7, 0, 0, tzinfo=datetime.timezone.utc) + gpx_file = setup_data.join("track.gpx") + # Two trips two days apart + _write_track_parts( + gpx_file, + [(start, 10, 37.0), (start + datetime.timedelta(days=2), 10, 37.1)], + ) + + [desc] = run_process_for_descs( + [ + *["--geotag_source", "gpx"], + *["--geotag_source_path", str(gpx_file)], + str(video_path), + ] + ) + + assert desc["error"]["type"] == "MapillaryGPXVideoStartUnknownError" + message = desc["error"]["message"] + assert "lasts 2.0 days" in message + assert "could start anywhere along the track" in message + assert "trim the GPX to the video" in message + + def test_process_image_with_a_placeholder_capture_time_and_offset( tmpdir: py.path.local, ): diff --git a/tests/integration/test_video_process.py b/tests/integration/test_video_process.py index 05aa0682..07e52c6c 100644 --- a/tests/integration/test_video_process.py +++ b/tests/integration/test_video_process.py @@ -503,14 +503,15 @@ def test_video_process_by_distance_places_a_gpx_by_the_video_start_time( @pytest.mark.parametrize( "creation_time", ["2025-03-14T07:00:36Z", None], ids=["creation time", "none"] ) -def test_video_process_assumes_a_long_gpx_starts_with_the_video( +def test_video_process_refuses_to_guess_where_the_video_starts( setup_data: py.path.local, sampling: list[str], creation_time: str | None ): """ - Without --video_start_time, a video without GPS of its own is assumed to - start with the GPX, which is taken to be right: never by its creation time, - which the camera's clock stamps, even where the track covers it. A track - much longer than the video warns to align the two, and is not refused. + Without --video_start_time, a GPX far longer than the video could start + anywhere along it, so assuming that it starts with the video is a guess: + refused, by distance and by interval alike. Nor does the creation time of + the video, which the camera's clock stamps, place it, even where the track + covers it. """ pytest_skip_if_not_ffmpeg_installed() @@ -523,14 +524,54 @@ def test_video_process_assumes_a_long_gpx_starts_with_the_video( video_path = str( _with_creation_time(py.path.local(video_path), creation_time, dir) ) + with pytest.raises(subprocess.CalledProcessError) as ex: + run_video_process( + [ + *sampling, + "--geotag_source=gpx", + *["--geotag_source_path", str(gpx_file)], + video_path, + str(Path(dir, "samples")), + ], + capture_output=True, + text=True, + ) + + assert 7 == ex.value.returncode, ex.value.stderr + assert "could start anywhere along the track" in ex.value.stderr + assert "specify --video_start_time 2025_03_14_07_00_00_000" in ex.value.stderr + + +@pytest.mark.parametrize( + "sampling", + [ + ["--video_sample_distance=3"], + ["--video_sample_distance=-1", "--video_sample_interval=1"], + ], + ids=["by distance", "by interval"], +) +def test_video_process_takes_a_long_gpx_that_fits_at_the_duration_ratio( + setup_data: py.path.local, sampling: list[str] +): + """ + At 30 GPS seconds per second, the 5.76 seconds of video span 173 seconds, + past which the 93-second track does not run on: assumed to start with the + video, as the track of a timelapse + """ + pytest_skip_if_not_ffmpeg_installed() + + gpx_file = setup_data.join("gpx").join("sf_30km_h.gpx") + + with tempfile.TemporaryDirectory() as dir: desc_path = Path(dir, "descs.json") result = run_video_process( [ *sampling, + "--video_duration_ratio=30", "--geotag_source=gpx", *["--geotag_source_path", str(gpx_file)], *["--desc_path", str(desc_path)], - video_path, + str(setup_data.join("videos").join("sample-5s.mp4")), str(Path(dir, "samples")), ], capture_output=True, @@ -542,8 +583,7 @@ def test_video_process_assumes_a_long_gpx_starts_with_the_video( assert all("error" not in desc for desc in descs), descs capture_times = sorted(desc["MAPCaptureTime"] for desc in descs) assert "2025_03_14_07_00_00_000" <= capture_times[0], capture_times - assert capture_times[-1] < "2025_03_14_07_00_07_000", capture_times - assert "Please align the video start time with the GPX track" in result.stderr + assert "could start anywhere along the track" not in result.stderr def test_video_process_by_distance_with_a_gpx_that_misses_the_video( diff --git a/tests/unit/test_gpx_video_alignment.py b/tests/unit/test_gpx_video_alignment.py index a7dcb59c..4e40a74e 100644 --- a/tests/unit/test_gpx_video_alignment.py +++ b/tests/unit/test_gpx_video_alignment.py @@ -5,9 +5,14 @@ """ A GPX track placed on a video without a GPS clock is assumed to start with the -video, and taken to be right. When the two lengths differ by more than a -minute (and a longer track also lasts more than 1.5 times as long), the video -start time needs aligning with the track, which process and sampling warn of. +video. When the two lengths differ by more than a minute (and a longer track +also lasts more than 1.5 times as long), the video start time needs aligning +with the track, which process and sampling warn of. + +A track that runs on for longer than the whole video could have the video start +anywhere along it: sampling refuses it, unless --video_start_time places the +video or --video_duration_ratio makes the track fit, and so does process a +track that lasts longer than a day. """ from __future__ import annotations @@ -97,15 +102,157 @@ def test_a_track_more_than_a_minute_longer_warns_in_process(tmp_path, caplog): assert warning.gpx_longer -def test_sampling_warns_with_its_own_options(tmp_path, caplog): - gpx = _write_gpx(tmp_path / "track.gpx", seconds=120) +def test_sampling_warns_with_its_own_options(tmp_path, caplog, monkeypatch): + """Of a track longer than the video, though not twice as long""" + _video_lasting(monkeypatch, 600) + gpx = _write_gpx(tmp_path / "track.gpx", seconds=1000) + extractor = GPXVideoExtractor(NO_GPS_VIDEO, gpx, video_sync=VideoSyncOption()) with caplog.at_level(logging.WARNING): - GPXVideoExtractor(NO_GPS_VIDEO, gpx, video_sync=VideoSyncOption()).extract() + metadata = extractor.extract() [message] = _warnings(caplog) assert "specify the time the video started as --video_start_time" in message assert "If the video is a timelapse, specify --video_duration_ratio" in message + assert metadata.points[0].time == 0 + [warning] = extractor.warnings + assert warning.gpx_longer + + +def test_sampling_refuses_a_track_more_than_twice_as_long_as_the_video( + tmp_path, caplog +): + """ + The video could start anywhere along it: refused, as it was before the + warning, rather than warned of + """ + gpx = _write_gpx(tmp_path / "track.gpx", seconds=120) + extractor = GPXVideoExtractor(NO_GPS_VIDEO, gpx, video_sync=VideoSyncOption()) + + with caplog.at_level(logging.WARNING): + with pytest.raises(exceptions.MapillaryGPXVideoStartUnknownError) as excinfo: + extractor.extract() + + message = str(excinfo.value) + assert "lasts 2.0 minutes and the video only 5.76 seconds" in message + assert "so the video could start anywhere along the track" in message + assert "If the video is a timelapse, specify --video_duration_ratio" in message + assert "specify --video_start_time 2025_03_14_07_00_00_000" in message + assert not _warnings(caplog) + assert not extractor.warnings + + +@pytest.mark.parametrize("seconds, refused", [(1200, False), (1201, True)]) +def test_sampling_refuses_past_twice_the_video(tmp_path, monkeypatch, seconds, refused): + _video_lasting(monkeypatch, 600) + gpx = _write_gpx(tmp_path / "track.gpx", seconds=seconds) + extractor = GPXVideoExtractor(NO_GPS_VIDEO, gpx, video_sync=VideoSyncOption()) + + if refused: + with pytest.raises(exceptions.MapillaryGPXVideoStartUnknownError): + extractor.extract() + else: + extractor.extract() + [warning] = extractor.warnings + assert warning.gpx_longer + + +def test_sampling_takes_a_track_placed_by_the_video_start_time(tmp_path, caplog): + """--video_start_time places the video along the track: nothing to refuse""" + gpx = _write_gpx(tmp_path / "track.gpx", seconds=600) + video_start = GPX_START.timestamp() + 300 + + with caplog.at_level(logging.WARNING): + metadata = GPXVideoExtractor( + NO_GPS_VIDEO, gpx, video_sync=VideoSyncOption(start_time=video_start) + ).extract() + + assert metadata.points[300].time == pytest.approx(0) + assert not _warnings(caplog) + + +def test_sampling_takes_a_track_looked_up_at_an_offset(tmp_path, caplog): + """--interpolation_offset_time places the video along the track""" + gpx = _write_gpx(tmp_path / "track.gpx", seconds=600) + + with caplog.at_level(logging.WARNING): + GPXVideoExtractor( + NO_GPS_VIDEO, + gpx, + video_sync=VideoSyncOption(), + interpolation=InterpolationOption(offset_time=30), + ).extract() + + assert not _warnings(caplog) + + +def test_process_refuses_a_track_longer_than_a_day(tmp_path, caplog, monkeypatch): + """A log of several trips, whose start is that of the first""" + _video_lasting(monkeypatch, 600) + gpx = _write_gpx(tmp_path / "track.gpx", seconds=86400) + extractor = GPXVideoExtractor(NO_GPS_VIDEO, gpx) + + with caplog.at_level(logging.WARNING): + with pytest.raises(exceptions.MapillaryGPXVideoStartUnknownError) as excinfo: + extractor.extract() + + message = str(excinfo.value) + assert "lasts 1.0 days and the video only 10.0 minutes" in message + assert ( + "A track that long covers more than this video: trim the GPX to the video, " + "or sample the video with video_process and --video_start_time" + ) in message + assert not _warnings(caplog) + assert not extractor.warnings + + +def test_process_warns_of_a_track_shorter_than_a_day(tmp_path, caplog, monkeypatch): + """Which may be that of a timelapse, as long as it lasts""" + _video_lasting(monkeypatch, 600) + gpx = _write_gpx(tmp_path / "track.gpx", seconds=86399) + extractor = GPXVideoExtractor(NO_GPS_VIDEO, gpx) + + with caplog.at_level(logging.WARNING): + metadata = extractor.extract() + + assert metadata.points[0].time == 0 + [warning] = extractor.warnings + assert warning.gpx_longer + + +def test_process_takes_a_track_longer_than_a_day_that_fits_the_video( + tmp_path, caplog, monkeypatch +): + """ + Of a video more than half as long, which the track does not run on past + for longer than it lasts: warned of as longer than the video + """ + _video_lasting(monkeypatch, 13 * 3600) + gpx = _write_gpx(tmp_path / "track.gpx", seconds=25 * 3600) + extractor = GPXVideoExtractor(NO_GPS_VIDEO, gpx) + + with caplog.at_level(logging.WARNING): + metadata = extractor.extract() + + assert metadata.points[0].time == 0 + [warning] = extractor.warnings + assert warning.gpx_longer + + +def test_process_takes_a_track_longer_than_a_day_looked_up_at_an_offset( + tmp_path, caplog, monkeypatch +): + """--interpolation_offset_time places the video along the track""" + _video_lasting(monkeypatch, 600) + gpx = _write_gpx(tmp_path / "track.gpx", seconds=86400) + + with caplog.at_level(logging.WARNING): + metadata = GPXVideoExtractor( + NO_GPS_VIDEO, gpx, interpolation=InterpolationOption(offset_time=3600) + ).extract() + + assert metadata.points[0].time == pytest.approx(-3600) + assert "Please align the video start time" not in caplog.text def test_a_track_up_to_a_minute_longer_is_silent(tmp_path, caplog): @@ -174,6 +321,23 @@ def test_the_duration_ratio_spans_the_video_in_sampling(tmp_path, caplog): assert not _warnings(caplog) +def test_the_duration_ratio_fits_a_track_in_sampling(tmp_path, caplog): + """ + At 15 GPS seconds per second, the 5.76 seconds of video span 86 seconds, + which the 120-second track, refused at 1x, runs on past by less than that: + warned of only + """ + gpx = _write_gpx(tmp_path / "track.gpx", seconds=120) + + with caplog.at_level(logging.WARNING): + metadata = GPXVideoExtractor( + NO_GPS_VIDEO, gpx, video_sync=VideoSyncOption(duration_ratio=15) + ).extract() + + assert metadata.points[0].time == 0 + assert not _warnings(caplog) + + def test_a_track_looked_up_at_an_offset_is_silent(tmp_path, caplog): """The offset places the video along the track: it is aligned""" gpx = _write_gpx(tmp_path / "track.gpx", seconds=600) diff --git a/tests/unit/test_gpx_video_sync.py b/tests/unit/test_gpx_video_sync.py index 58146c23..a2b01a34 100644 --- a/tests/unit/test_gpx_video_sync.py +++ b/tests/unit/test_gpx_video_sync.py @@ -99,26 +99,39 @@ def test_the_track_is_placed_by_the_video_start_time(self, tmp_path, caplog): video_start.timestamp() ) - def test_a_track_more_than_a_minute_longer_starts_with_the_video_and_warns( - self, tmp_path, caplog - ): - """The track is taken to be right, and assumed to start with the video.""" - gpx = _write_gpx(tmp_path / "track.gpx", seconds=120) + def test_a_track_far_longer_than_the_video_is_refused(self, tmp_path, caplog): + """The video could start anywhere along a track a minute long.""" + gpx = _write_gpx(tmp_path / "track.gpx", seconds=60) with caplog.at_level(logging.WARNING): - metadata = _extract(NO_GPS_VIDEO, gpx, VideoSyncOption()) + with pytest.raises( + exceptions.MapillaryGPXVideoStartUnknownError + ) as excinfo: + _extract(NO_GPS_VIDEO, gpx, VideoSyncOption()) - assert metadata.points[0].time == 0 - message = caplog.text - assert "Please align the video start time with the GPX track" in message - assert "specify the time the video started as --video_start_time" in message - assert "If the video is a timelapse, specify --video_duration_ratio" in message + message = str(excinfo.value) + assert "lasts 1.0 minutes and the video only 5.76 seconds" in message + assert "so the video could start anywhere along the track" in message + assert "--video_duration_ratio" in message + # For a logger left running, ready to paste: the track starts at GPX_START + assert ( + "started with the video and left running, specify --video_start_time " + "2025_03_14_07_00_00_000, when the track starts" + ) in message + assert "covers more than this video, specify the time the video started" in ( + message + ) + # Which starts the track with the video too, and refuses it alike assert "--interpolation_use_gpx_start_time" not in message + # Refused rather than warned of + assert "Please align the video start time" not in caplog.text def test_a_track_up_to_a_minute_longer_starts_with_the_video_silently( - self, tmp_path, caplog + self, tmp_path, caplog, monkeypatch ): - gpx = _write_gpx(tmp_path / "track.gpx", seconds=60) + """And no more than twice as long as the video, which would be refused""" + monkeypatch.setattr(gpx_extractor, "_video_duration", lambda video_path: 100.0) + gpx = _write_gpx(tmp_path / "track.gpx", seconds=160) with caplog.at_level(logging.WARNING): metadata = _extract(NO_GPS_VIDEO, gpx, VideoSyncOption()) @@ -126,10 +139,30 @@ def test_a_track_up_to_a_minute_longer_starts_with_the_video_silently( assert metadata.points[0].time == 0 assert not caplog.text - def test_a_log_of_days_starts_with_the_video_and_warns(self, tmp_path, caplog): + def test_a_track_up_to_twice_as_long_as_the_video_warns( + self, tmp_path, caplog, monkeypatch + ): + """ + More than a minute longer, and 1.5 times as long: the track is assumed + to start with the video, with a warning to align them + """ + monkeypatch.setattr(gpx_extractor, "_video_duration", lambda video_path: 100.0) + gpx = _write_gpx(tmp_path / "track.gpx", seconds=200) + + with caplog.at_level(logging.WARNING): + metadata = _extract(NO_GPS_VIDEO, gpx, VideoSyncOption()) + + assert metadata.points[0].time == 0 + message = caplog.text + assert "Please align the video start time with the GPX track" in message + assert "specify the time the video started as --video_start_time" in message + assert "If the video is a timelapse, specify --video_duration_ratio" in message + assert "--interpolation_use_gpx_start_time" not in message + + def test_a_log_of_days_offers_no_start_time(self, tmp_path): """ - Nothing but the start of a log of several trips places the video along - it, which the warning asks to align + A track of several days is a log of several trips, and its start that + of the first: placed there, the video would land on the wrong trip. """ gpx = _write_gpx_tracks( tmp_path / "track.gpx", @@ -139,15 +172,54 @@ def test_a_log_of_days_starts_with_the_video_and_warns(self, tmp_path, caplog): ], ) - with caplog.at_level(logging.WARNING): - metadata = _extract(NO_GPS_VIDEO, gpx, VideoSyncOption()) + with pytest.raises(exceptions.MapillaryGPXVideoStartUnknownError) as excinfo: + _extract(NO_GPS_VIDEO, gpx, VideoSyncOption()) - assert metadata.points[0].time == 0 - assert "lasts 2.0 days" in caplog.text - assert "specify the time the video started as --video_start_time" in ( - caplog.text + message = str(excinfo.value) + assert "lasts 2.0 days" in message + assert ( + "A track that long covers more than this video: specify the time the " + "video started as --video_start_time" + ) in message + assert "2025_03_14" not in message + assert "--interpolation_use_gpx_start_time" not in message + + def test_the_video_start_time_places_a_log_of_days(self, tmp_path): + """Which leaves nothing to refuse""" + gpx = _write_gpx_tracks( + tmp_path / "track.gpx", + [ + ["2025-03-14T07:00:00Z", "2025-03-14T07:00:10Z"], + ["2025-03-16T07:00:00Z", "2025-03-16T07:00:10Z"], + ], + ) + video_start = datetime.datetime( + 2025, 3, 16, 7, 0, 2, tzinfo=datetime.timezone.utc ) + metadata = _extract( + NO_GPS_VIDEO, gpx, VideoSyncOption(start_time=video_start.timestamp()) + ) + + assert metadata.points[2].time == pytest.approx(-2) + + def test_the_offset_time_places_a_track_far_longer_than_the_video( + self, tmp_path, caplog + ): + """--interpolation_offset_time places the video along it: not refused""" + gpx = _write_gpx(tmp_path / "track.gpx", seconds=600) + + with caplog.at_level(logging.WARNING): + metadata = _extract( + NO_GPS_VIDEO, + gpx, + VideoSyncOption(), + interpolation=InterpolationOption(offset_time=300), + ) + + assert metadata.points[300].time == pytest.approx(0) + assert "Please align the video start time" not in caplog.text + def test_gpx_start_time_on_a_track_far_longer_than_the_video_is_refused( self, tmp_path ): @@ -256,14 +328,31 @@ def test_the_duration_ratio_spreads_a_track_over_the_video( assert metadata.points[-1].time == pytest.approx(start + 5.7) assert not caplog.text - def test_a_track_too_long_even_at_the_duration_ratio_warns(self, tmp_path, caplog): + def test_a_track_too_long_even_at_the_duration_ratio_is_refused(self, tmp_path): + gpx = _write_gpx(tmp_path / "track.gpx", seconds=600) + + with pytest.raises(exceptions.MapillaryGPXVideoStartUnknownError) as excinfo: + _extract(NO_GPS_VIDEO, gpx, VideoSyncOption(duration_ratio=10)) + + assert "which at --video_duration_ratio 10 span 57.6 seconds" in str( + excinfo.value + ) + + def test_the_duration_ratio_fits_a_track_far_longer_than_the_video( + self, tmp_path, caplog + ): + """ + Refused at 1x, the 600 s track runs on past the 5.76 s video by less + than the 576 s that the video spans at 100x: assumed to start with it + """ gpx = _write_gpx(tmp_path / "track.gpx", seconds=600) with caplog.at_level(logging.WARNING): - metadata = _extract(NO_GPS_VIDEO, gpx, VideoSyncOption(duration_ratio=10)) + metadata = _extract(NO_GPS_VIDEO, gpx, VideoSyncOption(duration_ratio=100)) assert metadata.points[0].time == 0 - assert "which at --video_duration_ratio 10 span 57.6 seconds" in caplog.text + assert metadata.points[100].time == pytest.approx(1) + assert not caplog.text def test_a_track_a_little_longer_than_the_video_is_silent(self, tmp_path, caplog): gpx = _write_gpx(tmp_path / "track.gpx", seconds=9) @@ -298,9 +387,38 @@ def test_process_still_assumes_the_track_starts_with_the_video( assert metadata.points[0].time == 0 assert "--video_duration_ratio" not in caplog.text - def test_a_long_track_geotags_the_video_before_the_next_source(self, tmp_path): - """No longer refused, the GPX geotags the video, with a warning.""" - gpx = _write_gpx(tmp_path / "track.gpx", seconds=120) + def test_process_refuses_a_log_of_days(self, tmp_path): + """ + Which covers more than the video, at whatever pace, and whose start is + that of its first trip + """ + gpx = _write_gpx_tracks( + tmp_path / "track.gpx", + [ + ["2025-03-14T07:00:00Z", "2025-03-14T07:00:01Z"], + ["2025-03-16T07:00:00Z", "2025-03-16T07:00:01Z"], + ], + ) + + with pytest.raises(exceptions.MapillaryGPXVideoStartUnknownError) as excinfo: + GPXVideoExtractor(NO_GPS_VIDEO, gpx).extract() + + message = str(excinfo.value) + assert "lasts 2.0 days and the video only 5.76 seconds" in message + assert ( + "A track that long covers more than this video: trim the GPX to the " + "video, or sample the video with video_process and --video_start_time" + ) in message + # Options of sampling, which process does not take + assert "--video_duration_ratio" not in message + assert "specify --video_start_time" not in message + + def test_the_refusal_is_not_handed_on_to_the_next_source(self, tmp_path): + """ + The video has no GPS, so a native source after the GPX can only fail, + and its error would hide why the GPX was refused. + """ + gpx = _write_gpx(tmp_path / "track.gpx", seconds=60) video_sync = VideoSyncOption() [metadata] = factory.process( @@ -315,8 +433,8 @@ def test_a_long_track_geotags_the_video_before_the_next_source(self, tmp_path): ], ) - assert isinstance(metadata, types.VideoMetadata) - assert metadata.points[0].time == 0 + assert isinstance(metadata, types.ErrorMetadata) + assert isinstance(metadata.error, exceptions.MapillaryGPXVideoStartUnknownError) class TestVideoWithGPS: diff --git a/tests/unit/test_nmea_video.py b/tests/unit/test_nmea_video.py index c4dd1b35..02ffd3ff 100644 --- a/tests/unit/test_nmea_video.py +++ b/tests/unit/test_nmea_video.py @@ -181,8 +181,19 @@ def test_a_malformed_sentence_is_skipped(self, tmp_path): assert nmea_point == pytest.approx(gpx_point, abs=1e-6) def test_messages_name_the_nmea_file(self, tmp_path): + nmea = _write_nmea(tmp_path / "track.nmea", seconds=60) + + with pytest.raises(exceptions.MapillaryGPXVideoStartUnknownError) as excinfo: + NMEAVideoExtractor( + NO_GPS_VIDEO, nmea, video_sync=VideoSyncOption() + ).extract() + + assert str(excinfo.value).startswith(f"The NMEA track in {nmea} lasts") + + def test_warnings_name_the_nmea_file(self, tmp_path): + """process warns of a track far longer than the video, rather than refuse it""" nmea = _write_nmea(tmp_path / "track.nmea", seconds=120) - extractor = NMEAVideoExtractor(NO_GPS_VIDEO, nmea, video_sync=VideoSyncOption()) + extractor = NMEAVideoExtractor(NO_GPS_VIDEO, nmea) extractor.extract() diff --git a/tests/unit/test_sample_video.py b/tests/unit/test_sample_video.py index 9bf4d2c9..3de6f199 100644 --- a/tests/unit/test_sample_video.py +++ b/tests/unit/test_sample_video.py @@ -1185,6 +1185,73 @@ def test_gps_rejected_as_noise_still_has_a_clock(self, tmp_path, monkeypatch): sample_video._check_gpx_start_by_interval(self.VIDEO, options, video_sync) +class TestAssumedGPXStartByInterval: + """ + Without --video_start_time, the interval samples of a video without a GPS + clock are stamped from the start of the GPX track that geotags them, which + is assumed to start with the video, as in distance sampling. A track that + runs on for longer than the whole video is refused, as there. + """ + + VIDEO = TestGPXStartTimeByInterval.VIDEO + GPX_START = datetime.datetime(2025, 3, 14, 7, 0, 0, tzinfo=datetime.timezone.utc) + + @staticmethod + def _start_time( + tmp_path: Path, + seconds: int, + video_sync: VideoSyncOption, + interpolation: InterpolationOption | None = None, + ) -> datetime.datetime | None: + gpx = TestGPXStartTimeByInterval._write_gpx(tmp_path / "track.gpx", seconds) + options = sample_video._parse_geotag_options( + ["gpx"], + gpx, + None, + tmp_path, + video_sync, + interpolation or InterpolationOption(), + ) + return sample_video._track_file_start_time( + TestAssumedGPXStartByInterval.VIDEO, options, video_sync + ) + + def test_a_track_far_longer_than_the_video_is_refused(self, tmp_path, caplog): + with caplog.at_level(logging.WARNING): + with pytest.raises( + exceptions.MapillaryGPXVideoStartUnknownError + ) as excinfo: + self._start_time(tmp_path, 60, VideoSyncOption()) + + message = str(excinfo.value) + assert "so the video could start anywhere along the track" in message + assert "If the video is a timelapse, specify --video_duration_ratio" in message + assert "Please align the video start time" not in caplog.text + + def test_a_track_up_to_twice_as_long_starts_the_samples(self, tmp_path): + """The 5.76-second video, with an 11-second track""" + assert self._start_time(tmp_path, 11, VideoSyncOption()) == self.GPX_START + + def test_the_duration_ratio_fits_the_track(self, tmp_path): + """At 15x, the video spans 86 seconds, past which the track runs on less""" + assert ( + self._start_time(tmp_path, 60, VideoSyncOption(duration_ratio=15)) + == self.GPX_START + ) + + def test_the_offset_time_places_the_video_along_the_track(self, tmp_path): + """The geotagging stage adds the offset to the capture time of each sample""" + assert ( + self._start_time( + tmp_path, + 60, + VideoSyncOption(), + InterpolationOption(offset_time=30), + ) + == self.GPX_START + ) + + class TestGPSClockStartTime: """Tests for _gps_clock_start_time."""