Conversation
The exiftool reader looked for GPSHPositioningError only. Cameras with GPS9 telemetry (GoPro MAX 2, HERO11 and newer) do not write that tag; they write GPSDOP. So every point came back with precision=None, the DoP test in remove_noisy_points() was skipped entirely, and the reader accepted tracks the native GPMF parser rejects as noise. On a MAX 2 clip the two readers disagreed completely: native n=267 precision=185.0 / 207.0 exiftool n=267 precision=None (x267) The tags are mutually exclusive per telemetry generation and both scale by 100, though for different reasons: GPSDOP is a dilution of precision, which is what GPMF's GPSP holds, while GPSHPositioningError is a horizontal error in meters and only approximates it. Read GPSDOP first and fall back to GPSHPositioningError, so GPS5 cameras are untouched: MAX 2 (GPS9) GPSDOP 1.85, 2.07 -> 185, 207 matches native exactly hero8 (GPS5) GPSHPositioningError 99.99 -> 9999 unchanged This is why "--geotag_source exiftool_runtime" accepts the noisy MAX 2 clip from T288698491 that "--geotag_source native" rejects. The hole pre-dates PR 831; that PR only made the default chain reach it, and was revised so it no longer does. Verified on real MAX 2 footage for the tag reading. The rejection behaviour is covered by unit tests rather than a fixture: the MAX 2 files available locally all have good DoP, and the reported noisy clip is not on this machine.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The exiftool reader looks for
GPSHPositioningErroronly. Cameras with GPS9 telemetry (GoPro MAX 2, HERO11 and newer) do not write that tag — they writeGPSDOP.So every point came back with
precision=None, the DoP test inremove_noisy_points()was skipped entirely, and the reader accepted tracks the native GPMF parser rejects as noise. On a MAX 2 clip the two readers disagree completely:This is why
--geotag_source exiftool_runtimeaccepts the noisy MAX 2 clip from the report that--geotag_source nativerejects.Fix
The tags are mutually exclusive per telemetry generation, and both scale by 100 — though for different reasons:
GPSDOPis a dilution of precision, which is what GPMF'sGPSPholds, whileGPSHPositioningErroris a horizontal error in metres and only approximates it. ReadGPSDOPfirst, fall back toGPSHPositioningError:GPSDOP1.85 / 2.07GPSHPositioningError99.99GPS5 cameras are untouched.
Relationship to #831
This hole pre-dates #831 —
mainforced through--geotag_source exiftool_runtimealready yields the unfiltered track. #831 briefly made the default chain reach it, and was revised (898988b) so it no longer does. This PR closes the hole itself, independently.Verification
Tag reading verified on real MAX 2 footage. The rejection behaviour is covered by unit tests rather than a fixture: the MAX 2 files available locally all happen to have good DoP, and the reported noisy clip is not on this machine.
Full suite passes, mypy and ruff clean. (
test_persistent_cache::test_multiprocess_shared_cache_comprehensiveis flaky under parallel load on this machine — it passes in isolation both with and without this change.)