Commit ac3a9e7
authored
fix: encode_xmp() for aliased metadata names such as IPTC:Rating / xmp:Rating (#5108)
This PR fixes an inconsistency in OIIO's XMP write path that prevents
some metadata fields from being emitted into XMP, even when they are
present in `ImageSpec::extra_attribs`.
**Problem**
`encode_xmp()` gathers attributes through `gather_xmp_attribs()`, which
currently does:
```cpp
const XMPtag* tag = xmp_tagmap_ref().find(p.name());
if (tag) {
if (!Strutil::iequals(p.name(), tag->oiioname))
continue;
...
}
```
At the same time, `xmp_tagmap_ref()` builds its lookup map using
`xmpname`, not `oiioname`.
This becomes self-contradictory for mappings where the XMP name and the
OIIO-facing attribute name differ.
A concrete example from `xmp.cpp` is:
```cpp
{ "xmp:Rating", "IPTC:Rating", TypeDesc::INT, 0 },
```
With the current implementation:
- If the spec contains `IPTC:Rating`, lookup fails because the map is
keyed by `xmpname`.
- If the spec contains `Xmp:Rating` / `xmp:Rating`, lookup succeeds, but
the attribute is then rejected because it does not match `oiioname`.
As a result, `encode_xmp()` does not emit the rating field, and format
writers that rely on `encode_xmp()` (for example JPEG and TIFF) may fail
to write an XMP packet for such metadata.
**Fix**
This patch makes the XMP tag lookup accept both spellings:
- the XMP name (`xmpname`)
- the OIIO metadata name (`oiioname`)
and allows `gather_xmp_attribs()` to accept either as a valid match.
This preserves existing behavior for fields where `xmpname == oiioname`,
while fixing aliased mappings such as rating.
**Why this is correct**
OIIO already models these fields as explicit mappings between an XMP
name and an OIIO attribute name. The write-side gather logic should
therefore be able to consume metadata stored under either spelling.
Without this, some entries in the XMP mapping table are not actually
encodable.
**Observed impact**
In a local reproduction using JPEG output:
- `IPTC:Rating` was present in `ImageSpec::extra_attribs`
- the JPEG writer called `encode_xmp()`
- no XMP APP1 segment was written to the final file
This was traced back to the lookup/match mismatch described above.
**Testing**
I recommend adding tests that cover both direct XMP naming and aliased
OIIO naming.
1. Unit test: `IPTC:Rating` is encoded as XMP rating
```cpp
ImageSpec spec(1, 1, 3, TypeDesc::UINT8);
spec.attribute("IPTC:Rating", 4);
std::string xmp = encode_xmp(spec, true);
CHECK(xmp.find("xmp:Rating=\"4\"") != std::string::npos);
```
2. Unit test: `xmp:Rating` is encoded as XMP rating
```cpp
ImageSpec spec(1, 1, 3, TypeDesc::UINT8);
spec.attribute("xmp:Rating", 4);
std::string xmp = encode_xmp(spec, true);
CHECK(xmp.find("xmp:Rating=\"4\"") != std::string::npos);
```
3. Optional integration test: JPEG writer emits an XMP APP1 segment when
rating is present
- Create a small image
- Add `IPTC:Rating` or `xmp:Rating`
- Write JPEG
- Inspect the output bytes for the standard XMP APP1 namespace header:
`http://ns.adobe.com/xap/1.0/\0`
This verifies the full `encode_xmp()` -> JPEG writer path, not just the
XML generator.
**Notes**
This patch is intentionally narrow:
- it does not change the XMP mapping table itself
- it only fixes how mapped attributes are discovered during XMP
generation
Signed-off-by: Qi-fly <72260719+Qi-fly@users.noreply.github.com>1 parent 09a0352 commit ac3a9e7
1 file changed
+10
-4
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
228 | 228 | | |
229 | 229 | | |
230 | 230 | | |
231 | | - | |
232 | | - | |
233 | | - | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
234 | 239 | | |
235 | 240 | | |
236 | 241 | | |
| |||
614 | 619 | | |
615 | 620 | | |
616 | 621 | | |
617 | | - | |
| 622 | + | |
| 623 | + | |
618 | 624 | | |
619 | 625 | | |
620 | 626 | | |
| |||
0 commit comments