Fix GPS epoch vs Unix epoch mix-up when geotagging CAMM videos - #828
Conversation
|
Pushed What producers actually writeFour CAMM type-6 producers, 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 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). |
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.
4c21b35 to
2e00e71
Compare
Addresses review of the CAMM epoch follow-ups: - Write Unix time whatever the make, as main has since mapillary#828, instead of converting back to GPS time for Labpano. This is not what released versions wrote: v0.14.7 passed a Labpano camera's GPS time through unconverted, 1389017092.966 for the first sample of a PanoX V2 capture from 2024-01-11, where this writes the Unix time 1704981874.966, one GPS epoch less 18 leap seconds later. Our output copies the source creation time, which the reader already uses to tell Unix time from GPS time. Removes denormalize_gps_epochs and unix_to_gps_epoch. - Decide the epoch of a CAMM track by the median of its timestamps, so that one stray sample cannot flip the whole track. - Sync GPX to video time 0 rather than to the first video GPS point, which put the track early by that point's time when the video's GPS started late. - Replace the overlap check with a gap check: silent on overlap, a warning naming both files for a gap up to a day, and an error only beyond that, where no clock or time zone mistake can explain it. The error is now a geotagging error, so the next geotag source gets its turn. - ExifTool leap correction: match the CAMM format case-insensitively, log when it applies, and test it against real ExifTool output of a file we write. - Document the cost of mixing CAMM type 5 and type 6 samples.
1 participant
Summary
The CAMM spec defines the type 6 field
time_gps_epochas seconds since the GPS epoch (1980-01-06), but producers disagree: Labpano cameras write GPS time, while Insta360 and mapillary_tools itself write Unix time. GPX, GPMF and BlackVue all store Unix time. Labpano's GPS time was being compared and rendered as Unix time, which is a 315,964,800 s (~10 year) error.The crash
Geotagging a Labpano 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 Labpano's GPS time 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
CAMM GPS timestamps are converted once, on read (
camm_parser.py,telemetry.py)telemetry.gps_epoch_to_unix()converts GPS time to Unix time, 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.camm_parserconverts CAMM type 6 timestamps for makes that record GPS time:Labpano, matched exactly after trimming and lowercasing. Everything else, including Insta360 and the CAMM tracks mapillary_tools writes, is read as Unix time.CAMMGPSPoint.epoch_timeis therefore always Unix time in memory, the same asGPSPoint.epoch_time.mvhdcreation time.Writes are unchanged
parse_gpx()anduploader.prepare_camm_info()store Unix time, andcamm_builderwrites it into CAMM type 6, as every release has.Consumers go through
get_unix_time()Point.get_unix_time()is the canonical wall-clock accessor. It returnsNonefor a point without an absolute timestamp._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 Labpano CAMM videos — 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, the make-keyed conversion on read,get_unix_time()on both point classes, 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 Labpano CAMM 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.