diff --git a/negpy/features/metadata/writer.py b/negpy/features/metadata/writer.py index 5f0057eb..06da48ac 100644 --- a/negpy/features/metadata/writer.py +++ b/negpy/features/metadata/writer.py @@ -140,6 +140,41 @@ def _short_overflows(value) -> bool: return result +# IFD0 tags copied from an embedded RAW preview/thumbnail IFD. Valid inside the RAW +# container but invalid in a standalone JPEG APP1 block — ExifTool fails with +# "Can't read SubIFD data" / "Error reading StripOffsets data" when they remain. +_JPEG_STRIP_0TH = frozenset( + { + 254, # NewSubfileType + 256, + 257, + 258, + 259, + 262, # preview image structure + 273, + 277, + 278, + 279, + 284, # strip layout + 330, # SubIFDs + 513, + 514, # JpegIFOffset / JpegIFByteCount + } +) + + +def _prepare_jpeg_exif(exif_dict: dict) -> dict: + """Sanitize source EXIF and drop RAW preview IFD baggage before JPEG serialization.""" + prepared = _sanitize_exif(exif_dict) + prepared.pop("thumbnail", None) + prepared["1st"] = {} + zeroth = prepared.get("0th") + if isinstance(zeroth, dict): + for tag in _JPEG_STRIP_0TH: + zeroth.pop(tag, None) + return prepared + + def embed_metadata( image_bytes: bytes, config: MetadataConfig, @@ -209,7 +244,7 @@ def _dump_exif_within_app1_limit(merged: dict, config: MetadataConfig) -> bytes: ImageDescription) still overflows, falls back to NegPy's own small fields, and finally to orientation-only — which is guaranteed to fit, so piexif.insert can never overflow. """ - candidate = _sanitize_exif(merged) + candidate = _prepare_jpeg_exif(merged) def _fits() -> Optional[bytes]: # Treat both an oversized result and a dump failure (e.g. malformed source @@ -224,30 +259,23 @@ def _fits() -> Optional[bytes]: if exif_bytes is not None: return exif_bytes - # 1) Drop the embedded thumbnail (largest blob, and it'd be the un-edited source anyway). - candidate.pop("thumbnail", None) - candidate["1st"] = {} - exif_bytes = _fits() - if exif_bytes is not None: - return exif_bytes - - # 2) Drop MakerNote (can be tens of KB on some bodies). + # 1) Drop MakerNote (can be tens of KB on some bodies). if isinstance(candidate.get("Exif"), dict): candidate["Exif"].pop(piexif.ExifIFD.MakerNote, None) exif_bytes = _fits() if exif_bytes is not None: return exif_bytes - # 3) Source EXIF still too big (e.g. a bloated ImageDescription/XMP/GPS): discard it and + # 2) Source EXIF still too big (e.g. a bloated ImageDescription/XMP/GPS): discard it and # keep only NegPy's own fields, which are always small. _log.warning("source EXIF too large for JPEG APP1; keeping only NegPy metadata") - candidate = _sanitize_exif(_build_custom_exif(config)) + candidate = _prepare_jpeg_exif(_build_custom_exif(config)) candidate.setdefault("0th", {})[piexif.ImageIFD.Orientation] = 1 exif_bytes = _fits() if exif_bytes is not None: return exif_bytes - # 4) Absolute floor — orientation only. Cannot exceed the limit. + # 3) Absolute floor — orientation only. Cannot exceed the limit. candidate = {"0th": {piexif.ImageIFD.Orientation: 1}, "Exif": {}, "GPS": {}, "Interop": {}, "1st": {}} return piexif.dump(candidate) diff --git a/tests/test_metadata_writer.py b/tests/test_metadata_writer.py index ef833b08..163c88f2 100644 --- a/tests/test_metadata_writer.py +++ b/tests/test_metadata_writer.py @@ -4,6 +4,8 @@ """ import io +import shutil +import subprocess import piexif import pytest @@ -12,6 +14,8 @@ from negpy.features.metadata.models import MetadataConfig from negpy.features.metadata.writer import embed_metadata +_RAW_PREVIEW_0TH_TAGS = (330, 273, 279, 256, 257, 513, 514) + def _jpeg() -> bytes: buf = io.BytesIO() @@ -19,6 +23,64 @@ def _jpeg() -> bytes: return buf.getvalue() +def _raw_like_source_exif() -> dict: + """Synthetic EXIF mimicking piexif.load() output from a Nikon RAW preview IFD.""" + return { + "0th": { + piexif.ImageIFD.Make: b"NIKON CORPORATION", + piexif.ImageIFD.Model: b"NIKON D750", + 330: (12894, 13012, 13238), + 273: 210440, + 279: 57600, + 256: 160, + 257: 120, + 513: 999, + 514: 12345, + }, + "Exif": { + piexif.ExifIFD.ExposureTime: (1, 640), + piexif.ExifIFD.FNumber: (56, 10), + piexif.ExifIFD.ISOSpeedRatings: 100, + piexif.ExifIFD.FocalLengthIn35mmFilm: 60, + piexif.ExifIFD.DateTimeOriginal: b"2026:07:03 18:51:59", + }, + "GPS": {}, + "Interop": {}, + "1st": {}, + } + + +def test_embed_strips_raw_preview_ifd_tags_from_jpeg() -> None: + """RAW EXIF carries embedded preview IFD0 tags that break ExifTool on exported JPEGs.""" + source_exif = _raw_like_source_exif() + + out = embed_metadata(_jpeg(), MetadataConfig(), source_exif) + + loaded = piexif.load(out) + zeroth = loaded["0th"] + for tag in _RAW_PREVIEW_0TH_TAGS: + assert tag not in zeroth + assert zeroth[piexif.ImageIFD.Make] == b"NIKON CORPORATION" + assert loaded["Exif"][piexif.ExifIFD.FocalLengthIn35mmFilm] == 60 + + +@pytest.mark.skipif(not shutil.which("exiftool"), reason="exiftool not installed") +def test_embed_jpeg_exiftool_can_write_user_comment(tmp_path) -> None: + """Regression: exported JPEG EXIF must be writable by ExifTool (issue 0.32.1).""" + jpeg = embed_metadata(_jpeg(), MetadataConfig(), _raw_like_source_exif()) + path = tmp_path / "export.jpg" + path.write_bytes(jpeg) + + result = subprocess.run( + ["exiftool", "-overwrite_original", "-UserComment=foo", str(path)], + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr or result.stdout + assert "Error" not in (result.stderr or "") + + def test_embed_handles_oversized_exif_without_dropping_metadata() -> None: # Source EXIF far larger than the 64 KB APP1 limit (fat thumbnail + MakerNote). source_exif = {