diff --git a/mapillary_tools/exiftool_read_video.py b/mapillary_tools/exiftool_read_video.py
index 4257abca..f666944a 100644
--- a/mapillary_tools/exiftool_read_video.py
+++ b/mapillary_tools/exiftool_read_video.py
@@ -344,7 +344,7 @@ def _aggregate_gps_track_by_sample_time(
direction_tag: str | None = None,
ground_speed_tag: str | None = None,
gps_fix_tag: str | None = None,
- gps_precision_tag: str | None = None,
+ gps_precision_tags: T.Sequence[str] = (),
) -> list[GPSPoint]:
track: list[GPSPoint] = []
@@ -352,9 +352,7 @@ def _aggregate_gps_track_by_sample_time(
if gps_fix_tag is not None:
expanded_gps_fix_tag = expand_tag(gps_fix_tag)
- expanded_gps_precision_tag = None
- if gps_precision_tag is not None:
- expanded_gps_precision_tag = expand_tag(gps_precision_tag)
+ expanded_gps_precision_tags = [expand_tag(tag) for tag in gps_precision_tags]
for sample_time, sample_duration, elements in sample_iterator:
texts_by_tag = _index_text_by_tag(elements)
@@ -369,16 +367,21 @@ def _aggregate_gps_track_by_sample_time(
gps_fix = None
gps_precision = None
- if expanded_gps_precision_tag is not None:
+ for expanded_gps_precision_tag in expanded_gps_precision_tags:
gps_precision_texts = texts_by_tag.get(expanded_gps_precision_tag)
if gps_precision_texts:
gps_precision = _maybe_float(gps_precision_texts[0])
if gps_precision is not None:
- # GPS precision in ExifTool (i.e. horizontal positioning error) are in meters.
- # https://exiftool.org/forum/index.php?topic=11565.0
- # Here we multiply by 100 to be compatible with the GPSP
- # described in https://github.com/gopro/gpmf-parser
+ # Both tags hold the dilution of precision that GPSP holds,
+ # already divided by 100 by ExifTool, so scaling back up
+ # recovers the raw GPMF value the native parser stores:
+ # GPS9 reports it as GPSDOP, divided by its SCAL entry of
+ # 100, and GPS5 as GPSP, which ExifTool renames to
+ # GPSHPositioningError and applies ValueConv $val/100 to.
+ # Despite that name it is not the EXIF horizontal error in
+ # meters. https://github.com/gopro/gpmf-parser
gps_precision = gps_precision * 100
+ break
# Aggregate GPS points in the sample
points = _aggregate_gps_track(
@@ -547,7 +550,14 @@ def _extract_gps_track_from_track(self) -> list[GPSPoint]:
direction_tag=f"{track_ns}:GPSTrack",
ground_speed_tag=f"{track_ns}:GPSSpeed",
gps_fix_tag=f"{track_ns}:GPSMeasureMode",
- gps_precision_tag=f"{track_ns}:GPSHPositioningError",
+ # Which one a camera writes depends on its telemetry
+ # format: GPS9 (GoPro MAX 2, HERO11 and newer) reports
+ # GPSDOP, while GPS5 (HERO10 and older) reports
+ # GPSHPositioningError. Prefer the true DOP when present.
+ gps_precision_tags=[
+ f"{track_ns}:GPSDOP",
+ f"{track_ns}:GPSHPositioningError",
+ ],
)
if track:
return track
diff --git a/tests/unit/test_exiftool_read_video.py b/tests/unit/test_exiftool_read_video.py
index fe8c88a0..9756c806 100644
--- a/tests/unit/test_exiftool_read_video.py
+++ b/tests/unit/test_exiftool_read_video.py
@@ -5,9 +5,11 @@
from __future__ import annotations
+import dataclasses
import xml.etree.ElementTree as ET
import pytest
+from mapillary_tools import constants
from mapillary_tools.exiftool_read_video import (
_aggregate_gps_track,
_aggregate_gps_track_by_sample_time,
@@ -20,6 +22,7 @@
ExifToolReadVideo,
expand_tag,
)
+from mapillary_tools.gpmf.gpmf_gps_filter import remove_noisy_points
from mapillary_tools.telemetry import GPSFix, GPSPoint
@@ -121,6 +124,36 @@ def _make_element(tag: str, text: str) -> ET.Element:
"""
+# GPS9 telemetry (GoPro MAX 2, HERO11+): GPSDOP in place of
+# GPSHPositioningError. DoP values are from a real MAX 2 clip.
+GPS9_XML = """\
+
+
+
+ GoPro
+ GoPro Max 2
+ 0
+ 1.001
+ 47.359832
+ 8.522706
+ 414.9
+ 2026:07:31 00:25:23.200Z
+ 3
+ 1.85
+ 1.001
+ 1.001
+ 47.359810
+ 8.522680
+ 415.2
+ 2026:07:31 00:25:24.200Z
+ 3
+ 2.07
+
+
+"""
+
INSTA360_XML = """\
@@ -785,11 +818,117 @@ def test_gps_precision_scaled(self):
sample_iterator,
lon_tag=f"{track_ns}:GPSLongitude",
lat_tag=f"{track_ns}:GPSLatitude",
- gps_precision_tag=f"{track_ns}:GPSHPositioningError",
+ gps_precision_tags=[f"{track_ns}:GPSHPositioningError"],
)
assert len(track) == 1
assert track[0].precision == pytest.approx(219.0)
+ def _precision_from(self, tags: dict[str, str]) -> float | None:
+ """Read precision from a sample carrying the given DoP-ish tags."""
+ track_ns = "Track1"
+ elements = [
+ _make_element(f"{track_ns}:GPSLongitude", "8.0"),
+ _make_element(f"{track_ns}:GPSLatitude", "47.0"),
+ *(_make_element(f"{track_ns}:{tag}", value) for tag, value in tags.items()),
+ ]
+ track = _aggregate_gps_track_by_sample_time(
+ [(0.0, 1.0, elements)],
+ lon_tag=f"{track_ns}:GPSLongitude",
+ lat_tag=f"{track_ns}:GPSLatitude",
+ gps_precision_tags=[
+ f"{track_ns}:GPSDOP",
+ f"{track_ns}:GPSHPositioningError",
+ ],
+ )
+ assert len(track) == 1
+ return track[0].precision
+
+ def test_gps9_cameras_report_dop_instead(self):
+ """
+ GPS9 telemetry (GoPro MAX 2, HERO11+) reports GPSDOP and no
+ GPSHPositioningError, so reading only the latter loses precision
+ entirely and the noise filter silently keeps a track it should drop.
+ """
+ assert self._precision_from({"GPSDOP": "1.85"}) == pytest.approx(185.0)
+
+ def test_gps5_cameras_still_work(self):
+ """HERO10 and older report only GPSHPositioningError."""
+ assert self._precision_from({"GPSHPositioningError": "99.99"}) == pytest.approx(
+ 9999.0
+ )
+
+ def test_dop_wins_when_a_camera_reports_both(self):
+ """Both spellings carry the quantity GPSP holds, so the order only
+ matters for a file reporting both; GPSDOP, the GPS9 one, is read first."""
+ assert self._precision_from(
+ {"GPSDOP": "1.85", "GPSHPositioningError": "99.99"}
+ ) == pytest.approx(185.0)
+
+ def test_no_precision_tags_at_all(self):
+ assert self._precision_from({}) is None
+
+ def _track_from(self, tags: dict[str, str]) -> list[GPSPoint]:
+ """
+ Build a two-sample track carrying the given DoP-ish tags.
+
+ Two points keep remove_outliers() a no-op -- it returns early below
+ two distances -- so the DoP gate is what the assertions measure.
+ """
+ track_ns = "Track1"
+ sample_iterator = [
+ (
+ float(idx),
+ 1.0,
+ [
+ _make_element(f"{track_ns}:GPSLongitude", f"{8.0 + idx * 0.0001}"),
+ _make_element(f"{track_ns}:GPSLatitude", f"{47.0 + idx * 0.0001}"),
+ *(
+ _make_element(f"{track_ns}:{tag}", value)
+ for tag, value in tags.items()
+ ),
+ ],
+ )
+ for idx in range(2)
+ ]
+ return list(
+ _aggregate_gps_track_by_sample_time(
+ sample_iterator,
+ lon_tag=f"{track_ns}:GPSLongitude",
+ lat_tag=f"{track_ns}:GPSLatitude",
+ gps_precision_tags=[
+ f"{track_ns}:GPSDOP",
+ f"{track_ns}:GPSHPositioningError",
+ ],
+ )
+ )
+
+ def test_a_noisy_gps9_track_is_now_filtered(self):
+ """
+ The reported failure: a MAX 2 whose DoP is far over the limit was
+ accepted by the exiftool reader while the native parser rejected it.
+
+ Reading GPSDOP is only worth anything if the noise filter then drops
+ the track, so assert that end rather than the parsed number alone.
+ """
+ noisy = self._track_from({"GPSDOP": "21.39"})
+ assert [p.precision for p in noisy] == [
+ pytest.approx(2139.0),
+ pytest.approx(2139.0),
+ ]
+ assert list(remove_noisy_points(noisy)) == []
+
+ # Leaving the tag unread is what the filter saw before this change:
+ # no precision to test, so the same noisy track survives
+ unread = [dataclasses.replace(p, precision=None) for p in noisy]
+ assert len(remove_noisy_points(unread)) == 2
+
+ def test_a_clean_gps9_track_survives_the_filter(self):
+ """A healthy MAX 2 DoP is far under the limit and must not be dropped."""
+ clean = self._track_from({"GPSDOP": "1.85"})
+ assert clean[0].precision == pytest.approx(185.0)
+ assert clean[0].precision < constants.GOPRO_MAX_DOP100
+ assert len(remove_noisy_points(clean)) == 2
+
def test_multiple_points_per_sample_get_interpolated_time(self):
"""Multiple GPS points within a single sample get evenly spaced times."""
track_ns = "Track1"
@@ -1170,6 +1309,22 @@ def test_gopro_track_gps(self):
assert track[0].lat == pytest.approx(47.359832)
assert track[0].lon == pytest.approx(8.522706)
+ def test_gps9_track_carries_dop_through_extract(self):
+ """
+ The tag list extract_gps_track() asks for has to include GPSDOP.
+
+ The other DoP tests drive _aggregate_gps_track_by_sample_time()
+ with their own tag list, so dropping GPSDOP from the reader would
+ leave them all green. This one goes through the real entry point,
+ and the DoP it returns is what remove_noisy_points() gates on.
+ """
+ reader = ExifToolReadVideo(_etree_from_xml(GPS9_XML))
+ track = reader.extract_gps_track()
+ assert [p.precision for p in track] == [
+ pytest.approx(185.0),
+ pytest.approx(207.0),
+ ]
+
def test_empty_gps_track(self):
"""When no GPS data is present, returns empty list."""
xml = """\