From 0901a7382d139a520e1db300c356a790cd70a6c4 Mon Sep 17 00:00:00 2001 From: jboneng Date: Sun, 5 Jul 2026 09:24:24 +0200 Subject: [PATCH 1/2] Fix exported JPEG EXIF so ExifTool can rewrite tags by stripping stale RAW preview SubIFD pointers. --- negpy/features/metadata/writer.py | 52 ++++++++++++++++++------ tests/test_metadata_writer.py | 66 +++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 12 deletions(-) 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..d0e02d12 100644 --- a/tests/test_metadata_writer.py +++ b/tests/test_metadata_writer.py @@ -4,6 +4,9 @@ """ import io +import shutil +import subprocess +from pathlib import Path import piexif import pytest @@ -11,6 +14,12 @@ from negpy.features.metadata.models import MetadataConfig from negpy.features.metadata.writer import embed_metadata +from negpy.infrastructure.loaders.helpers import read_exif_from_file + +_TEST_IMGS = Path(r"c:\Users\jbone\Desktop\TestImgs") +_NEF_FIXTURES = sorted(_TEST_IMGS.glob("*.NEF")) if _TEST_IMGS.is_dir() else [] + +_RAW_PREVIEW_0TH_TAGS = (330, 273, 279, 256, 257, 513, 514) def _jpeg() -> bytes: @@ -19,6 +28,63 @@ def _jpeg() -> bytes: return buf.getvalue() +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 = { + "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.FocalLengthIn35mmFilm: 60}, + "GPS": {}, + "Interop": {}, + "1st": {}, + } + + 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") +@pytest.mark.skipif(not _NEF_FIXTURES, reason="TestImgs NEF fixtures not available") +@pytest.mark.parametrize("nef_path", _NEF_FIXTURES, ids=lambda p: p.name) +def test_embed_real_raw_exif_exiftool_can_write_user_comment(nef_path: Path) -> None: + """Regression: exported JPEG EXIF must be writable by ExifTool (issue 0.32.1).""" + source_exif = read_exif_from_file(str(nef_path)) + assert source_exif is not None + + jpeg = embed_metadata(_jpeg(), MetadataConfig(), source_exif) + loaded = piexif.load(jpeg) + for tag in _RAW_PREVIEW_0TH_TAGS: + assert tag not in loaded["0th"] + + tmp = nef_path.parent / "negpy_export_test" / f"{nef_path.stem}_verify.jpg" + tmp.parent.mkdir(parents=True, exist_ok=True) + tmp.write_bytes(jpeg) + + result = subprocess.run( + ["exiftool", "-overwrite_original", "-UserComment=foo", str(tmp)], + 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 = { From f838fe4ea65c03e5170b0ba30dffd4e666c7644d Mon Sep 17 00:00:00 2001 From: jboneng Date: Sun, 5 Jul 2026 09:46:06 +0200 Subject: [PATCH 2/2] changed a unit test that I used that relied on test images not in repo, changed it to a self containing synthetic test --- tests/test_metadata_writer.py | 44 ++++++++++++++++------------------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/tests/test_metadata_writer.py b/tests/test_metadata_writer.py index d0e02d12..163c88f2 100644 --- a/tests/test_metadata_writer.py +++ b/tests/test_metadata_writer.py @@ -6,7 +6,6 @@ import io import shutil import subprocess -from pathlib import Path import piexif import pytest @@ -14,10 +13,6 @@ from negpy.features.metadata.models import MetadataConfig from negpy.features.metadata.writer import embed_metadata -from negpy.infrastructure.loaders.helpers import read_exif_from_file - -_TEST_IMGS = Path(r"c:\Users\jbone\Desktop\TestImgs") -_NEF_FIXTURES = sorted(_TEST_IMGS.glob("*.NEF")) if _TEST_IMGS.is_dir() else [] _RAW_PREVIEW_0TH_TAGS = (330, 273, 279, 256, 257, 513, 514) @@ -28,9 +23,9 @@ def _jpeg() -> bytes: return buf.getvalue() -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 = { +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", @@ -42,12 +37,23 @@ def test_embed_strips_raw_preview_ifd_tags_from_jpeg() -> None: 513: 999, 514: 12345, }, - "Exif": {piexif.ExifIFD.FocalLengthIn35mmFilm: 60}, + "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) @@ -59,24 +65,14 @@ def test_embed_strips_raw_preview_ifd_tags_from_jpeg() -> None: @pytest.mark.skipif(not shutil.which("exiftool"), reason="exiftool not installed") -@pytest.mark.skipif(not _NEF_FIXTURES, reason="TestImgs NEF fixtures not available") -@pytest.mark.parametrize("nef_path", _NEF_FIXTURES, ids=lambda p: p.name) -def test_embed_real_raw_exif_exiftool_can_write_user_comment(nef_path: Path) -> None: +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).""" - source_exif = read_exif_from_file(str(nef_path)) - assert source_exif is not None - - jpeg = embed_metadata(_jpeg(), MetadataConfig(), source_exif) - loaded = piexif.load(jpeg) - for tag in _RAW_PREVIEW_0TH_TAGS: - assert tag not in loaded["0th"] - - tmp = nef_path.parent / "negpy_export_test" / f"{nef_path.stem}_verify.jpg" - tmp.parent.mkdir(parents=True, exist_ok=True) - tmp.write_bytes(jpeg) + 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(tmp)], + ["exiftool", "-overwrite_original", "-UserComment=foo", str(path)], capture_output=True, text=True, check=False,