bugfix/gracefully-handle-large-thumbnails

mapillary_tools/authenticate.py

@@ -7,7 +7,6 @@
 import typing as T
 
 import jsonschema
-
 import requests
 
 from . import api_v4, config, constants, exceptions, http

mapillary_tools/blackvue_parser.py

@@ -1,7 +1,6 @@
 from __future__ import annotations
 
 import dataclasses
-
 import json
 import logging
 import re

mapillary_tools/camm/camm_builder.py

@@ -9,7 +9,6 @@
     mp4_sample_parser as sample_parser,
     simple_mp4_builder as builder,
 )
-
 from . import camm_parser
 
 

mapillary_tools/commands/process_and_upload.py

@@ -1,7 +1,6 @@
 import inspect
 
 from ..authenticate import fetch_user_items
-
 from .process import Command as ProcessCommand
 from .upload import Command as UploadCommand
 

mapillary_tools/commands/video_process_and_upload.py

@@ -1,7 +1,6 @@
 import inspect
 
 from ..authenticate import fetch_user_items
-
 from .upload import Command as UploadCommand
 from .video_process import Command as VideoProcessCommand
 

mapillary_tools/exif_write.py

@@ -214,6 +214,18 @@ def _safe_dump(self) -> bytes:
                     else:
                         del self._ef[ifd][tag]
                         # retry later
+                elif "thumbnail is too large" in message.lower():
+                    # Handle oversized thumbnails (max 64kB per EXIF spec)
+                    if thumbnail_removed:
+                        raise exc
+                    LOG.debug(
+                        "Thumbnail too large (max 64kB) -- removing thumbnail and 1st: %s",
+                        exc,
+                    )
+                    del self._ef["thumbnail"]
+                    del self._ef["1st"]
+                    thumbnail_removed = True
+                    # retry later
                 else:
                     raise exc
             except Exception as exc:

mapillary_tools/http.py

@@ -1,7 +1,6 @@
 from __future__ import annotations
 
 import logging
-
 import ssl
 import sys
 import typing as T

mapillary_tools/serializer/gpx.py

@@ -12,7 +12,6 @@
 import gpxpy.gpx
 
 from .. import geo, types
-
 from ..telemetry import CAMMGPSPoint, GPSPoint
 from ..types import (
     BaseSerializer,

mapillary_tools/uploader.py

@@ -797,6 +797,10 @@ def dump_image_bytes(cls, metadata: types.ImageMetadata) -> bytes:
             raise ExifError(
                 f"Failed to dump EXIF bytes: {ex}", metadata.filename
             ) from ex
+        except ValueError as ex:
+            raise ExifError(
+                f"Failed to dump EXIF bytes: {ex}", metadata.filename
+            ) from ex
 
     # Thread-safe
     def _get_cached_file_handle(self, key: str) -> str | None:

mapillary_tools/utils.py

@@ -1,7 +1,6 @@
 from __future__ import annotations
 
 import concurrent.futures
-
 import hashlib
 import logging
 import os

pyproject.toml

@@ -58,6 +58,7 @@ include = ["mapillary_tools*"]
 [dependency-groups]
 dev = [
     "mypy",
+    "Pillow",
     "pyinstaller",
     "pyre-check",
     "pytest",

tests/cli/blackvue_parser.py

@@ -7,7 +7,6 @@
 
 import gpxpy
 import gpxpy.gpx
-
 from mapillary_tools import blackvue_parser, geo, utils
 
 

tests/cli/camm_parser.py

@@ -7,7 +7,6 @@
 
 import gpxpy
 import gpxpy.gpx
-
 from mapillary_tools import geo, utils
 from mapillary_tools.camm import camm_parser
 

tests/cli/gpmf_parser.py

@@ -11,7 +11,6 @@
 import gpxpy
 import gpxpy.gpx
 import mapillary_tools.geo as geo
-
 import mapillary_tools.gpmf.gpmf_parser as gpmf_parser
 import mapillary_tools.gpmf.gps_filter as gps_filter
 import mapillary_tools.telemetry as telemetry

tests/cli/gps_filter.py

@@ -5,7 +5,6 @@
 import typing as T
 
 import gpxpy
-
 from mapillary_tools import constants, geo, telemetry
 from mapillary_tools.gpmf import gps_filter
 

tests/cli/simple_mp4_builder.py

@@ -2,7 +2,6 @@
 from pathlib import Path
 
 from mapillary_tools.camm import camm_builder
-
 from mapillary_tools.geotag import geotag_videos_from_video
 from mapillary_tools.mp4 import simple_mp4_builder as builder
 

tests/cli/upload_api_v4.py

@@ -7,7 +7,6 @@
 import requests
 import tqdm
 from mapillary_tools import api_v4, authenticate, http
-
 from mapillary_tools.upload_api_v4 import FakeUploadService, UploadService
 
 

tests/integration/fixtures.py

@@ -12,10 +12,8 @@
 
 import exifread
 import jsonschema
-
 import py.path
 import pytest
-
 from mapillary_tools import upload_api_v4, utils
 
 EXECUTABLE = os.getenv(

tests/integration/test_video_process.py

@@ -4,7 +4,6 @@
 from pathlib import Path
 
 import exifread
-
 import py.path
 import pytest
 

tests/unit/test_config.py

@@ -1,7 +1,6 @@
 import typing as T
 
 import py.path
-
 from mapillary_tools import config
 
 

tests/unit/test_exifedit.py

@@ -1,13 +1,15 @@
 import datetime
+import io
 import os
 import shutil
 import unittest
 from pathlib import Path
 
+import piexif
 import py.path
-
 from mapillary_tools.exif_read import ExifRead
 from mapillary_tools.exif_write import ExifEdit
+from PIL import Image
 
 this_file = Path(__file__)
 this_file_dir = this_file.parent
@@ -260,6 +262,90 @@ def test_add_repeatedly_time_original_corrupt_exif(self):
     def test_add_repeatedly_time_original_corrupt_exif_2(self):
         add_repeatedly_time_original_general(self, CORRUPT_EXIF_FILE_2)
 
+    def test_large_thumbnail_handling(self):
+        """Test that images with thumbnails larger than 64kB are handled gracefully."""
+        # Create a test image with a large thumbnail (>64kB)
+        test_image_path = data_dir.joinpath("tmp", "large_thumbnail.jpg")
+
+        # Create a simple test image
+        img = Image.new("RGB", (100, 100), color="red")
+        img.save(test_image_path, "JPEG")
+
+        # Create a large thumbnail (>64kB) by creating a high-quality large thumbnail
+        # Use a larger size and add noise to make it incompressible
+        large_thumbnail = Image.new("RGB", (2048, 2048))
+        # Fill with random-like data to prevent compression
+        pixels = large_thumbnail.load()
+        for i in range(2048):
+            for j in range(2048):
+                pixels[i, j] = (
+                    (i * 7 + j * 13) % 256,
+                    (i * 11 + j * 17) % 256,
+                    (i * 19 + j * 23) % 256,
+                )
+
+        thumbnail_bytes = io.BytesIO()
+        large_thumbnail.save(thumbnail_bytes, "JPEG", quality=100)
+        thumbnail_data = thumbnail_bytes.getvalue()
+
+        # Verify thumbnail is larger than 64kB
+        self.assertGreater(
+            len(thumbnail_data),
+            64 * 1024,
+            f"Test thumbnail should be larger than 64kB but got {len(thumbnail_data)} bytes",
+        )
+
+        # Load the image and add GPS data first
+        exif_edit = ExifEdit(test_image_path)
+        test_latitude = 50.5475894785
+        test_longitude = 15.595866685
+        exif_edit.add_lat_lon(test_latitude, test_longitude)
+
+        # Manually insert the large thumbnail into the internal EXIF structure
+        # This simulates what would happen if an image came in with a large thumbnail
+        exif_edit._ef["thumbnail"] = thumbnail_data
+        exif_edit._ef["1st"] = {
+            piexif.ImageIFD.Compression: 6,
+            piexif.ImageIFD.XResolution: (72, 1),
+            piexif.ImageIFD.YResolution: (72, 1),
+            piexif.ImageIFD.ResolutionUnit: 2,
+            piexif.ImageIFD.JPEGInterchangeFormat: 0,
+            piexif.ImageIFD.JPEGInterchangeFormatLength: len(thumbnail_data),
+        }
+
+        # Given thumbnail is too large, max 64kB, thumbnail and 1st metadata should be removed.
+        image_bytes = exif_edit.dump_image_bytes()
+
+        # Verify the output is valid
+        self.assertIsNotNone(image_bytes)
+        self.assertGreater(len(image_bytes), 0)
+
+        # Verify the resulting image is a valid JPEG
+        result_image = Image.open(io.BytesIO(image_bytes))
+        self.assertEqual(result_image.format, "JPEG")
+        self.assertEqual(result_image.size, (100, 100))
+
+        # Verify we can read the GPS data from the result
+        output_exif = piexif.load(image_bytes)
+        self.assertIn("GPS", output_exif)
+        self.assertIn(piexif.GPSIFD.GPSLatitude, output_exif["GPS"])
+        self.assertIn(piexif.GPSIFD.GPSLongitude, output_exif["GPS"])
+
+        # CRITICAL: Verify the large thumbnail was actually removed
+        # The fix should have deleted both "thumbnail" and "1st" to handle the error
+        # piexif.load() may include "thumbnail": None after removal
+        thumbnail_value = output_exif.get("thumbnail")
+        self.assertTrue(
+            thumbnail_value is None or thumbnail_value == b"",
+            f"Large thumbnail should have been removed but got: {thumbnail_value[:100] if thumbnail_value else None}",
+        )
+
+        first_value = output_exif.get("1st")
+        self.assertTrue(
+            first_value is None or first_value == {} or len(first_value) == 0,
+            f"1st metadata should have been removed but got: {first_value}",
+        )
+
 
 def test_exif_write(tmpdir: py.path.local):
     image_dir = tmpdir.mkdir("images")

tests/unit/test_exifread.py

@@ -1,14 +1,11 @@
 import datetime
 import os
-
 import typing as T
 from pathlib import Path
 
 import py.path
-
 import pytest
 from mapillary_tools import geo
-
 from mapillary_tools.exif_read import (
     _parse_coord,
     ExifRead,

tests/unit/test_ffmpeg.py

@@ -3,9 +3,7 @@
 from pathlib import Path
 
 import py.path
-
 import pytest
-
 from mapillary_tools import ffmpeg
 
 from ..integration.fixtures import pytest_skip_if_not_ffmpeg_installed, setup_data

tests/unit/test_persistent_cache.py

@@ -6,7 +6,6 @@
 import traceback
 
 import pytest
-
 from mapillary_tools.history import PersistentCache
 
 

tests/unit/test_sample_video.py

@@ -9,7 +9,6 @@
 
 import py.path
 import pytest
-
 from mapillary_tools import exif_read, ffmpeg, sample_video
 from mapillary_tools.serializer import description
 

tests/unit/test_sequence_processing.py

@@ -6,7 +6,6 @@
 
 import py.path
 import pytest
-
 from mapillary_tools import (
     exceptions,
     geo,

tests/unit/test_store.py

@@ -3,7 +3,6 @@
 import tempfile
 
 import pytest
-
 from mapillary_tools.store import KeyValueStore
 
 

tests/unit/test_upload_api_v4.py

@@ -2,7 +2,6 @@
 from pathlib import Path
 
 import py
-
 from mapillary_tools import upload_api_v4
 
 

tests/unit/test_uploader.py

@@ -5,7 +5,6 @@
 
 import py.path
 import pytest
-
 from mapillary_tools import api_v4, uploader
 from mapillary_tools.serializer import description
 
@@ -619,7 +618,6 @@ def test_image_sequence_uploader_multiple_sequences(
         test_exif.copy(test_exif2)
         test_exif3 = setup_unittest_data.join("test_exif_3.jpg")
         test_exif.copy(test_exif3)
-        fixed_exif = setup_unittest_data.join("fixed_exif.jpg")
 
         # Create metadata for multiple sequences
         image_metadatas = [

tests/unit/test_utils.py

@@ -2,7 +2,6 @@
 from pathlib import Path
 
 import py.path
-
 from mapillary_tools import utils