bugfix/gracefully-handle-large-thumbnails

Summary:
User report:
mapillary_tools crashes when trying to upload the attached set of images.

Also, there is no mention on which image is causing the crash, meaning the user has to hunt through potentially thousands of images to find the misbehaving one.

Traceback (most recent call last):
File "main.py", line 8, in <module>
File "mapillary_tools\commands\__main__.py", line 164, in main
File "mapillary_tools\commands\process_and_upload.py", line 33, in run
File "mapillary_tools\commands\upload.py", line 75, in run
File "mapillary_tools\upload.py", line 117, in upload
File "mapillary_tools\upload.py", line 108, in upload
File "mapillary_tools\upload.py", line 631, in _continue_or_fail
File "mapillary_tools\uploader.py", line 517, in upload_images
File "mapillary_tools\uploader.py", line 543, in _upload_sequence
File "concurrent\futures\_base.py", line 619, in result_iterator
File "concurrent\futures\_base.py", line 317, in _result_or_cancel
File "concurrent\futures\_base.py", line 449, in result
File "concurrent\futures\_base.py", line 401, in __get_result
File "concurrent\futures\thread.py", line 59, in run
File "mapillary_tools\uploader.py", line 602, in upload
File "mapillary_tools\uploader.py", line 643, in dump_image_bytes
File "mapillary_tools\exif_write.py", line 201, in dump_image_bytes
File "mapillary_tools\exif_write.py", line 186, in _safe_dump
File "mapillary_tools\exif_write.py", line 153, in _safe_dump
File "piexif\_dump.py", line 94, in dump
ValueError: Given thumbnail is too large. max 64kB
[PYI-9472:ERROR] Failed to execute script 'main' due to unhandled exception!

"But I guess the actual problems are quite clear: (1) ValueError: Given thumbnail is too large. max 64kB, Mapillary needs to gracefully handle that situation and (2) on file errors, Mapillary needs to report the failing image name."

Test Plan:
Added unit tests, ran the existing ones as described in README.

It looks like the tests get stuck on
tests/unit/test_persistent_cache.py::test_multithread_shared_cache_comprehensive

but I believe this is unrelated to the change.

To reproduce test getting stuck run:

```
pytest -s -vv tests
```
and observe it getting stuck after 181 tests.

Author: Balys Anikevicius

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/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:

pyproject.toml

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

tests/unit/test_exifedit.py

@@ -1,13 +1,16 @@
 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 +263,93 @@ 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),
+        }
+
+        # Without the fix, this would raise: ValueError: Given thumbnail is too large. max 64kB
+        # With the fix, it should remove the thumbnail and succeed
+        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}",
+        )
+        # If 1st exists, it should be empty or not contain thumbnail reference data
+        if "1st" in output_exif:
+            # If 1st exists, it should be empty or minimal
+            self.assertNotIn(
+                piexif.ImageIFD.JPEGInterchangeFormat,
+                output_exif.get("1st", {}),
+                "Thumbnail metadata should have been removed",
+            )
+
 
 def test_exif_write(tmpdir: py.path.local):
     image_dir = tmpdir.mkdir("images")