Fix GPS epoch vs Unix epoch mix-up when geotagging CAMM videos - #828
Open
caglarpir wants to merge 1 commit into
Open
Fix GPS epoch vs Unix epoch mix-up when geotagging CAMM videos#828caglarpir wants to merge 1 commit into
caglarpir wants to merge 1 commit into
Conversation
CAMM stores GPS time in CAMMGPSPoint.time_gps_epoch (seconds since 1980-01-06, per the CAMM spec), while GPX, GPMF and BlackVue all store Unix time. Those two were being compared and rendered interchangeably, which is a ~315,964,800s (10 year) error. The visible symptom: geotagging a CAMM video from an external GPX made GPXVideoExtractor._gpx_offset() subtract a GPS time from a Unix time, so every GPX point was rebased ~10 years after the video start. At upload, camm_builder wrote that as the segment_duration of a version 0 elst, which is Int32sb, and the upload aborted with construct.core.FormatFieldError: Error in path (building) -> ... -> segment_duration struct '>l' error during building, given value 315964800000 There is also a silent variant with no crash: sampled frames, exported GPX and the description file all rendered a CAMM GPS timestamp directly as Unix time, dating imagery ~10 years too early. Changes: - telemetry: add gps_epoch_to_unix()/unix_to_gps_epoch(), including leap seconds (GPS time does not count them, so a naive +315964800 is currently 18s off -- ~250m of error at highway speed). - Point.get_unix_time() is now the canonical wall clock accessor; get_gps_epoch_time() is kept for the CAMM serialization boundary. Both are honest about their epoch for GPSPoint and CAMMGPSPoint. - parse_gpx() and uploader.prepare_camm_info() now convert to GPS time when populating time_gps_epoch, instead of storing Unix time in it. - _gpx_offset() compares Unix times via get_unix_time(), which also ignores zero/invalid timestamps rather than treating them as 1970. - sample_video, the GPX serializer and the description serializer render get_unix_time(). MAPGPSTrack[5] is now consistently Unix time; the schema said "GPS epoch time" but every consumer read it as Unix. - camm_builder falls back to a version 1 (64-bit) elst instead of overflowing, so an oversized offset can never abort an upload again. - warn when a GPX has to be shifted more than a day to sync. Two unrelated bugs found while tracing this: - camm_parser filed every CAMM type 6 GPS point into CAMMInfo.mini_gps, leaving CAMMInfo.gps always empty, because CAMMGPSPoint subclasses geo.Point and the isinstance checks were in the wrong order. - SourceOption.from_dict() assigned to a misspelled `.sourthe_path`, silently dropping an explicit source_path passed alongside a pattern.
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.
Summary
CAMM stores GPS time —
CAMMGPSPoint.time_gps_epochis seconds since the GPS epoch (1980-01-06), per the CAMM spec. GPX, GPMF and BlackVue all store Unix time. These two were being compared and rendered interchangeably, which is a 315,964,800 s (~10 year) error.The crash
Geotagging a CAMM video from an external GPX made
GPXVideoExtractor._gpx_offset()subtract a GPS time from a Unix time, so every GPX point got rebased ~10 years after the video start. At upload,camm_builderwrote that offset as thesegment_durationof a version 0elst, which isInt32sb:mapillary_toolsthen exits 1 and nothing is uploaded. Reproduced end-to-end on two real Labpano PanoX V2 captures (2.9 GB and 6.2 GB) with a sidecar GPX; both crash before the fix and both upload after it.The silent variant
No crash, wrong data.
sample_video, the GPX serializer and the description serializer each rendered a CAMM GPS timestamp directly as Unix time. On the videos above that stampsDateTimeOriginal/GPSDateTimeas 2016-08-07 instead of 2026-08-12 — off by 3656 days — with exit code 0.Changes
Epoch conversion is now explicit (
telemetry.py)gps_epoch_to_unix()/unix_to_gps_epoch(), including leap seconds. GPS time does not count them, so a naive+315964800is currently 18 s off — about 250 m of positional error at highway speed, which matters for a mapping tool.Point.get_unix_time()is the canonical wall-clock accessor.get_gps_epoch_time()is kept for the one place that needs GPS time: serializing CAMM. Both are now honest about their epoch onGPSPointandCAMMGPSPoint.Producers no longer store Unix time in a GPS-epoch field
parse_gpx()anduploader.prepare_camm_info()convert before populatingtime_gps_epoch. Previously a GPX-sourced upload wrote Unix time into CAMM type 6, so a spec-conformant reader decoded it as 2036.Consumers go through
get_unix_time()_gpx_offset()compares Unix times, and by using the accessor it also ignores zero/invalid timestamps instead of treating them as 1970.sample_video,serializer/gpx,serializer/description.MAPGPSTrack[5]is now consistently Unix time. The schema described it as "GPS epoch time", but every consumer already read it as Unix, and it was only ever GPS-epoch for the CAMM-native path — i.e. the bug. Schema description updated to match.Defence in depth
camm_builderfalls back to a version 1 (64-bit)elstinstead of overflowing, so an oversized offset can never abort an upload again.Two unrelated bugs found while tracing this
camm_parser.extract_camm_info()filed every CAMM type 6 GPS point intoCAMMInfo.mini_gps, leavingCAMMInfo.gpsalways empty —CAMMGPSPointsubclassesgeo.Pointand theisinstancechecks were in the wrong order. Masked downstream bycamm_info.gps or camm_info.mini_gps, so not currently user-visible, but it makesCAMMInfo.gps == []mean the opposite of what it says and violates the declared types. Verified on a real capture:gps=0, mini_gps=1573before,gps=1573, mini_gps=0after.SourceOption.from_dict()assigned to a misspelled.sourthe_path, silently dropping an explicitsource_pathpassed alongside apattern.On leap seconds
The two test captures confirm the camera writes true GPS time: the first GPS sample plus the video duration lands 16 s after the
mvhdcreation_time, matching the 18 s GPS−UTC offset less a couple of seconds of file-finalization slop. Interpretingtime_gps_epochas GPS time also makes the derived timestamp agree with the camera's own filename (local capture time) to the second. The leap-second table needs an entry appended if one is ever announced; there has been none since 2017.Testing
pytest tests— 726 passed, 18 skipped, 1 xfailed, 0 failures.ruff check/ruff format --check/usort diff/mypyall clean.tests/unit/test_gps_epoch.pycovers the conversion, both point accessors, GPX sync against CAMM and GoPro videos, invalid timestamps, theelst64-bit fallback, and thesource_pathtypo.tests/unit/test_camm_parser.pyfor the type 5 / type 6 routing.Compatibility note
Description files written by earlier versions for CAMM-native videos carry a GPS-epoch value in
MAPGPSTrack[5]; read back by this version they are interpreted as Unix, i.e. exactly as wrong as before — no regression, but re-runningprocesswill correct them. GPX-sourced description files are unaffected.