Offer the start of a long GPX track instead of a flag that refuses it

README.md

@@ -327,7 +327,10 @@ mapillary_tools upload MY_SAMPLES --desc_path=/tmp/mapillary_description.json
 If you use external GPS devices for mapping, you will need to geotag your captures with the external GPS tracks.
 
 To geotag images with a GPX file, the capture time (extracted from EXIF tag "Date/Time Original" or "GPS Date/Time") is minimally required.
-It is used to locate the images along the GPS tracks.
+It is used to locate the images along the GPS tracks. Images captured before the track starts, after it ends, or in a
+gap of more than an hour between two of its points (such as between two trips in one GPX file) fall outside the track,
+and are not geotagged from it. The same applies to videos. To allow longer gaps, set the environment variable
+`MAPILLARY_TOOLS_MAX_GPX_GAP_SECONDS` (3600 by default).
 
 ```sh
 mapillary_tools process MY_IMAGE_DIR --geotag_source "gpx" --geotag_source_path MY_EXTERNAL_GPS.gpx
@@ -462,6 +465,8 @@ with a GPX file called `foo.gpx`.
 ```sh
 mapillary_tools process --video_geotag_source gpx VIDEO_DIR/
 ```
+NMEA files work the same way with `--video_geotag_source nmea`, which associates `foo.mp4` with `foo.nmea`. What
+follows about GPX tracks applies to NMEA tracks too.
 
 #### Insta360 stitched videos
 

mapillary_tools/commands/video_process.py

@@ -6,8 +6,10 @@
 from __future__ import annotations
 
 import logging
+from pathlib import Path
 
-from .. import exceptions
+from .. import constants, exceptions
+from ..sample_video import find_sample_dirs
 from ..types import FileType
 from .process import Command as ProcessCommand
 from .sample_video import Command as SampleCommand
@@ -43,7 +45,39 @@ def run(self, args: dict):
             )
             args[option] = {FileType.IMAGE}
 
+        if args["skip_subfolders"]:
+            _add_sample_dirs(args)
+
         ProcessCommand().run(args)
 
         if sample_error is not None:
             raise sample_error
+
+
+def _add_sample_dirs(args: dict) -> None:
+    """
+    Add the folders that the videos were sampled into to the import paths.
+    --skip_subfolders chose the videos to sample, and hides the subfolders of
+    the import path from processing (and from uploading in
+    video_process_and_upload), which the samples of each video are in
+    """
+    import_path: Path = args["import_path"]
+    sample_dirs = [
+        sample_dir
+        for sample_dir in find_sample_dirs(
+            args["video_import_path"], import_path, skip_subfolders=True
+        )
+        # Not a video that failed to sample
+        if sample_dir.is_dir()
+    ]
+    if not sample_dirs:
+        return
+
+    if args.get("desc_path") is None:
+        # Where process writes it for the import path alone, rather than to
+        # STDOUT, as for several import paths
+        args["desc_path"] = str(
+            import_path.joinpath(constants.IMAGE_DESCRIPTION_FILENAME)
+        )
+
+    args["import_path"] = [import_path, *sample_dirs]

mapillary_tools/constants.py

@@ -82,6 +82,11 @@ def _parse_scaled_integers(
 ###################
 USER_DATA_DIR = appdirs.user_data_dir(appname="mapillary_tools", appauthor="Mapillary")
 PROMPT_DISABLED: bool = _yes_or_no(os.getenv(_ENV_PREFIX + "PROMPT_DISABLED", "NO"))
+# In seconds. Consecutive points of a GPX or NMEA track further apart than
+# this leave a gap that no position is interpolated across, such as a GPS
+# logger switched off between trips: the time inside it is outside the track,
+# like the time before its start or after its end
+MAX_GPX_GAP_SECONDS = float(os.getenv(_ENV_PREFIX + "MAX_GPX_GAP_SECONDS", 3600))
 
 
 ############################

mapillary_tools/exif_read.py

@@ -45,6 +45,18 @@ def eval_frac(value: Ratio) -> float:
 
 
 def gps_to_decimal(values: tuple[Ratio, Ratio, Ratio]) -> float | None:
+    """
+    Convert degrees, minutes and seconds to decimal degrees, or None if they
+    are not a coordinate: a GPS IFD that a camera wrote at bad offsets reads
+    as minutes or seconds of more than 60 (60 itself may be 59.996 rounded)
+
+    >>> gps_to_decimal((Ratio(10, 1), Ratio(22, 1), Ratio(30, 1)))
+    10.375
+    >>> round(gps_to_decimal((Ratio(10, 1), Ratio(22, 1), Ratio(60, 1))), 6)
+    10.383333
+    >>> gps_to_decimal((Ratio(0, 1), Ratio(1, 4), Ratio(381, 5))) is None
+    True
+    """
     try:
         deg, min, sec, *_ = values
     except (TypeError, ValueError):
@@ -61,6 +73,8 @@ def gps_to_decimal(values: tuple[Ratio, Ratio, Ratio]) -> float | None:
         seconds = eval_frac(sec)
     except ZeroDivisionError:
         return None
+    if not (0 <= minutes <= 60 and 0 <= seconds <= 60):
+        return None
     return degrees + minutes / 60 + seconds / 3600
 
 

mapillary_tools/exiftool_read_video.py

@@ -30,6 +30,7 @@
     "XMP-aux": "http://ns.exiftool.org/XMP/XMP-aux/1.0/",
     "DJI": "http://ns.exiftool.org/MakerNotes/DJI/1.0/",
     "XMP-drone-dji": "http://ns.exiftool.org/XMP/XMP-drone-dji/1.0/",
+    "LIGO": "http://ns.exiftool.org/QuickTime/LIGO/1.0/",
     **{
         f"Track{track_id}": f"http://ns.exiftool.org/QuickTime/Track{track_id}/1.0/"
         for track_id in range(1, MAX_TRACK_ID + 1)
@@ -310,6 +311,121 @@ def _aggregate_gps_track(
     return track
 
 
+def _group_gps_records(
+    elements: T.Iterable[ET.Element],
+    tags: T.Collection[str],
+    record_start_tag: str | None = None,
+    first_tag: str | None = None,
+) -> T.Generator[dict[str, str], None, None]:
+    """
+    Group the tags that ExifTool lists for one GPS record after another, one
+    dict per record from tag (expanded) to text. A record starts at
+    **record_start_tag**, at **first_tag**, which ExifTool lists first in a
+    record that has it, or at a tag that the record so far already has.
+
+    A tag that a record does not have is left out of its dict, rather than
+    taken from a later record: a BlackVue writes no GPSTrack while stationary.
+
+    >>> elements = []
+    >>> for tag, text in [
+    ...     ("SampleTime", "0"), ("GPSLatitude", "1"), ("GPSTrack", "90"),
+    ...     ("SampleTime", "1"), ("GPSLatitude", "2"),
+    ...     ("GPSLatitude", "3"), ("GPSTrack", "180"),
+    ... ]:
+    ...     elements.append(ET.Element(expand_tag(f"QuickTime:{tag}")))
+    ...     elements[-1].text = text
+    >>> records = _group_gps_records(
+    ...     elements,
+    ...     ["QuickTime:GPSLatitude", "QuickTime:GPSTrack"],
+    ...     record_start_tag="QuickTime:SampleTime",
+    ... )
+    >>> [sorted(record.values()) for record in records]
+    [['1', '90'], ['2'], ['180', '3']]
+    """
+    expanded_tags = {expand_tag(tag) for tag in tags}
+    expanded_record_start_tag = (
+        expand_tag(record_start_tag) if record_start_tag is not None else None
+    )
+    expanded_first_tag = expand_tag(first_tag) if first_tag is not None else None
+
+    record: dict[str, str] = {}
+    for element in elements:
+        if element.tag == expanded_record_start_tag:
+            if record:
+                yield record
+            record = {}
+        elif element.tag in expanded_tags and element.text is not None:
+            if record and (element.tag in record or element.tag == expanded_first_tag):
+                yield record
+                record = {}
+            record[element.tag] = element.text
+    if record:
+        yield record
+
+
+def _aggregate_gps_track_by_record(
+    elements: T.Iterable[ET.Element],
+    time_tag: str,
+    lon_tag: str,
+    lat_tag: str,
+    alt_tag: str,
+    direction_tag: str,
+    record_start_tag: str | None = None,
+) -> list[GPSPoint]:
+    """
+    Aggregate the GPS records that ExifTool lists one after another, each with
+    its GPS time in **time_tag**, into a track whose times start at 0. Every
+    point takes its altitude and direction from its own record.
+    """
+    expanded_time_tag = expand_tag(time_tag)
+    expanded_lon_tag = expand_tag(lon_tag)
+    expanded_lat_tag = expand_tag(lat_tag)
+    expanded_alt_tag = expand_tag(alt_tag)
+    expanded_direction_tag = expand_tag(direction_tag)
+
+    track: list[GPSPoint] = []
+    for record in _group_gps_records(
+        elements,
+        [time_tag, lon_tag, lat_tag, alt_tag, direction_tag],
+        record_start_tag=record_start_tag,
+        first_tag=time_tag,
+    ):
+        time_text = record.get(expanded_time_tag)
+        dt = exif_read.parse_gps_datetime(time_text) if time_text is not None else None
+        timestamp = geo.as_unix_time(dt) if dt is not None else None
+        lon = _maybe_float(record.get(expanded_lon_tag))
+        lat = _maybe_float(record.get(expanded_lat_tag))
+        if timestamp is None or lon is None or lat is None:
+            continue
+
+        track.append(
+            GPSPoint(
+                time=timestamp,
+                lon=lon,
+                lat=lat,
+                alt=_maybe_float(record.get(expanded_alt_tag)),
+                angle=_maybe_float(record.get(expanded_direction_tag)),
+                epoch_time=timestamp,
+                fix=None,
+                precision=None,
+                ground_speed=None,
+            )
+        )
+
+    track.sort(key=lambda point: point.time)
+
+    # A BlackVue video holds a copy of its GPS log in the low resolution
+    # recording embedded in it, which ExifTool lists as well
+    track = _deduplicate_gps_points(track, same_gps_point=_same_gps_point)
+
+    if track:
+        first_time = track[0].time
+        for point in track:
+            point.time = point.time - first_time
+
+    return track
+
+
 def _aggregate_samples(
     elements: T.Iterable[ET.Element],
     sample_time_tag: str,
@@ -436,6 +552,12 @@ def extract_gps_track(self) -> list[geo.Point]:
         if track_with_fix:
             return T.cast(T.List[geo.Point], track_with_fix)
 
+        # dashcams that write LIGOGPSINFO (e.g. GKU, Abask), which ExifTool
+        # decrypts. Its GPS times are in UTC, unlike the clocks of these cameras
+        track_with_fix = self._extract_gps_track_from_quicktime(namespace="LIGO")
+        if track_with_fix:
+            return T.cast(T.List[geo.Point], track_with_fix)
+
         return []
 
     def _extract_make_and_model(self) -> tuple[str | None, str | None]:
@@ -585,12 +707,16 @@ def _extract_gps_track_from_quicktime(
         ):
             return []
 
-        return _aggregate_gps_track(
-            self._texts_by_tag,
+        root = self.etree.getroot()
+        if root is None:
+            raise ValueError("ElementTree root is None")
+
+        return _aggregate_gps_track_by_record(
+            root,
             time_tag=f"{namespace}:GPSDateTime",
             lon_tag=f"{namespace}:GPSLongitude",
             lat_tag=f"{namespace}:GPSLatitude",
             alt_tag=f"{namespace}:GPSAltitude",
-            gps_time_tag=f"{namespace}:GPSDateTime",
             direction_tag=f"{namespace}:GPSTrack",
+            record_start_tag=f"{namespace}:SampleTime",
         )

mapillary_tools/ffmpeg.py

@@ -54,6 +54,7 @@ class Stream(T.TypedDict):
     r_frame_rate: str
     avg_frame_rate: str
     nb_frames: str
+    sample_aspect_ratio: str
 
 
 class Format(T.TypedDict, total=False):
@@ -199,6 +200,7 @@ def extract_frames_by_interval(
         sample_dir: Path,
         sample_interval: float,
         stream_specifier: int | str = "v",
+        sample_aspect_ratio: str | None = None,
     ) -> None:
         """
         Extract frames from video at regular time intervals using fps filter.
@@ -210,6 +212,8 @@ def extract_frames_by_interval(
             stream_specifier: Stream specifier to target specific stream(s).
                               Can be an integer (stream index) or "v" (all video streams)
                               See https://ffmpeg.org/ffmpeg.html#Stream-specifiers-1
+            sample_aspect_ratio: The sample aspect ratio of the stream, as ffprobe
+                                 reports it (e.g. "4:3"). See _scale_to_square_pixels
 
         Raises:
             FFmpegNotFoundError: If ffmpeg binary is not found
@@ -229,7 +233,15 @@ def extract_frames_by_interval(
             # Select stream
             *stream_selector,
             # Filter videos
-            *["-vf", f"fps=1/{sample_interval}"],
+            *[
+                "-vf",
+                ",".join(
+                    [
+                        f"fps=1/{sample_interval}",
+                        *self._scale_to_square_pixels(sample_aspect_ratio),
+                    ]
+                ),
+            ],
             # Video quality level (or the alias -q:v)
             # -q:v=1 is the best quality but larger image sizes
             # see https://stackoverflow.com/a/10234065
@@ -286,6 +298,7 @@ def extract_specified_frames(
         sample_dir: Path,
         frame_indices: set[int],
         stream_specifier: int | str = "v",
+        sample_aspect_ratio: str | None = None,
     ) -> None:
         """
         Extract specific frames from video by frame number using select filter.
@@ -300,6 +313,8 @@ def extract_specified_frames(
             stream_specifier: Stream specifier to target specific stream(s).
                               Can be an integer (stream index) or "v" (all video streams)
                               See https://ffmpeg.org/ffmpeg.html#Stream-specifiers-1
+            sample_aspect_ratio: The sample aspect ratio of the stream, as ffprobe
+                                 reports it (e.g. "4:3"). See _scale_to_square_pixels
 
         Raises:
             FFmpegNotFoundError: If ffmpeg binary is not found
@@ -333,7 +348,14 @@ def extract_specified_frames(
         # https://devblogs.microsoft.com/oldnewthing/20031210-00/?p=41553
         with tempfile.NamedTemporaryFile(mode="w+", delete=delete) as select_file:
             try:
-                select_file.write(f"select={eqs}")
+                select_file.write(
+                    ",".join(
+                        [
+                            f"select={eqs}",
+                            *self._scale_to_square_pixels(sample_aspect_ratio),
+                        ]
+                    )
+                )
                 select_file.flush()
                 # If not close, error "The process cannot access the file because it is being used by another process"
                 if not delete:
@@ -463,6 +485,48 @@ def iterate_samples(
                 stream_specifier, frame_idx = result
                 yield (stream_specifier, frame_idx, sample_path)
 
+    @classmethod
+    def _scale_to_square_pixels(cls, sample_aspect_ratio: str | None) -> list[str]:
+        """
+        The filters that scale the frames of a stream with non-square pixels
+        to square ones, so that they are written at the aspect ratio the
+        video is displayed at. A JPEG keeps only a density hint for the
+        pixel shape, which most viewers ignore. The frames are stretched
+        along one side, never shrunk along the other, so no detail is lost.
+
+        Frames of a stream with square pixels, or whose pixel shape is
+        unknown ("0:1" or not reported), are written as they are decoded.
+
+        Examples:
+            >>> FFMPEG._scale_to_square_pixels("4:3")
+            ['scale=w=if(gt(sar\\\\,1)\\\\,round(iw*sar/2)*2\\\\,iw):h=if(lt(sar\\\\,1)\\\\,round(ih/sar/2)*2\\\\,ih)', 'setsar=1']
+            >>> FFMPEG._scale_to_square_pixels("1:1")
+            []
+            >>> FFMPEG._scale_to_square_pixels("0:1")
+            []
+            >>> FFMPEG._scale_to_square_pixels(None)
+            []
+        """
+        if not sample_aspect_ratio:
+            return []
+
+        try:
+            num, den = (int(x) for x in sample_aspect_ratio.split(":"))
+        except ValueError:
+            return []
+
+        if num <= 0 or den <= 0 or num == den:
+            return []
+
+        # sar is that of the frames as decoded, which ffmpeg may have rotated
+        # since the stream was probed. The stretched side is rounded to an
+        # even size, as 4:2:0 chroma subsampling halves it
+        return [
+            "scale=w=if(gt(sar\\,1)\\,round(iw*sar/2)*2\\,iw)"
+            ":h=if(lt(sar\\,1)\\,round(ih/sar/2)*2\\,ih)",
+            "setsar=1",
+        ]
+
     def _read_filter_from_file_args(self, filter_path: str) -> list[str]:
         """
         Arguments that apply a video filter graph read from a file.