Skip more named companion files with a warning

README.md

@@ -241,13 +241,14 @@ Unsupported entries include their filename, category, stable reason code, and
 normalized extension. Hidden/system files and common GPS or metadata sidecars
 are ignored during directory discovery, but an explicitly supplied unsupported
 file is always reported. Such a file also fails processing, with a
-`MapillaryUnsupportedFileError` entry in the image description file, whether
-or not the report is requested, unless it is a known sidecar or companion file
-(`.thm`, `.lrv`, `.gpx`, `.nmea`, `.xml`, `.json`, `.srt`, `.txt`, `.csv`,
-`.log`, `.md5`, macOS `._*` files and `.DS_Store`): those are skipped with a
-warning, so that all the files of a camera folder can be named. The report is
-written only to the requested local path and contains absolute source
-filenames, so treat it as local capture metadata.
+`MapillaryUnsupportedFileError` entry in the image description file, whether or
+not the report is requested, unless it is a known sidecar or companion file
+(`.thm`, `.lrv`, `.lrf`, `.aae`, `.gpx`, `.nmea`, `.kml`, `.kmz`, `.fit`,
+`.xml`, `.json`, `.xmp`, `.srt`, `.txt`, `.csv`, `.log`, `.md5`, macOS `._*`
+files and `.DS_Store`, and Windows `Thumbs.db` and `desktop.ini`): those are
+skipped with a warning, so that all the files of a camera folder can be named.
+The report is written only to the requested local path and contains absolute
+source filenames, so treat it as local capture metadata.
 
 ## Upload
 

mapillary_tools/geotag/geotag_images_from_nmea_file.py

@@ -40,18 +40,28 @@
 # The byte order mark, which some editors write at the start of a UTF-8 file
 _BOM = "\ufeff"
 
-# How much of a file tells whether it is text at all, before it is read whole:
-# a video given as an NMEA file is gigabytes
+# How much of a file tells whether it is an NMEA log at all, before it is read
+# whole: a video given as an NMEA file is gigabytes
 _SNIFF_BYTES = 64 * 1024
 
+# An RMC or a GGA sentence, as in _SENTENCE_RE, in the bytes of a file
+_SENTENCE_BYTES_RE = re.compile(rb"\$[A-Z]{2}(RMC|GGA),")
+
+# A NUL byte, which a logger leaves in a log after a power cut, such as on an
+# SD card: it ends a line, as no sentence holds one
+_NUL = "\x00"
+
 # The checksum that ends a sentence: *HH. It is optional
 _CHECKSUM_RE = re.compile(r"\*[0-9A-Fa-f]{2}\s*$")
 
-# The fields of a complete sentence: an RMC sentence has 11 (NMEA 2.0 to
-# 2.2), 12 with the mode of NMEA 2.3, and 13 with the navigational status of
-# NMEA 4.1; a GGA sentence has 14. One without its checksum that has fewer was
-# cut short, such as the last one of a log when the recording stopped
-_MIN_FIELDS = {"RMC": 11, "GGA": 14}
+# The fields that a fix is read from: those of an RMC sentence through the
+# date, the 9th, and those of a GGA sentence through the altitude and its
+# unit, the 10th. One without its checksum that has fewer was cut short, such
+# as the last one of a log when the recording stopped. The fields after them
+# are optional, and receivers leave them off: the magnetic variation, the mode
+# and the navigational status of an RMC sentence, the height of the geoid and
+# the DGPS fields of a GGA sentence
+_MIN_FIELDS = {"RMC": 9, "GGA": 10}
 
 # The time of a sentence (hhmmss, and a fraction of a second), and the date of
 # an RMC sentence (ddmmyy). pynmea2 reads a field cut short as another time or
@@ -157,15 +167,16 @@ def get_lat_lon_time_from_nmea(nmea_file: Path) -> list[geo.Point]:
     own fields show it. A file of which no fix can be read raises ValueError,
     which says why, if it has malformed sentences or bytes that are not text.
 
-    A file that is not text at all, which its first bytes show, raises
-    ValueError without being read further.
+    A file that is no NMEA log at all, such as a video, which its first bytes
+    show (see _looks_binary()), raises ValueError without being read further.
+    NUL bytes, which a logger leaves in a log after a power cut, end a line.
 
     A file is read once in a process, however many times it is asked for.
     """
     with nmea_file.open("rb") as f:
         head = f.read(_SNIFF_BYTES)
-        if not _looks_like_text(head):
-            raise ValueError("it is not text")
+        if _looks_binary(head):
+            raise ValueError(_describe_binary(head))
         data = head + f.read()
     key = (str(nmea_file.resolve()), hashlib.sha256(data).hexdigest())
     read = _READ.get(key)
@@ -182,25 +193,49 @@ def get_lat_lon_time_from_nmea(nmea_file: Path) -> list[geo.Point]:
     return [dataclasses.replace(p) for p in read]
 
 
-def _looks_like_text(head: bytes) -> bool:
+def _looks_binary(head: bytes) -> bool:
     r"""
-    Whether the first bytes **head** of a file can be those of a text file:
-    without NUL bytes, and mostly bytes that decode as UTF-8, where a log may
-    have a few bytes damaged
+    Whether the first bytes **head** of a file show it to be no NMEA log at
+    all, such as a video: they hold no RMC or GGA sentence, and NUL bytes or
+    mostly bytes that do not decode as UTF-8. A log padded with NUL bytes, as a
+    logger leaves it after a power cut, holds sentences, and so does one with
+    a few bytes damaged
 
-    >>> _looks_like_text(b"$GPRMC,024636.00,A,3735.30670,N\r\n")
-    True
-    >>> _looks_like_text(b"$GPRMC,024636.00,\xe9,3735.30670,N\r\n")
-    True
-    >>> _looks_like_text(b"\x00\x00\x00\x18ftypmp42")
+    >>> _looks_binary(b"$GPRMC,024636.00,A,3735.30670,N\r\n" + b"\x00" * 100)
     False
-    >>> _looks_like_text(b"\xca\xfe\xba\xbe" * 100)
+    >>> _looks_binary(b"\x00" * 100 + b"$GPGGA,024636.00,3735.30670,N\r\n")
     False
+    >>> _looks_binary(b"$GPRMC,024636.00,\xe9,3735.30670,N\r\n")
+    False
+    >>> _looks_binary(b"$GPGSV,3,1,11,03,03,111,00\r\n")
+    False
+    >>> _looks_binary(b"\x00\x00\x00\x18ftypmp42")
+    True
+    >>> _looks_binary(b"\xca\xfe\xba\xbe" * 100)
+    True
     """
-    if b"\x00" in head:
+    if _SENTENCE_BYTES_RE.search(head):
         return False
+    if b"\x00" in head:
+        return True
     text = head.decode("utf-8", errors="replace")
-    return text.count(_NOT_TEXT) * 2 <= len(text)
+    return len(text) < text.count(_NOT_TEXT) * 2
+
+
+def _describe_binary(head: bytes) -> str:
+    r"""
+    Why a file whose first bytes **head** show it to be no NMEA log is not
+    read: it is UTF-16 text, which some Windows tools write, or no text at all
+
+    >>> _describe_binary("$GPRMC,024636.00,A".encode("utf-16"))
+    'it is UTF-16 text, which is not read: save it as UTF-8 or ASCII'
+    >>> _describe_binary(b"\x00\x00\x00\x18ftypmp42")
+    'it is not text'
+    """
+    for encoding in ("utf-16", "utf-16-le", "utf-16-be"):
+        if _SENTENCE_RE.search(head.decode(encoding, errors="ignore")):
+            return "it is UTF-16 text, which is not read: save it as UTF-8 or ASCII"
+    return "it is not text"
 
 
 def _read_nmea_data(nmea_file: Path, data: bytes) -> list[geo.Point]:
@@ -217,14 +252,15 @@ def _read_nmea_data(nmea_file: Path, data: bytes) -> list[geo.Point]:
         io.BytesIO(data), encoding="utf-8-sig", errors="replace"
     ) as f:
         lines = f.readlines()
-        lines = [line.rstrip("\n\r").lstrip(_BOM) for line in lines]
     not_text = any(_NOT_TEXT in line for line in lines)
 
-    # (line number, line) of each RMC and GGA sentence
+    # (line number, line) of each RMC and GGA sentence, of the lines that NUL
+    # bytes end too (see _NUL)
     sentences = [
-        (number, line)
+        (number, piece.lstrip(_BOM))
         for number, line in enumerate(lines, start=1)
-        if _SENTENCE_RE.search(line)
+        for piece in line.rstrip("\n\r").split(_NUL)
+        if _SENTENCE_RE.search(piece)
     ]
 
     messages = []
@@ -330,7 +366,7 @@ def _parse_sentence(line: str) -> pynmea2.NMEASentence:
     """
     Parse an NMEA sentence, raising ValueError if it is malformed: if it does
     not parse or its checksum does not match, if it has no checksum and fewer
-    fields than a complete sentence (see _MIN_FIELDS), if its time or the date
+    fields than a fix is read from (see _MIN_FIELDS), if its time or the date
     of an RMC sentence is not hhmmss[.ss] or ddmmyy, or if a field that a fix
     is read from does not parse, which pynmea2 leaves a string, or if it has a
     byte that is not text, decoded as U+FFFD
@@ -347,15 +383,17 @@ def _parse_sentence(line: str) -> pynmea2.NMEASentence:
     Traceback (most recent call last):
     ...
     ValueError: malformed datestamp: '2212'
-    >>> _parse_sentence("$GPRMC,024636.00,A,3735.30670,N,12220.25810,W,0.0,0.0,221222")
+    >>> _parse_sentence("$GPRMC,024636.00,A,3735.30670,N,12220.25810,W,0.0,0.0")
     Traceback (most recent call last):
     ...
-    ValueError: cut short: 9 of the 11 fields of an RMC sentence
+    ValueError: cut short: 8 of the 9 fields of an RMC sentence that a fix is read from
+    >>> type(_parse_sentence("$GPRMC,024636.00,A,3735.30670,N,12220.25810,W,0.0,0.0,221222")).__name__
+    'RMC'
     >>> _parse_sentence("$GPGGA,024636.00,3735.30670,N,12220.25810,W,1,08,1.0,12")
     Traceback (most recent call last):
     ...
-    ValueError: cut short: 9 of the 14 fields of a GGA sentence
-    >>> type(_parse_sentence("$GPGGA,024636.00,3735.30670,N,12220.25810,W,1,08,1.0,12.5,M,0.0,M,,")).__name__
+    ValueError: cut short: 9 of the 10 fields of a GGA sentence that a fix is read from
+    >>> type(_parse_sentence("$GPGGA,024636.00,3735.30670,N,12220.25810,W,1,08,1.0,12.5,M")).__name__
     'GGA'
     """
     if _NOT_TEXT in line:
@@ -382,7 +420,7 @@ def _parse_sentence(line: str) -> pynmea2.NMEASentence:
         if kind in _MIN_FIELDS and len(message.data) < _MIN_FIELDS[kind]:
             article = "an" if kind == "RMC" else "a"
             raise ValueError(
-                f"cut short: {len(message.data)} of the {_MIN_FIELDS[kind]} fields of {article} {kind} sentence"
+                f"cut short: {len(message.data)} of the {_MIN_FIELDS[kind]} fields of {article} {kind} sentence that a fix is read from"
             )
 
     fields: dict[str, type] = {}

mapillary_tools/process_geotag_properties.py

@@ -29,17 +29,25 @@
 ]
 
 # Files that cameras and other tools write beside images and videos: GoPro
-# thumbnails (.THM) and low-resolution proxies (.LRV), GPS logs, metadata,
-# subtitles (.SRT), notes and checksums. Selecting all the files of a camera
-# folder names them along with its images and videos
+# thumbnails (.THM) and low-resolution proxies (.LRV), DJI low-resolution
+# proxies (.LRF), Apple photo edits (.AAE), GPS logs and tracks (.gpx, .nmea,
+# .kml, .kmz, .fit), metadata (.xml, .json, .xmp), subtitles (.SRT), notes and
+# checksums. Selecting all the files of a camera folder names them along with
+# its images and videos
 _SIDECAR_EXTENSIONS = frozenset(
     {
         ".thm",
         ".lrv",
+        ".lrf",
+        ".aae",
         ".gpx",
         ".nmea",
+        ".kml",
+        ".kmz",
+        ".fit",
         ".xml",
         ".json",
+        ".xmp",
         ".srt",
         ".txt",
         ".csv",
@@ -48,6 +56,11 @@
     }
 )
 
+# Files that operating systems write into folders of images and videos: the
+# Finder settings of macOS, and the thumbnail cache and folder settings of
+# Windows. Compared case-insensitively
+_SIDECAR_FILENAMES = frozenset({".ds_store", "thumbs.db", "desktop.ini"})
+
 
 def _normalize_import_paths(import_path: Path | T.Sequence[Path]) -> T.Sequence[Path]:
     import_paths: T.Sequence[Path]
@@ -155,21 +168,23 @@ def process_geotag_properties(
 def _is_sidecar_file(path: Path) -> bool:
     """
     Whether a file is one that accompanies images and videos (see
-    _SIDECAR_EXTENSIONS), or one that macOS writes beside them: .DS_Store, and
-    the AppleDouble file ._NAME of each file it copies to a drive that cannot
-    hold its metadata, which has the extension of that file but is no image or
-    video
+    _SIDECAR_EXTENSIONS), one that an operating system writes beside them (see
+    _SIDECAR_FILENAMES), or the AppleDouble file ._NAME that macOS writes for
+    each file it copies to a drive that cannot hold its metadata, which has the
+    extension of that file but is no image or video
 
     >>> [_is_sidecar_file(Path(name)) for name in ["GH010001.THM", "GL010001.LRV", "track.GPX"]]
     [True, True, True]
-    >>> [_is_sidecar_file(Path(name)) for name in ["._GOPR0001.JPG", ".DS_Store"]]
-    [True, True]
+    >>> [_is_sidecar_file(Path(name)) for name in ["DJI_0001.LRF", "IMG_0001.AAE", "photo.XMP"]]
+    [True, True, True]
+    >>> [_is_sidecar_file(Path(name)) for name in ["._GOPR0001.JPG", ".DS_Store", "Thumbs.db", "Desktop.ini"]]
+    [True, True, True, True]
     >>> [_is_sidecar_file(Path(name)) for name in ["GOPR0001.JPG", "VID_00_001.insv", "photo.png"]]
     [False, False, False]
     """
     return (
         path.name.startswith("._")
-        or path.name.casefold() == ".ds_store"
+        or path.name.casefold() in _SIDECAR_FILENAMES
         or path.suffix.lower() in _SIDECAR_EXTENSIONS
     )
 

tests/integration/test_process_report.py

@@ -215,3 +215,41 @@ def test_process_skips_named_sidecars_of_a_camera_folder(tmpdir: py.path.local):
         (desc["filename"], desc["error"]["type"])
         for desc in json.loads(desc_path.read_text(encoding="utf-8"))
     ] == [(str(original.resolve()), "MapillaryUnsupportedFileError")]
+
+
+def test_process_skips_named_companion_files_of_other_cameras_and_systems(
+    tmpdir: py.path.local,
+):
+    """
+    Named along with the images, DJI proxies, Apple photo edits, GPS tracks,
+    XMP metadata and the files that Windows writes into folders failed the run
+    """
+    root = Path(str(tmpdir))
+    folder = root / "DCIM"
+    folder.mkdir()
+    companions = [
+        folder / "DJI_0001.LRF",
+        folder / "IMG_0001.AAE",
+        folder / "IMG_0001.XMP",
+        folder / "track.kml",
+        folder / "track.KMZ",
+        folder / "ride.fit",
+        folder / "Thumbs.db",
+        folder / "desktop.ini",
+    ]
+    for companion in companions:
+        companion.write_bytes(b"not an image")
+
+    descs, report = _run_process_with_report(companions, root)
+    assert descs == []
+    assert report["processed_file_count"] == 0
+    assert sorted(skipped["filename"] for skipped in report["skipped_files"]) == sorted(
+        str(companion.resolve()) for companion in companions
+    )
+
+    desc_path = root / "description.json"
+    run_command(
+        ["--desc_path", str(desc_path), *(str(companion) for companion in companions)],
+        command="process",
+    )
+    assert json.loads(desc_path.read_text(encoding="utf-8")) == []