Fix CAMM GPS epoch follow-ups from #828

mapillary_tools/camm/camm_parser.py

@@ -10,6 +10,8 @@
 import dataclasses
 import io
 import logging
+import statistics
+import time
 import typing as T
 from enum import Enum
 
@@ -131,59 +133,139 @@ 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, so converting unconditionally would push those
+# ~10 years into the future. 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 median GPS timestamp of a track has 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 be
+# generous enough to absorb local-time creation times and long recordings
+# without ever being ambiguous.
+_CREATION_TIME_TOLERANCE = 30 * 24 * 3600
 
+# A timestamp this far from the median of its track cannot belong to it: even
+# a time-lapse spans days, not months
+_STRAY_TOLERANCE = 30 * 24 * 3600
 
-def _normalize_gps_epochs(
-    gps: list[telemetry.CAMMGPSPoint], make: str, moov: MovieBoxParser | None = None
-) -> None:
+
+def make_records_gps_time(make: str) -> bool:
     """
-    Rewrite CAMMGPSPoint.epoch_time in place so it is Unix time regardless of
-    which epoch the producer used.
+    Whether cameras of this make record GPS time in CAMM type 6 samples.
+
+    >>> make_records_gps_time("Labpano")
+    True
+    >>> make_records_gps_time("Labpano Technology Co.,Ltd")
+    True
+    >>> make_records_gps_time("Insta360")
+    False
+    """
+    normalized = make.strip().lower()
+    return any(m in normalized for m in _GPS_EPOCH_MAKES)
 
-    This is the only place CAMM GPS timestamps change epoch. Everything
-    downstream, including the serializer, treats them as Unix time.
+
+def _records_gps_time(
+    epoch_time: float, make: str, creation_time: float | None
+) -> bool:
     """
-    if not gps:
-        return
+    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 make decides only
+    when the creation time settles neither, because it is missing or
+    meaningless (a GoPro HERO7 recorded in 2022 reports 2016). Even then, a
+    timestamp that GPS time would put in the future is Unix time: that is what
+    mapillary_tools writes for a Labpano source without a creation time.
+
+    >>> 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", 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
 
-    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)
+    if not make_records_gps_time(make):
+        return False
 
-    first = next((p.epoch_time for p in gps if p.epoch_time > 0), None)
-    if first is None or moov is None:
-        return
+    # Read as GPS time, Unix time lands ten years late, so in the future for
+    # any recording less than ten years old
+    return (
+        telemetry.gps_epoch_to_unix(epoch_time) < time.time() + _CREATION_TIME_TOLERANCE
+    )
+
+
+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
+        return None
 
     if not creation_time:
+        return None
+
+    return creation_time - _MP4_EPOCH_UNIX_OFFSET
+
+
+def _normalize_gps_epochs(
+    gps: list[telemetry.CAMMGPSPoint], make: str, moov: MovieBoxParser | None = None
+) -> None:
+    """
+    Rewrite CAMMGPSPoint.epoch_time in place so it is Unix time regardless of
+    which epoch the producer used, and drop the points whose timestamps are
+    strays.
+
+    This is the only place CAMM GPS timestamps change epoch. Everything
+    downstream, including the serializer, treats them as Unix time.
+    """
+    epoch_times = [p.epoch_time for p in gps if p.epoch_time > 0]
+    if not epoch_times:
         return
 
-    gap = abs(first - (creation_time - _MP4_EPOCH_UNIX_OFFSET))
-    if abs(gap - telemetry.GPS_EPOCH_UNIX_OFFSET) < _GPS_EPOCH_GAP_TOLERANCE:
+    # 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)
+
+    # A stray timestamp would still be wrong by years once converted with the
+    # track, and would corrupt the timestamps interpolated next to it
+    kept = [
+        p
+        for p in gps
+        if p.epoch_time <= 0 or abs(p.epoch_time - median_epoch_time) < _STRAY_TOLERANCE
+    ]
+    if len(kept) < len(gps):
         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,
+            "Dropped %d of %d CAMM GPS points whose timestamps are more than 30 days from the rest of the track",
+            len(gps) - len(kept),
+            len(gps),
         )
+        gps[:] = kept
+
+    if not _records_gps_time(median_epoch_time, make, _extract_creation_time(moov)):
+        return
+
+    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 +387,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,

mapillary_tools/exceptions.py

@@ -72,7 +72,9 @@ class MapillaryStationaryVideoError(MapillaryDescriptionError):
     pass
 
 
-class MapillaryOutsideGPXTrackError(MapillaryDescriptionError):
+# A geotagging error, so that the next geotag source gets its turn: a GPX track
+# that misses the capture time says nothing about the file itself
+class MapillaryOutsideGPXTrackError(MapillaryGeoTaggingError):
     def __init__(
         self, message: str, image_time: str, gpx_start_time: str, gpx_end_time: str
     ):
@@ -81,6 +83,14 @@ def __init__(
         self.gpx_start_time = gpx_start_time
         self.gpx_end_time = gpx_end_time
 
+    def __reduce__(self):
+        # Pickle with every argument, so that the error survives the trip back
+        # from a worker process (video geotagging raises it in one)
+        return (
+            self.__class__,
+            (self.args[0], self.image_time, self.gpx_start_time, self.gpx_end_time),
+        )
+
 
 class MapillaryDuplicationError(MapillaryDescriptionError):
     def __init__(

mapillary_tools/exiftool_read_video.py

@@ -11,7 +11,8 @@
 import typing as T
 import xml.etree.ElementTree as ET
 
-from . import exif_read, exiftool_read, geo
+from . import exif_read, exiftool_read, geo, telemetry
+from .camm import camm_parser
 from .telemetry import GPSFix, GPSPoint
 from .utils import sanitize_serial
 
@@ -51,6 +52,17 @@ def _maybe_float(text: str | None) -> float | None:
         return None
 
 
+def _exiftool_gps_time_to_unix(exiftool_time: float) -> float:
+    """
+    Convert a CAMM GPS timestamp as ExifTool renders it -- GPS time plus the
+    epoch difference, with no leap-second correction -- to Unix time (UTC).
+
+    >>> _exiftool_gps_time_to_unix(1705574461.6)  # 2024-01-18T10:41:01.6
+    1705574443.6
+    """
+    return telemetry.gps_epoch_to_unix(exiftool_time - telemetry.GPS_EPOCH_UNIX_OFFSET)
+
+
 def _index_text_by_tag(elements: T.Iterable[ET.Element]) -> dict[str, list[str]]:
     texts_by_tag: dict[str, list[str]] = {}
     for element in elements:
@@ -550,9 +562,41 @@ def _extract_gps_track_from_track(self) -> list[GPSPoint]:
                     gps_precision_tag=f"{track_ns}:GPSHPositioningError",
                 )
                 if track:
+                    if self._is_camm_track_in_gps_time(track_ns):
+                        LOG.debug(
+                            f"Correcting the CAMM GPS timestamps in {track_ns} by the leap seconds"
+                        )
+                        for point in track:
+                            if point.epoch_time is not None:
+                                point.epoch_time = _exiftool_gps_time_to_unix(
+                                    point.epoch_time
+                                )
                     return track
         return []
 
+    def _is_camm_track_in_gps_time(self, track_ns: str) -> bool:
+        """
+        Whether the track is CAMM from a camera that records GPS time.
+
+        ExifTool converts those timestamps by the epoch difference alone, so
+        they read 18s (the leap seconds since 1980) ahead of what the native
+        CAMM parser returns for the same video.
+
+        Only camera originals qualify. Cameras put their CAMM track under a
+        meta handler, which ExifTool reports as MetaFormat. The CAMM tracks
+        mapillary_tools writes use a camm handler, which ExifTool reports as
+        OtherFormat, and hold Unix time, which ExifTool reads as is.
+        """
+        meta_format = self._extract_alternative_fields([f"{track_ns}:MetaFormat"], str)
+        if (meta_format or "").strip().lower() != "camm":
+            return False
+
+        make = self.extract_make()
+        if not make:
+            return False
+
+        return camm_parser.make_records_gps_time(make)
+
     def _extract_alternative_fields(
         self,
         fields: T.Sequence[str],