Fix GPS epoch vs Unix epoch mix-up when geotagging CAMM videos - #828
Fix GPS epoch vs Unix epoch mix-up when geotagging CAMM videos#828caglarpir wants to merge 2 commits 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.
Follow-up to review of the previous commit. Treating time_gps_epoch as uniformly GPS time was wrong, and broke two things. What producers actually write in CAMM type 6 time_gps_epoch, measured across 608 videos in the device corpus: Labpano Pilot One / Pilot Era / PanoX V2 GPS time 14 files Insta360 Pro Unix time 1 file mapillary_tools itself Unix time GPMF / NMEA sources (converted on write) Unix time Defect 1, read side: converting unconditionally put Insta360 Pro at 2030 and CAMM written by mapillary_tools at 2033. Defect 2, write side: prepare_camm_info() converted GoPro/BlackVue/NMEA timestamps to GPS time on the way out, so the uploaded artifact carried capture times ~10 years in the past (a 2022-06-17 GoPro recording came back as 2012-06-12). Read and write were inverses of each other, so the round trip looked fine while being incompatible with every released version. This is the higher severity of the two: it ships wrong data, not just a red test. Instead: - CAMMGPSPoint.time_gps_epoch is renamed to epoch_time and now always holds Unix time, the same meaning GPSPoint.epoch_time already had. The old name described the CAMM box field, not the value in memory, which is what made both defects easy to write. - The conversion happens exactly once, in camm_parser, keyed on the camera make, right after the samples are parsed. Nowhere else. - The serializer writes epoch_time straight through, so on-disk stays Unix time exactly as every released version writes it. No format change, no migration needed, files stay readable both ways. - get_gps_epoch_time() and unix_to_gps_epoch() are gone; nothing needed GPS time once the boundary was fixed. gps_epoch_to_unix() now has a single caller. A camera writing GPS time that is not on the make list would be silently wrong, so parsing also warns when the first GPS timestamp sits almost exactly one GPS epoch from the container creation_time. It checks for that specific distance rather than general implausibility because some cameras write a meaningless creation_time -- a GoPro HERO7 recorded in 2022 reports 2016 -- which a generic bound would flag constantly.
|
Pushed What producers actually writeSwept all 608 videos in the device corpus. Only four CAMM type-6 producers exist in it, and they split exactly as the review said:
Both defects reproduced, then fixedDefect 1 (read). Confirmed: Insta360 Pro parsed as 2030. Now 2020-04-02, matching its container to within the 3 h of local time that camera writes into Defect 2 (write). Reproduced exactly on a real GoPro HERO7 — source 2022-06-17, generated CAMM read back as 2012-06-12. The commonality across the affected cases is not a missing-epoch fallback: it is the The fix
Compatibility: no change, and that is verifiedThe mp4 this branch writes is byte-identical (md5) to the one released Acceptance criteria1. Container cross-check — 9/10 devices pass within 24 h. The exception is a GoPro HERO7 whose 2. Round trip is identity — verified end to end on a real GoPro, and covered by two new tests. Re-reading a CAMM written by the current release is correct, trivially so given the byte-identical output above. 3. exiftool agreement is uniform —
Zero for Unix producers, exactly −18 s for both GPS producers — the intended leap offset and nothing else. The GoPro row is not a conversion difference: unfiltered, our first point is 22:00:29, identical to exiftool. The 221 s is 4. Residual check — relative to a golden generated from released One note on the "+18 s only" group of 76: measured against released Safety netA camera writing GPS time that is not on the make list would be silently wrong, so parsing now warns when the first GPS timestamp sits almost exactly one GPS epoch from the container Suite: 733 passed, 18 skipped, 1 xfailed, 0 failures. ruff / usort / mypy clean. Windows CI will stay red until #829 lands (unrelated ffmpeg 9 issue). |
1 participant
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.