diff --git a/mapillary_tools/camm/camm_parser.py b/mapillary_tools/camm/camm_parser.py index b5f42049..ee4662e0 100644 --- a/mapillary_tools/camm/camm_parser.py +++ b/mapillary_tools/camm/camm_parser.py @@ -10,6 +10,7 @@ import dataclasses import io import logging +import statistics import typing as T from enum import Enum @@ -131,19 +132,71 @@ def extract_camm_info(fp: T.BinaryIO, telemetry_only: bool = False) -> CAMMInfo # Makes whose CAMM type 6 samples record GPS time (seconds since 1980-01-06), -# as the CAMM spec describes. Everything else -- Insta360, and the CAMM tracks -# mapillary_tools writes itself -- records Unix time in the same field, so -# converting unconditionally would push those ~10 years into the future. -_GPS_EPOCH_MAKES = frozenset(["labpano"]) +# as the CAMM spec describes. Everything else -- Insta360, for one -- records +# Unix time in the same field. Matched as a substring of the lowercased make, +# because firmware reports the same vendor in more than one form. +_GPS_EPOCH_MAKES = ("labpano",) # Seconds between the mp4 epoch (1904-01-01) and the Unix epoch. _MP4_EPOCH_UNIX_OFFSET = 2082844800 -# Tolerance for recognizing a gap as "off by exactly one GPS epoch". Checking -# for that specific distance rather than for general implausibility matters: -# some cameras write a meaningless mvhd creation_time (a GoPro HERO7 recorded -# in 2022 reports 2016), so a generic bound would fire constantly. -_GPS_EPOCH_GAP_TOLERANCE = 30 * 24 * 3600 +# How close the timestamps of a track have to be to the mvhd creation_time, +# read either as Unix time or as GPS time, for the creation time to decide the +# epoch. The two readings are ten years apart, so this can absorb local-time +# creation times and long recordings without ever being ambiguous. +_CREATION_TIME_TOLERANCE = 30 * 24 * 3600 + + +def _records_gps_time( + epoch_time: float, make: str, creation_time: float | None +) -> bool: + """ + Decide the epoch of raw CAMM type 6 timestamps, given one representative + of them. + + The creation time of the video decides when it can: GPS time reads as one + GPS epoch before it, Unix time reads close to it. The CAMM tracks + mapillary_tools writes hold Unix time but copy the make and the creation + time of their source, so for a Labpano video it is the creation time that + tells our output from the camera original. The make decides only when the + creation time is missing or meaningless (a GoPro HERO7 recorded in 2022 + reports 2016). + + >>> creation_time = 1705574637 # 2024-01-18T10:43:57Z + >>> _records_gps_time(1389609661, "", creation_time) # GPS time + True + >>> _records_gps_time(1705574443, "Labpano", creation_time) # Unix time + False + >>> _records_gps_time(1389609661, "Labpano Technology Co.,Ltd", None) + True + >>> _records_gps_time(1705574443, "Insta360", None) + False + """ + if creation_time: + gap = creation_time - epoch_time + if abs(gap - telemetry.GPS_EPOCH_UNIX_OFFSET) < _CREATION_TIME_TOLERANCE: + return True + if abs(gap) < _CREATION_TIME_TOLERANCE: + return False + + normalized = make.strip().lower() + return any(m in normalized for m in _GPS_EPOCH_MAKES) + + +def _extract_creation_time(moov: MovieBoxParser | None) -> float | None: + """Return the mvhd creation_time as Unix time, or None if unset.""" + if moov is None: + return None + + try: + creation_time = moov.extract_mvhd_boxdata().get("creation_time", 0) + except Exception: + return None + + if not creation_time: + return None + + return creation_time - _MP4_EPOCH_UNIX_OFFSET def _normalize_gps_epochs( @@ -156,34 +209,19 @@ def _normalize_gps_epochs( This is the only place CAMM GPS timestamps change epoch. Everything downstream, including the serializer, treats them as Unix time. """ - if not gps: - return - - if make.strip().lower() in _GPS_EPOCH_MAKES: - for point in gps: - if point.epoch_time > 0: - point.epoch_time = telemetry.gps_epoch_to_unix(point.epoch_time) - - first = next((p.epoch_time for p in gps if p.epoch_time > 0), None) - if first is None or moov is None: + epoch_times = [p.epoch_time for p in gps if p.epoch_time > 0] + if not epoch_times: return - try: - creation_time = moov.extract_mvhd_boxdata().get("creation_time", 0) - except Exception: + # Decide by the median rather than by any one sample, so that a stray + # timestamp cannot flip the epoch of the whole track + median_epoch_time = statistics.median(epoch_times) + if not _records_gps_time(median_epoch_time, make, _extract_creation_time(moov)): return - if not creation_time: - return - - gap = abs(first - (creation_time - _MP4_EPOCH_UNIX_OFFSET)) - if abs(gap - telemetry.GPS_EPOCH_UNIX_OFFSET) < _GPS_EPOCH_GAP_TOLERANCE: - LOG.warning( - "CAMM GPS timestamps are one GPS epoch away from the creation time " - "of the video. The camera (make %r) may record GPS time where Unix " - "time is expected, or the reverse; please report this video", - make, - ) + for point in gps: + if point.epoch_time > 0: + point.epoch_time = telemetry.gps_epoch_to_unix(point.epoch_time) def extract_camera_make_and_model(fp: T.BinaryIO) -> tuple[str, str]: @@ -305,9 +343,10 @@ def serialize(cls, data: telemetry.CAMMGPSPoint) -> bytes: { "type": cls.serialized_camm_type.value, "data": { - # Written as Unix time, which is what every released - # version of mapillary_tools has written and what readers - # of our output expect. Do not convert here. + # Written as Unix time, whatever the make. Readers tell it + # from the GPS time some cameras record by the creation + # time the file carries (see _records_gps_time). Do not + # convert here. "time_gps_epoch": data.epoch_time, "gps_fix_type": data.gps_fix_type, "latitude": data.lat, diff --git a/mapillary_tools/geotag/video_extractors/native.py b/mapillary_tools/geotag/video_extractors/native.py index 272602e2..f3a197c6 100644 --- a/mapillary_tools/geotag/video_extractors/native.py +++ b/mapillary_tools/geotag/video_extractors/native.py @@ -78,11 +78,19 @@ def extract(self) -> types.VideoMetadata: if not camm_info.gps and not camm_info.mini_gps: raise exceptions.MapillaryGPXEmptyError("Empty GPS data found") + # A track may mix type 6 and type 5 samples, so use both. No camera is + # known to interleave them, and it would cost the absolute timestamps + # of sampled frames: interpolating between the two types returns a + # plain geo.Point (see CAMMGPSPoint.interpolate_with), so sample_video + # falls back to the container start time. + points: list[geo.Point] = [*(camm_info.gps or []), *(camm_info.mini_gps or [])] + points.sort(key=lambda p: p.time) + return types.VideoMetadata( filename=self.video_path, filesize=utils.get_file_size(self.video_path), filetype=types.FileType.CAMM, - points=T.cast(T.List[geo.Point], camm_info.gps or camm_info.mini_gps), + points=points, make=camm_info.make, model=camm_info.model, ) diff --git a/tests/cli/camm_parser.py b/tests/cli/camm_parser.py index 1acd0155..ff8f7ed8 100644 --- a/tests/cli/camm_parser.py +++ b/tests/cli/camm_parser.py @@ -44,7 +44,9 @@ def _convert(path: pathlib.Path): track.description = "Invalid CAMM video" return track - points = T.cast(T.List[geo.Point], camm_info.gps or camm_info.mini_gps) + # A track may mix type 6 and type 5 samples, so use both + points: T.List[geo.Point] = [*(camm_info.gps or []), *(camm_info.mini_gps or [])] + points.sort(key=lambda p: p.time) track.segments.append(_convert_points_to_gpx_segment(points)) make_model = json.dumps({"make": camm_info.make, "model": camm_info.model}) diff --git a/tests/unit/test_camm_parser.py b/tests/unit/test_camm_parser.py index 112a1df5..04f0fea9 100644 --- a/tests/unit/test_camm_parser.py +++ b/tests/unit/test_camm_parser.py @@ -802,10 +802,9 @@ def test_extract_camm_info_routes_plain_points_to_mini_gps(): def test_camm_gps_timestamps_round_trip_as_unix(): """process -> build CAMM -> re-read must return the input timestamps. - mapillary_tools has always written Unix time into the CAMM type 6 - time_gps_epoch field, and released versions read it back as Unix time. - Writing anything else would make our output unreadable by them, so the - serializer must not convert. + mapillary_tools writes Unix time into the CAMM type 6 time_gps_epoch + field whatever the make, so the serializer must not convert. Makes that + record GPS time are covered in test_gps_epoch.py. """ unix_times = [1655503450.5, 1655503451.5] points = [ diff --git a/tests/unit/test_gps_epoch.py b/tests/unit/test_gps_epoch.py index f0bc4075..528d702c 100644 --- a/tests/unit/test_gps_epoch.py +++ b/tests/unit/test_gps_epoch.py @@ -12,22 +12,29 @@ is a ~315,964,800s (10 year) error. The invariant these tests protect: ``CAMMGPSPoint.epoch_time`` is *always* -Unix time in memory. The conversion happens exactly once, when the CAMM track -is parsed, and never again -- in particular not in the serializer, which must -keep writing Unix time so files stay readable by released versions. +Unix time in memory. It is converted from GPS time once, when the CAMM track is +parsed, and never on the way out: mapillary_tools writes Unix time whatever the +make. + +Which epoch a track records is decided by the mvhd creation time of the video +when that is conclusive, and by the make otherwise. The CAMM tracks +mapillary_tools writes carry the make and the creation time of their source, +so for a Labpano video it is the creation time that says Unix time. """ from __future__ import annotations +import io +import typing as T from pathlib import Path import pytest - -from mapillary_tools import geo, telemetry +from mapillary_tools import geo, telemetry, types, uploader from mapillary_tools.camm import camm_builder, camm_parser from mapillary_tools.geotag.options import SourceOption, SourceType from mapillary_tools.geotag.video_extractors.gpx import GPXVideoExtractor -from mapillary_tools.mp4 import construct_mp4_parser as cparser +from mapillary_tools.geotag.video_extractors.native import CAMMVideoExtractor +from mapillary_tools.mp4 import construct_mp4_parser as cparser, simple_mp4_builder # Seconds between the two epochs, i.e. the size of the bug @@ -37,6 +44,12 @@ A_GPS_TIME = 1470558405.9798455 A_UNIX_TIME = 1786523187.9798455 +# Seconds between the mp4 epoch (1904-01-01) and the Unix epoch +MP4_UNIX_DELTA = 2082844800 + +# Labpano stamps the creation time at the end of the recording +A_CREATION_TIME = A_UNIX_TIME + 600 + def _camm_point(time: float, epoch_time: float) -> telemetry.CAMMGPSPoint: return telemetry.CAMMGPSPoint( @@ -70,6 +83,67 @@ def _gps_point(time: float, epoch_time: float | None) -> telemetry.GPSPoint: ) +def _write_camm_mp4( + points: T.Sequence[geo.Point], make: str, creation_time: float | None +) -> bytes: + """ + Write points as a CAMM track into an empty mp4, the way the uploader does. + + creation_time is the Unix time to put in mvhd, or None to leave it unset. + """ + mp4_creation_time = ( + 0 if creation_time is None else int(creation_time) + MP4_UNIX_DELTA + ) + mvhd: cparser.BoxDict = { + "type": b"mvhd", + "data": { + "creation_time": mp4_creation_time, + "modification_time": mp4_creation_time, + "timescale": 1000, + "duration": 10000, + }, + } + src = cparser.MP4WithoutSTBLBuilderConstruct.build_boxlist( + [ + {"type": b"ftyp", "data": b"test"}, + {"type": b"moov", "data": [mvhd]}, + ] + ) + metadata = types.VideoMetadata( + Path(""), + filetype=types.FileType.CAMM, + points=list(points), + make=make, + model="PanoX V2", + ) + camm_info = uploader.VideoUploader.prepare_camm_info(metadata) + target_fp = simple_mp4_builder.transform_mp4( + io.BytesIO(src), camm_builder.camm_sample_generator2(camm_info) + ) + return target_fp.read() + + +def _read_camm(data: bytes) -> camm_parser.CAMMInfo: + camm_info = camm_parser.extract_camm_info(io.BytesIO(data)) + assert camm_info is not None + return camm_info + + +def _unix_times(camm_info: camm_parser.CAMMInfo) -> list[float]: + return [p.epoch_time for p in camm_info.gps or []] + + +# A two point track at A_UNIX_TIME, as parse_gpx() or the parser would return it +UNIX_TIMES = [A_UNIX_TIME, A_UNIX_TIME + 1] + + +def _unix_track() -> list[telemetry.CAMMGPSPoint]: + return [ + _camm_point(time=float(idx), epoch_time=epoch_time) + for idx, epoch_time in enumerate(UNIX_TIMES) + ] + + class TestEpochConversion: def test_known_instant(self): assert telemetry.gps_epoch_to_unix(A_GPS_TIME) == A_UNIX_TIME @@ -112,6 +186,141 @@ def test_invalid_timestamps_are_not_converted(self): assert points[0].epoch_time == 0.0 assert points[0].get_unix_time() is None + @pytest.mark.parametrize( + "make", ["Labpano Technology Co.,Ltd", "LABPANO TECHNOLOGY", "Labpano Pilot"] + ) + def test_make_variants_are_matched(self, make: str): + """Firmware reports the vendor in more than one form.""" + points = [_camm_point(time=0.0, epoch_time=A_GPS_TIME)] + camm_parser._normalize_gps_epochs(points, make) + assert points[0].epoch_time == A_UNIX_TIME + + +class TestCreationTimeEvidence: + """The creation time decides the epoch whenever it is conclusive.""" + + @pytest.mark.parametrize("make", ["", "Insta360", "Some Future Camera"]) + def test_gps_time_is_recognized_whatever_the_make(self, make: str): + assert camm_parser._records_gps_time(A_GPS_TIME, make, A_CREATION_TIME) + + @pytest.mark.parametrize("make", ["Labpano", "Labpano Technology Co.,Ltd"]) + def test_unix_time_is_recognized_whatever_the_make(self, make: str): + assert not camm_parser._records_gps_time(A_UNIX_TIME, make, A_CREATION_TIME) + + def test_inconclusive_creation_time_falls_back_to_make(self): + # A GoPro HERO7 recorded in 2022 reports 2016 + meaningless = A_UNIX_TIME - 6 * 365 * 24 * 3600 + assert camm_parser._records_gps_time(A_GPS_TIME, "Labpano", meaningless) + assert not camm_parser._records_gps_time(A_UNIX_TIME, "Insta360", meaningless) + + def test_missing_creation_time_falls_back_to_make(self): + assert camm_parser._records_gps_time(A_GPS_TIME, "Labpano", None) + assert not camm_parser._records_gps_time(A_UNIX_TIME, "Insta360", None) + + def test_unknown_make_in_gps_time_is_read_from_the_file(self): + """The mvhd creation time reaches the decision through the parser.""" + data = _write_camm_mp4( + [_camm_point(time=0.0, epoch_time=A_GPS_TIME)], + make="", + creation_time=A_CREATION_TIME, + ) + assert _unix_times(_read_camm(data)) == [A_UNIX_TIME] + + def test_labpano_original_without_creation_time_is_converted(self): + data = _write_camm_mp4( + [_camm_point(time=0.0, epoch_time=A_GPS_TIME)], "Labpano", None + ) + assert _unix_times(_read_camm(data)) == [A_UNIX_TIME] + + +class TestMedianDecides: + """The whole track decides its epoch, not its first timestamp.""" + + def _read_back(self, raw_times: list[float]) -> list[float]: + points = [ + _camm_point(time=float(idx), epoch_time=raw_time) + for idx, raw_time in enumerate(raw_times) + ] + # Written as is, so the raw times are what the reader sees + data = _write_camm_mp4(points, "Labpano", A_CREATION_TIME) + return _unix_times(_read_camm(data)) + + def test_stray_unix_time_does_not_flip_a_gps_time_track(self): + raw_times = [A_UNIX_TIME] + [A_GPS_TIME + t for t in range(1, 10)] + assert self._read_back(raw_times)[1:] == [A_UNIX_TIME + t for t in range(1, 10)] + + def test_stray_gps_time_does_not_flip_a_unix_time_track(self): + raw_times = [A_GPS_TIME] + [A_UNIX_TIME + t for t in range(1, 10)] + assert self._read_back(raw_times)[1:] == [A_UNIX_TIME + t for t in range(1, 10)] + + +class TestWriteRoundTrip: + """ + Whatever mapillary_tools writes must read back to the same Unix times. + + It writes Unix time whatever the make. The source make is copied into the + output, so for a Labpano video it is the creation time, copied from the + source too, that keeps the reader from converting it one GPS epoch later: + 2026 used to become 2036. + """ + + @pytest.mark.parametrize( + "make", ["Labpano", "Labpano Technology Co.,Ltd", "Insta360", ""] + ) + def test_round_trip(self, make: str): + data = _write_camm_mp4(_unix_track(), make, A_CREATION_TIME) + assert _unix_times(_read_camm(data)) == UNIX_TIMES + + def test_reprocessing_output_is_stable(self): + """process_and_upload output, processed again, is the same track.""" + data = _write_camm_mp4(_unix_track(), "Labpano", A_CREATION_TIME) + for _ in range(2): + camm_info = _read_camm(data) + assert _unix_times(camm_info) == UNIX_TIMES + data = _write_camm_mp4(camm_info.gps or [], camm_info.make, A_CREATION_TIME) + + @pytest.mark.parametrize("make", ["Labpano", "Insta360", ""]) + def test_written_as_unix_time_whatever_the_make(self, monkeypatch, make: str): + data = _write_camm_mp4(_unix_track(), make, A_CREATION_TIME) + monkeypatch.setattr(camm_parser, "_normalize_gps_epochs", lambda *_: None) + assert _unix_times(_read_camm(data)) == UNIX_TIMES + + +class TestMixedCAMMTypes: + """A track may mix type 6 and type 5 samples; neither may be dropped.""" + + def _mixed_track(self) -> list[geo.Point]: + return [ + geo.Point(time=0.0, lat=37.0, lon=14.0, alt=None, angle=None), + _camm_point(time=1.0, epoch_time=A_UNIX_TIME + 1), + geo.Point(time=2.0, lat=37.0, lon=14.0, alt=None, angle=None), + _camm_point(time=3.0, epoch_time=A_UNIX_TIME + 3), + ] + + def test_extractor_returns_both_types_in_order(self, tmp_path: Path): + video_path = tmp_path / "mixed.mp4" + video_path.write_bytes( + _write_camm_mp4(self._mixed_track(), "Labpano", A_CREATION_TIME) + ) + + points = CAMMVideoExtractor(video_path).extract().points + + assert [p.time for p in points] == [0.0, 1.0, 2.0, 3.0] + assert [type(p) for p in points] == [ + geo.Point, + telemetry.CAMMGPSPoint, + geo.Point, + telemetry.CAMMGPSPoint, + ] + + def test_gpx_clock_anchors_on_the_first_timestamped_point(self): + clock = GPXVideoExtractor._read_video_clock(self._mixed_track()) + # Anchored at the type 6 point 1s into the video, which is at + # A_UNIX_TIME + 1, not at the type 5 point the track starts with + assert clock is not None + # So a GPX point recorded 30s before the video starts lands at -30s + assert clock.to_video_time(A_UNIX_TIME - 30) == -30.0 + class TestPointAccessors: def test_both_point_types_report_unix_time(self):