Fail only the video whose points are too far apart to upload

mapillary_tools/camm/camm_builder.py

@@ -92,6 +92,25 @@ def _create_edit_list_from_points(
     }
 
 
+class SampleDurationOverflowError(ValueError):
+    """
+    Two consecutive measurements are too far apart for a CAMM track to store
+    the time between them: a sample duration is a 32-bit count of the media
+    timescale
+    """
+
+
+def _sample_duration_overflow_message(
+    time: float, next_time: float, timescale: int
+) -> str:
+    return (
+        f"Points at {time:.3f} s and {next_time:.3f} s of the video are "
+        f"{next_time - time:.3f} s apart, more than the "
+        f"{builder.UINT32_MAX / timescale:.3f} s that can be stored between "
+        "two points in the uploaded video"
+    )
+
+
 def convert_telemetry_to_raw_samples(
     measurements: T.Sequence[camm_parser.TelemetryMeasurement],
     timescale: int,
@@ -104,9 +123,15 @@ def convert_telemetry_to_raw_samples(
         else:
             timedelta = 0
 
-        assert 0 <= timedelta <= builder.UINT32_MAX, (
-            f"expected timedelta {timedelta} between {measurements[idx]} and {measurements[idx + 1]} with timescale {timescale} to be <= UINT32_MAX"
+        assert 0 <= timedelta, (
+            f"expected measurements sorted by time but got {measurement} before {measurements[idx + 1]}"
         )
+        if builder.UINT32_MAX < timedelta:
+            raise SampleDurationOverflowError(
+                _sample_duration_overflow_message(
+                    measurement.time, measurements[idx + 1].time, timescale
+                )
+            )
 
         yield sample_parser.RawSample(
             # will update later

mapillary_tools/uploader.py

@@ -165,6 +165,13 @@ class InvalidMapillaryZipFileError(SequenceError):
     pass
 
 
+class GPSPointsTooFarApartError(SequenceError):
+    """
+    Two consecutive points of a video are too far apart in time to be stored
+    in the uploaded video
+    """
+
+
 # BELOW demonstrates the pseudocode for a typical upload workflow
 # and when upload events are emitted
 #################################################################
@@ -285,7 +292,16 @@ def build_camm_stream(cls, video_metadata: types.VideoMetadata):
 
         with video_metadata.filename.open("rb") as src_fp:
             # Build the mp4 stream with the CAMM samples
-            yield simple_mp4_builder.transform_mp4(src_fp, camm_sample_generator)
+            try:
+                camm_fp = simple_mp4_builder.transform_mp4(
+                    src_fp, camm_sample_generator
+                )
+            except camm_builder.SampleDurationOverflowError as ex:
+                # Fail this video only: the others can still be uploaded
+                raise GPSPointsTooFarApartError(
+                    f"{video_metadata.filename.name}: {ex}"
+                ) from ex
+            yield camm_fp
 
     @classmethod
     def prepare_camm_info(

tests/unit/test_camm_parser.py

@@ -8,6 +8,7 @@
 import typing as T
 from pathlib import Path
 
+import pytest
 from mapillary_tools import geo, telemetry, types, uploader
 from mapillary_tools.camm import camm_builder, camm_parser
 from mapillary_tools.mp4 import (
@@ -859,3 +860,27 @@ def test_gpspoint_timestamps_round_trip_as_unix():
     decoded = encode_decode_empty_camm_mp4(metadata).points
 
     assert T.cast(telemetry.CAMMGPSPoint, decoded[0]).epoch_time == unix_time
+
+
+def test_sample_duration_overflow():
+    def _point(time: float) -> geo.Point:
+        return geo.Point(time=time, lat=1, lon=1, alt=None, angle=None)
+
+    # The longest sample duration that fits 32 bits
+    samples = list(
+        camm_builder.convert_telemetry_to_raw_samples(
+            [_point(0), _point(simple_mp4_builder.UINT32_MAX)], 1
+        )
+    )
+    assert [simple_mp4_builder.UINT32_MAX, 0] == [s.timedelta for s in samples]
+
+    # A tick longer fails with an error naming both points, not an assertion
+    with pytest.raises(
+        camm_builder.SampleDurationOverflowError,
+        match=r"Points at 0\.000 s and 4294967296\.000 s of the video",
+    ):
+        list(
+            camm_builder.convert_telemetry_to_raw_samples(
+                [_point(0), _point(simple_mp4_builder.UINT32_MAX + 1)], 1
+            )
+        )

tests/unit/test_uploader.py

@@ -10,7 +10,8 @@
 
 import py.path
 import pytest
-from mapillary_tools import api_v4, uploader
+from mapillary_tools import api_v4, geo, types, upload, uploader
+from mapillary_tools.mp4 import construct_mp4_parser as cparser
 from mapillary_tools.serializer import description
 
 from ..integration.fixtures import extract_all_uploaded_descs, setup_upload
@@ -225,6 +226,72 @@ def test_upload_blackvue(tmpdir: py.path.local, setup_upload: py.path.local):
         assert fp.read() == b"this is a fake video"
 
 
+def _write_empty_mp4(path: Path, timescale: int) -> None:
+    mvhd: cparser.BoxDict = {
+        "type": b"mvhd",
+        "data": {
+            "creation_time": 0,
+            "modification_time": 0,
+            "timescale": timescale,
+            "duration": 10 * timescale,
+        },
+    }
+    path.write_bytes(
+        cparser.MP4WithoutSTBLBuilderConstruct.build_boxlist(
+            [{"type": b"ftyp", "data": b"test"}, {"type": b"moov", "data": [mvhd]}]
+        )
+    )
+
+
+def test_upload_continues_after_points_too_far_apart(
+    tmpdir: py.path.local, setup_upload: py.path.local, caplog
+):
+    video_dir = Path(tmpdir.mkdir("videos"))
+    metadatas = []
+    # 79740 s is 4784400000 ticks of 1/60000 s, more than 32 bits can hold
+    for name, last_time in [("a_bad.mp4", 79740.0), ("b_good.mp4", 1.0)]:
+        _write_empty_mp4(video_dir / name, 60000)
+        metadatas.append(
+            types.VideoMetadata(
+                video_dir / name,
+                types.FileType.VIDEO,
+                points=[
+                    geo.Point(time=0.0, lat=1, lon=1, alt=None, angle=None),
+                    geo.Point(time=last_time, lat=1.0001, lon=1, alt=None, angle=None),
+                ],
+            )
+        )
+
+    results = {
+        metadata.filename.name: result
+        for metadata, result in uploader.VideoUploader.upload_videos(
+            uploader.Uploader(
+                uploader.UploadOptions(
+                    {"user_upload_token": "YOUR_USER_ACCESS_TOKEN"}, dry_run=True
+                )
+            ),
+            metadatas,
+        )
+    }
+    assert isinstance(results["a_bad.mp4"].error, uploader.GPSPointsTooFarApartError)
+    assert "a_bad.mp4: Points at 0.000 s and 79740.000 s of the video" in str(
+        results["a_bad.mp4"].error
+    )
+    assert results["b_good.mp4"].error is None
+    assert results["b_good.mp4"].result == "0"
+
+    # The whole upload goes on to the next video instead of stopping
+    upload.upload(
+        video_dir,
+        {"user_upload_token": "YOUR_USER_ACCESS_TOKEN"},
+        num_upload_workers=1,
+        _metadatas_from_process=metadatas,
+        reupload=True,
+        dry_run=True,
+    )
+    assert "GPSPointsTooFarApartError: a_bad.mp4" in caplog.text
+
+
 def test_upload_zip_with_emitter(
     setup_unittest_data: py.path.local, setup_upload: py.path.local
 ):