Repository navigation
fix: write EXIF metadata and the sRGB profile into TIFF exports - #1752
FabianWeiss90 wants to merge 8 commits into
Conversation
The TIFF encoder needs to write EXIF tags itself, which image does not expose. Pin it to the version image resolves and fail the build if the two ever drift apart into two copies of the crate.
Patching a finished TIFF invalidates the strip offsets stored in its IFD, so metadata writing was skipped for the format. Write the tags while encoding instead, into a proper Exif and GPS sub-IFD, before the strip offsets are laid out. Copying metadata from a TIFF source now skips the tags describing its pixel layout, which is what the separate source-TIFF guard protected against.
Denoising, negative conversion, panorama stitching, HDR merging and focus stacking wrote their output without any metadata, leaving it readable only through RapidRAW's own sidecar. Embed what that sidecar already records, so other tools see it too. Panorama and HDR keep their 32-bit float data, so the TIFF encoder learns that sample format here.
exiftool -validate flagged two issues in the written files. ExifImageWidth and ExifImageHeight are not allowed in TIFF, which states its size in IFD0, so they are left out. Odd-length text values pushed everything written after them onto an odd offset, so ASCII values are padded to even length.
|
Heads-up on an overlap: I have a small change ready that embeds an sRGB ICC profile (CC0, 736 bytes) in JPEG/PNG/TIFF exports for #1489, via |
|
Thanks for the heads-up — good to catch that early. I'd go with your first option: let this land, then add the ICC tag on top. After this PR the TIFF tags all get written in one place, and the Heads-up for the follow-up: If the maintainer would rather take yours first, fine by me — then I'll rebase and add the tag myself, with your profile and credit. |
|
Sounds good, thanks. I'll send the JPEG/PNG part on its own now and add the ICC tag in |
Exports are sRGB-encoded but carried no color profile, so color-managed apps and print services had to assume sRGB (#1489). Embed the CC0 sRGB-v2-magic profile (736 bytes, from saucecontrol/Compact-ICC-Profiles) through image's ImageEncoder::set_icc_profile. Pixel data is unchanged. TIFF follows once #1752 lands; WebP, AVIF and JPEG XL are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
A related EXIF finding you may want for this PR, since it touches the same file: #1161, where the UserComment is written without its 8-byte character-code header. Readers then report "invalid charset" and drop the first 8 characters. The fix is small (+14/−2 in Happy for you to fold it into #1752, or I'll open a separate PR after #1752 lands, whichever you prefer. (Found and written by an AI agent, like my other PRs.) |
# Conflicts: # src-tauri/src/export_processing.rs # src-tauri/src/lib.rs
|
@yojen7 your ICC PR landed first, so the order we discussed is reversed — I've merged main into this branch and your profile is in here now. TIFF is still untagged, and since this PR replaces that branch with its own writer, the tag has to go in there: |
JPEG and PNG exports carry the profile since it was added to the repo, but TIFF could not: it no longer goes through image's encoder, which is what applies it. Write the tag directly instead, in the main IFD and with the UNDEFINED type the specification asks for. This covers the conversion outputs as well, since they share the writer.
|
Please go ahead and add it here, thanks. One PR for #1489 is simpler. Once it's in, I'll check that exported TIFFs carry the profile and that it matches what the JPEG/PNG exports embed. |
FabianWeiss90
changed the title
|
@yojen7 Done. As soon as this PR lands, it should be in there |
CyberTimon#1897 split write_image_with_metadata into write_exif_metadata and embed_xmp, while this branch had split the same function differently to get at the gathered metadata before encoding. Resolved in favour of the upstream structure: the gathering step is extracted out of write_exif_metadata into collect_metadata_from_source again, which the TIFF encoder calls before writing any pixels. ExifVersion and FlashpixVersion, which CyberTimon#1897 adds, sit in that shared step now, so TIFF carries them as well and exiftool -validate passes without warnings.
|
Resolved conflicts with current upstream changes. |
CyberTimon#1910 added a DPI setting for print exports and, for TIFF, started encoding through the `tiff` crate to write the resolution tags — the same technique this branch uses for EXIF. That left two TIFF writers side by side, so they are merged into one: `encode_tiff_with_metadata` takes the DPI and records it as the image resolution, and the `write_tiff` helper added upstream is gone. The resolution tags for JPEG and WebP move to `write_exif_metadata`, since TIFF gets its resolution from the encoder rather than from the patched EXIF block, and the structural-tag filter would drop them anyway.
2 participants
Description
Fixes #1750. Also fixes #1489 for TIFF, with @yojen7's agreement — see below.
TIFF exports carried no metadata, because metadata is patched into the encoded
bytes afterwards with
little_exif— which is safe for JPEG/PNG/WebP, but notfor TIFF: its pixel data is addressed through absolute
StripOffsetsstored inthe IFD, so inserting entries into a finished file moves the pixel data away
from the offsets pointing at it. Hence the
FIXMEguard that skipped theformat.
This writes the tags while encoding instead, through the
tiffcrate'slow-level encoder (the same crate
imagealready uses, and the same mechanismimage's ownTiffEncoder::set_icc_profile()uses internally). Because everytag value is known before a single pixel is written, the Exif and GPS sub-IFDs
are written first, their offsets are kept, and the main IFD then points at
them — all before
write_data()lays out the strips. Nothing needs patching,so nothing can be corrupted.
The result is spec-shaped: shooting data in a proper Exif sub-IFD (
0x8769),GPS in its own (
0x8825), descriptive tags in IFD0 — not flattened into themain IFD.
One dependency change worth a look:
tiffbecomes a direct dependency. Itis already in the binary through
image, and is pinned to the same version, sonothing is added — but
build.rsnow fails the build if those versions everdrift apart. Details under Changes Made.
The same fix applies to the outputs that never went through the export path at
all (denoising, negative conversion, panorama stitching, HDR merging, focus
stacking). Those wrote their files with
image::save()and no metadata,leaving the data readable only through RapidRAW's own sidecar. They now embed
exactly what that sidecar already records — sidecar writing itself is
unchanged, so RapidRAW and external tools agree.
Type of Change
Changes Made
src-tauri/src/tiff_metadata.rs: encodes a TIFF with its metadata inthe right IFDs. Supports 8-bit, 16-bit and 32-bit float (panorama and HDR
keep float data).
tiffbecomes a direct dependency, pinned to the versionimagealreadyresolves, so no second copy is built.
imagedoes not expose its TIFFencoder beyond
new()/encode()/set_icc_profile(), so writing custom tagsis not possible through it.
build.rsreadsCargo.lockand fails the buildwith an explicit message if the two versions ever drift apart — otherwise
Cargo would silently resolve two copies of the crate into the binary.
tag's group, number and format are read from
little_exifand written withthe matching TIFF type, so everything the existing gathering stage produces
survives, including what
copy_full_exif_from_source()copies wholesale.ImageWidth,StripOffsets,BitsPerSample,Compression,RowsPerStrip, tile and thumbnail tags,stale sub-IFD pointers) — the encoder writes its own, and copying the
source's would describe the wrong image.
MakerNotegoes too, since it holdsoffsets into the source file.
copying IFD0 wholesale from a TIFF source drags those layout tags into the
output. The filter above removes exactly those, so the guard is no longer
needed — and removing it is what makes a re-export of a denoised or stacked
TIFF keep its metadata.
exif_processing::collect_metadata_from_source(), so it can run beforeencoding instead of after. JPEG/PNG/WebP behaviour is unchanged.
UNDEFINEDtype the specification asks for), so TIFF is no longer the oneexport format without it. Export and the conversions share the writer, so
their TIFFs are covered as well.
through the
tiffcrate as well, to write the resolution tags for its newDPI setting — the same technique this PR uses for EXIF. Rather than leave two
encoders side by side,
encode_tiff_with_metadatanow takes the DPI andrecords it as the image resolution, and the
write_tiffhelper from feat(export): add DPI setting for print exports #1910 isgone. Its resolution tags for JPEG and WebP move into
write_exif_metadata,since TIFF takes its resolution from the encoder and the structural-tag
filter would drop them from a patched EXIF block anyway.
PixelXDimension/PixelYDimensionnow come from the encoded image ratherthan the source, so a resized export reports its own dimensions. In TIFF they
are left out entirely, since TIFF records its size in IFD0 and the Exif
specification does not allow them there —
exiftool -validateflags it.Note on sub-IFD pointers: they are written as
LONG, not as the TIFFIFDtype. The latter is legal TIFF but not what the Exif specification asks for,
and readers reject it (kamadak-exif returns
InvalidFormat("Invalid pointer")).On the ICC profile: #1820 tagged JPEG and PNG but had to leave TIFF out,
because this PR replaces the branch that would have applied it —
image'sencoder is no longer involved there. Since that profile is now in the repo,
the tag is written here directly instead, which closes #1489 for TIFF too.
@yojen7 and I agreed on this order rather than a follow-up PR.
Screenshots/Videos
Testing
Verified manually with a Sony SLT-A77V file: Make, Model, f/4, 1/60 s, ISO 320,
focal length, lens model, capture date, ExifVersion and orientation all present
in the exported TIFF, which decodes at full resolution. Repeated with that TIFF
as the source — the case the removed guard existed for — with correct results
and strip offsets belonging to the new file. Pixel data was checked to survive
unchanged at 8-bit, 16-bit and 32-bit float, and an export with metadata turned
off still produces a file without any.
Checked every output path with exiftool after merging #1897, going through the
real export encoder rather than the TIFF writer alone, with a Canon CR3 as the
source: JPEG, PNG and TIFF exports (8-bit and 16-bit) all carry the sRGB
profile, the camera and lens, a valid capture date and the white balance, as do
the TIFFs the conversions write. Patching EXIF into a finished JPEG or PNG
afterwards leaves the profile intact, the two living in different segments, but
it was worth confirming. An export with metadata switched off still carries the
profile, which seems right: it describes how to interpret the pixels rather than
where the photo came from.
The DPI setting from #1910 was checked through this path as well: at 300 DPI the
TIFF reports
XResolution: 300withResolutionUnit: inches, and without thesetting it keeps the encoder's default, exactly as before.
exiftool -validatereportsValidate: OKon the written TIFFs, with nowarnings at all. That is partly thanks to #1897: it added the
ExifVersionandFlashpixVersiontags the specification requires, and since the merge resolutionputs them in the shared gathering step, TIFF carries them too.
cargo clippy --all-targets -- -D warningsandcargo fmt --checkare clean.Test Configuration:
Checklist
Additional Notes
Rebased onto current main thrice while this was open, which is worth a note
since both times the same code moved underneath it:
write_image_with_metadataintowrite_exif_metadataandembed_xmp, while this branch had split the same function differently toreach the gathered metadata before encoding. Resolved in favour of the
upstream structure, with the gathering step extracted back out into
collect_metadata_from_source(). Its newExifVersionandFlashpixVersiontags live in that shared step now, which is why TIFF validates cleanly.
one as described above.
Four things this PR leaves out on purpose, all easy to add if you would rather
have them here:
image, because that is what their sidecar already records. Merging metadata
across the inputs (keeping only values identical in all of them, leaving the
differing ones empty) would be more correct for exposure time and ISO, but it
changes what those outputs claim, which is a separate decision.
WebP through format-specific embedders, and there is none for TIFF — it would
mean writing tag 700 in the new writer. Worth doing, but it is that PR's
feature rather than this one's.
EXIF but no colour profile, since only exports are tagged today. That leaves
those conversions tagging their TIFF but not their PNG, which is a handful of
lines through
image's PNG encoder.webpcrate's doing rather thanthis code, and it was already the case before.
AI Disclaimer:
Please state the involvement of AI in this PR: