Trust the GPS timestamps of a video, never its creation time

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 = [

mapillary_tools/geotag/video_extractors/gpx.py

@@ -13,6 +13,7 @@
 import math
 import statistics
 import sys
+import time
 import typing as T
 from pathlib import Path
 
@@ -25,7 +26,6 @@
     from typing_extensions import override
 
 from ... import constants, exceptions, geo, telemetry, types, utils
-from ...camm import camm_parser
 from ...mp4 import construct_mp4_parser as cparser, simple_mp4_parser as sparser
 from ...mp4.mp4_sample_parser import MovieBoxParser
 from ...serializer.description import build_capture_time
@@ -41,6 +41,21 @@
 # of magnitude.
 _IMPLAUSIBLE_OFFSET_SECONDS = 24 * 3600
 
+# A video GPS timestamp before this is a placeholder, not the GPS time: until it
+# acquires the time, a GoPro stamps 2000-01-01, or a date in October 2015
+_MIN_GPS_UNIX_TIME = datetime.datetime(
+    2016, 1, 1, tzinfo=datetime.timezone.utc
+).timestamp()
+
+# Nor can a video GPS timestamp lie further than this in the future
+_MAX_GPS_FUTURE_SECONDS = 24 * 3600
+
+# A GPS clock that leaps ahead of the video by more than this has just acquired
+# the GPS time, as a GoPro does from its placeholder date: only its timestamps
+# from there on tell when the video was recorded. Stale first timestamps (see
+# _STALE_STEP_RATE) lag by seconds
+_MAX_CLOCK_JUMP_SECONDS = 60.0
+
 # A GPX track that lasts longer than this is not that of one recording, such as
 # a GPS logger left running after the video, but a log of several trips
 _MAX_RECORDING_SECONDS = 24 * 3600
@@ -208,7 +223,7 @@ def extract(self) -> types.VideoMetadata:
         )
 
         try:
-            native_video_metadata, noisy = self._extract_native_video_metadata()
+            native_video_metadata, _ = self._extract_native_video_metadata()
         except exceptions.MapillaryVideoGPSUnusableError as ex:
             # The GPX replaces the video's own GPS, so GPS that is missing,
             # empty or too noisy must not reject the video
@@ -228,16 +243,7 @@ def extract(self) -> types.VideoMetadata:
             )
 
         video_clock = self._read_video_clock(native_video_metadata.points)
-        clock = self._sync_clock(video_clock, gpx_points, noisy)
-        if video_clock is not None and clock is None:
-            LOG.warning(
-                "The GPS clock of %s, whose GPS was rejected as noise, is %.1f days away from %s. "
-                "Syncing the %s from the start of the video instead",
-                self.video_path,
-                abs(gpx_points[0].time - video_clock.to_unix_time(0)) / 86400,
-                self.gpx_path,
-                self.file_format,
-            )
+        clock = self._sync_clock(video_clock, gpx_points)
         if clock is not None and gpx_points and self.sync_mode is SyncMode.REBASE:
             # Distance sampling reads the frames by their composition times:
             # start the track at that of the first frame
@@ -841,14 +847,19 @@ def _sync_clock(
         self,
         clock: VideoClock | None,
         gpx_points: T.Sequence[geo.Point],
-        noisy: bool,
     ) -> VideoClock | None:
         """
         The clock to place the GPX track by along the video, from the GPS
-        clock of the video (None if it has none), whose GPS was rejected as
-        noise if **noisy**. Return None if the track is to be placed as for a
-        video without a GPS clock. --interpolation_use_gpx_start_time starts
-        the track at video time 0.
+        clock of the video (None if it has none). Return None if the track is
+        to be placed as for a video without a GPS clock.
+        --interpolation_use_gpx_start_time starts the track at video time 0.
+
+        The GPS clock places the track whether or not the GPS positions were
+        rejected as noise: its timestamps, which _read_timestamps() keeps only
+        once they are the GPS time, tell when the video was recorded, and a
+        track recorded at another time misses the video. The camera's own
+        clock, such as the creation time of the video, is never consulted:
+        it can be anything.
         """
         if clock is None or not gpx_points:
             return clock
@@ -861,40 +872,8 @@ def _sync_clock(
                 video_time=0.0, unix_time=gpx_points[0].time, rate=clock.rate
             )
 
-        # GPS rejected as noise may not have GPS time yet: GoPro stamps a
-        # placeholder date until it acquires it. That GPS is what the GPX
-        # replaces, so do not let a placeholder clock place the GPX days away
-        # from the video. A clock that agrees with the creation time of the
-        # video is no placeholder: it places the GPX as for any video with GPS,
-        # and a GPX days away from it misses the video
-        offset = gpx_points[0].time - clock.to_unix_time(0)
-        if (
-            abs(offset) > _IMPLAUSIBLE_OFFSET_SECONDS
-            and noisy
-            and self.sync_mode is SyncMode.SYNC
-            and not self._clock_matches_creation_time(clock)
-        ):
-            return None
-
         return clock
 
-    def _clock_matches_creation_time(self, clock: VideoClock) -> bool:
-        """
-        Whether the GPS clock of the video starts within a day of the mvhd
-        creation time of the video, or False if that cannot be read.
-
-        A day absorbs any time zone the creation time is written in (GoPro
-        writes local time), and whether it marks the start or the end of the
-        recording. Some cameras write a creation time that means nothing (a
-        GoPro HERO10 recorded in 2022 reports 2016-01-01), so a clock that does
-        not match it may still be real.
-        """
-        creation_time = _video_creation_time(self.video_path)
-        if creation_time is None:
-            return False
-        gap = abs(clock.to_unix_time(0) - creation_time)
-        return gap <= _IMPLAUSIBLE_OFFSET_SECONDS
-
     def _warn_of_a_long_track(self, gpx_points: T.Sequence[geo.Point]) -> None:
         """
         Warn, in process, of a GPX track assumed to start with the video that
@@ -1027,17 +1006,22 @@ def _read_timestamps(
     ) -> list[tuple[float, float]]:
         """
         Read the (video time, Unix time) pairs of the GPS timestamps of the
-        points, leaving out those that the others show to be wrong: those out
-        of order, those before a jump of days (from a placeholder date to the
-        GPS time), and stale first ones
+        points, leaving out those that cannot be the GPS time (zero, before
+        2016, or in the future), and those that the others show to be wrong:
+        those out of order, those before a leap of more than a minute (from a
+        placeholder date to the GPS time), and stale first ones.
+
+        Only the timestamps are judged, not the positions: a GPS can have the
+        time before it has a position good enough to publish.
 
         >>> points = [
-        ...     telemetry.GPSPoint(time=v, lat=1, lon=1, alt=None, angle=None, epoch_time=u, fix=None, precision=None, ground_speed=None)
-        ...     for v, u in [(0, 992.4), (0.4, 1000.4), (1.4, 1001.4), (1.8, 1001.4), (2.4, 1002.4)]
+        ...     telemetry.GPSPoint(time=v, lat=1, lon=1, alt=None, angle=None, epoch_time=1789141180 + u, fix=None, precision=None, ground_speed=None)
+        ...     for v, u in [(0, -7.6), (0.4, 0.4), (1.4, 1.4), (1.8, 1.4), (2.4, 2.4)]
         ... ]
-        >>> GPXVideoExtractor._read_timestamps(points)
-        [(0.4, 1000.4), (1.4, 1001.4), (2.4, 1002.4)]
+        >>> [(v, round(u - 1789141180, 1)) for v, u in GPXVideoExtractor._read_timestamps(points)]
+        [(0.4, 0.4), (1.4, 1.4), (2.4, 2.4)]
         """
+        latest = time.time() + _MAX_GPS_FUTURE_SECONDS
         pairs: list[tuple[float, float]] = []
         for p in video_gps_points:
             # Video GPS timestamps are stored in whatever epoch their container
@@ -1046,6 +1030,11 @@ def _read_timestamps(
             unix_time = p.get_unix_time()
             if unix_time is None:
                 continue
+            if not _MIN_GPS_UNIX_TIME <= unix_time <= latest:
+                # A placeholder date, or a clock gone wrong
+                continue
+            if not math.isfinite(p.time):
+                continue
             if pairs and unix_time == pairs[-1][1]:
                 # GoPro GPS5 stamps a whole block (~1s) of points with one
                 # timestamp. Keep the first point of the block
@@ -1062,11 +1051,13 @@ def _read_timestamps(
         if pace is None:
             return pairs
 
-        # GoPro stamps a placeholder date until the GPS acquires the time
+        # Until it acquires the GPS time, a GoPro stamps a placeholder date,
+        # which can be one since 2016: keep the timestamps after the last leap
+        # (see _MAX_CLOCK_JUMP_SECONDS)
         start = 0
         for idx in range(1, len(pairs)):
             (v0, u0), (v1, u1) = pairs[idx - 1], pairs[idx]
-            if _IMPLAUSIBLE_OFFSET_SECONDS < (u1 - u0) - (v1 - v0) * pace:
+            if _MAX_CLOCK_JUMP_SECONDS < (u1 - u0) - (v1 - v0) * pace:
                 start = idx
 
         # Stale first timestamps (see _STALE_STEP_RATE). Judged by the steps of
@@ -1356,19 +1347,6 @@ def _first_frame_time(video_path: Path) -> float:
         return 0.0
 
 
-def _video_creation_time(video_path: Path) -> float | None:
-    """
-    The creation time of the video as Unix time, from its mvhd box, or None if
-    it is unset or cannot be read.
-    """
-    try:
-        moov = MovieBoxParser.parse_file(video_path)
-    except (OSError, sparser.ParsingError, C.ConstructError) as ex:
-        LOG.debug("Failed to read the creation time of %s: %s", video_path, ex)
-        return None
-    return camm_parser._extract_creation_time(moov)
-
-
 def _format_duration(seconds: float) -> str:
     """
     Format a duration in the largest unit it fills, rounded rather than

mapillary_tools/sample_video.py

@@ -11,7 +11,6 @@
 import os
 import re
 import shutil
-import statistics
 import time
 import typing as T
 from contextlib import contextmanager
@@ -457,21 +456,6 @@ def wip_sample_dir(sample_dir: Path) -> Path:
     )
 
 
-# GPS clocks that report a time before this have never been set: GoPro writes
-# 2000-01-01 until it gets its first fix, for example
-_MIN_PLAUSIBLE_START_TIME = datetime.datetime(
-    2010, 1, 1, tzinfo=datetime.timezone.utc
-).timestamp()
-
-# Leave room for the clock of the machine running this to be behind
-_MAX_FUTURE_START_TIME_SECONDS = 24 * 3600
-
-# How many GPS timestamps to take the median of, so that a single bad one cannot
-# shift the start time: the Labpano PanoX V2 can record a stale first fix,
-# seconds older than the rest. Only the first few are used because in timelapses
-# the video clock runs slower than the GPS clock, so the two drift apart
-_GPS_CLOCK_SAMPLES = 5
-
 # Cameras known to stamp the creation time at the end of the recording, as
 # lowercase (make, model) from the container tags. BlackVue is not listed
 # because it does not write those tags: blackvue_parser.is_blackvue detects it
@@ -499,32 +483,17 @@ def _gps_clock_start_time(
     points: T.Sequence[geo.Point],
 ) -> datetime.datetime | None:
     """
-    Map the absolute GPS timestamps at the start of a track back to the video's time 0.
-
-    Point times are relative to the start of the video, so subtracting one from
-    its own absolute timestamp gives the wall clock at which the video started.
-    Timestamps outside the plausible range are skipped.
+    The wall clock time at which the video started, by its GPS clock, or None
+    if it has none: by the timestamps that GPX sync reads the clock from (see
+    GPXVideoExtractor._read_timestamps), at the clock's pace, which a timelapse
+    packs several GPS seconds into each second of video at
     """
-    max_start_time = time.time() + _MAX_FUTURE_START_TIME_SECONDS
-
-    start_times: list[float] = []
-    for point in points:
-        unix_time = point.get_unix_time()
-        if unix_time is None:
-            continue
-        start_time = unix_time - point.time
-        # Written as a negated range check so that NaN is skipped too
-        if not (_MIN_PLAUSIBLE_START_TIME <= start_time <= max_start_time):
-            continue
-        start_times.append(start_time)
-        if len(start_times) >= _GPS_CLOCK_SAMPLES:
-            break
-
-    if not start_times:
+    clock = GPXVideoExtractor._read_video_clock(points)
+    if clock is None:
         return None
 
     return datetime.datetime.fromtimestamp(
-        statistics.median(start_times), tz=datetime.timezone.utc
+        clock.to_unix_time(0.0), tz=datetime.timezone.utc
     )
 
 
@@ -787,7 +756,7 @@ def _gpx_sync_clock(
         if not noisy:
             return None
         video_clock = extractor._read_video_clock(native_video_metadata.points)
-        return extractor._sync_clock(video_clock, gpx_points, noisy)
+        return extractor._sync_clock(video_clock, gpx_points)
 
     return None