Conversation
Add an optional C++20 bridge for OpenMeta behind a C-compatible boundary and expose decoded values to OIIO through a C++17 adapter. Use OpenMeta for WebP EXIF and XMP decoding when USE_OPENMETA is enabled. Prefer typed native metadata for flat OIIO names, then emit non-conflicting XMP names and preserve collisions or structured XMP with canonical names. Support shared and static builds, export the static dependency closure, and import Highway before OpenMeta where required by package targets. Add bridge unit tests and WebP coverage for TIFF and Exif-prefixed payloads, EXIF/XMP collisions, and structured XMP properties. Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
Signed-off-by: Vlad <shaamaan@gmail.com>
Signed-off-by: Vlad <shaamaan@gmail.com>
lgritz
left a comment
There was a problem hiding this comment.
I have a few scattered nits in the code as I started to read it, but then in the main thread I'll talk about some more general design issues or big questions I have as I'm trying to understand this.
| # OpenMeta dependencies may otherwise create a partial Highway import. | ||
| find_dependency(hwy CONFIG) | ||
| endif () | ||
| if (@OIIO_OPENMETA_BRIDGE_ENABLED@ AND @EMBEDPLUGINS@) |
There was a problem hiding this comment.
OpenMeta only works if EMBEDPLUGINS is used?
| checked_find_package (hwy) | ||
| endif () | ||
|
|
||
| option (USE_OPENMETA "Enable experimental OpenMeta metadata decoding" OFF) |
There was a problem hiding this comment.
Let's use our set_option, to allow env variables to override it.
| if (OpenMeta_FOUND AND NOT TARGET OpenMeta::openmeta) | ||
| message (WARNING |
There was a problem hiding this comment.
We don't do this kind of gated check with any of the other dependencies. What is the circumstance in which OpenMeta_FOUND would be set, but that the target doesn't exist?
| command += oiiotool( | ||
| 'openmeta-metadata.webp ' | ||
| '--echo "TIFF Make={TOP.Make}" ' |
There was a problem hiding this comment.
Wouldn't this be easier to just be
oiiotool('-info -v openmeta-metadata.webp')
and then just compare the output to what we've saved as the reference, rather than individually --echo every piece of metadata?
And similar for the other oiiotool command below?
|
Big picture questions to help set the stage for understanding this work and how it fits into OpenImageIO. I'm just going to ramble a bit, this isn't necessarily in any order. OpenMeta requires C++20? So though we don't need to expose any of that in OIIO's public APIs, at least some part of the code that talks directly to OpenMeta needs to be build with C++20, then, right? Now, that may be as little as one module, and it can present a C++17 wrapped small API to the rest of OIIO that just exposes the entry points to what we need, of course. I didn't see anything that seemed conditional on C++20 availability. It seems to me that there should be some kind of a check somewhere on our side, where the OpenMeta support is only enabled if we're building with a compiler that can compile that module in C++20 mode so that it can use OpenMeta. Much like you did for Highway, I think there should be both a build-time option (search for OpenMeta and use it if found) and also a runtime switch (enable/disable it on a run by run basis). At the very least, this will make it a lot easier to take a single (OM-enabled) build and test behavior with and without it enabled, which I think we'll want to do in a lot of cases as we explore using it. I'm not sure why you organized it into a separate library "OpenMeta Bridge" instead of just a regular cpp file within libOpenImageIO that is the one module (compilation unit, I mean) that needs to directly interface with OM and therefore to be build with C++20. It looks like the main interfaces you've set up are based on passing an IOProxy, and therefore directly reading on the OM side? I'm no expert on OM, but does it contain entry points that can read the metadata from memory? Because that's usually what we already have on hand -- the buffer containing raw Exif, or XMP, or a MakerNote, or whatever. I would have expected an interface (not necessarily in OpenMeta, but I mean, on our side for how we wrap OpenMeta) that looks a little more like Put more simply, why bool ok = decode_exif(exif_span, m_spec);turning into bool ok;
#ifdef OIIO_USE_OPENMETA
if (OIIO::pvt::openmetadata_enabled())
ok = openmetadata_decode_exif(exif_span, m_spec);
else
#endif
ok = decode_exif(exif_span, m_spec);and nothing else in webpinput.cpp needs to change? Or, for that matter, maybe NOTHING in webpinput.cpp needs to change at all, and the change is entirely in bool
decode_exif(cspan<uint8_t> exif, ImageSpec& spec)
{
#ifdef OIIO_USE_OPENMETA
if (OIIO::pvt::openmetadata_enabled())
return openmetadata_decode_exif(exif_span, m_spec);
#endif
// ... rest of the existing decode_exif as it is now ...
}and then it would work with all exif parsing for all formats? Here's really the TL;DR summary. To drastically summarize: I really like the idea of outsourcing much of the metadata parsing to a separate project that specializes in that particular task (assuming it works well, etc.). Really all my comments boil down to: How thin can we make the interface between OM and OIIO? How can we minimize the amount of OIIO-side code that needs to be changed in any way to get the maximum use of OM? (I.e. a side call within decode_exif is better than changing every reader separately that calls decode_exif.) And how can we make it really simple to enable/disable at runtime, until we are confident that this is the way we want to do all metadata parsing? I know that I'm sweeping some raw image details under the rug for now. But it seems like the exif / xmp / iptc is low-hanging fruit to get things started in a really simple way. |
|
@lgritz, thank you for the feedback! OpenMeta supports all common metadata types. I think it already outperforms all built-in metadata parsers in most image codecs (including LibRAW). XMP in WebP was just a good example of what OpenMeta can add, because WebP plugins do not decode this block :) I will refactor PR! |
Replace the WebP-specific whole-container integration with bounded-memory OpenMeta adapters called from the shared decode_exif and decode_xmp entry points. Add an enable_openmeta runtime attribute and OPENIMAGEIO_ENABLE_OPENMETA environment variable. Default to the native decoders. OpenMeta failures leave destination attributes unchanged without retrying the native parser. Compile the adapter as a private C++20 object target within libOpenImageIO. Remove the separate bridge libraries and add a C++20 compile/link probe. Preserve C++17 interfaces and propagate the OpenMeta dependency for static builds independently of EMBEDPLUGINS. Reuse OIIO metadata names and conversions, retain structured XMP and conflicting values under namespace-qualified names, and handle rational values and arrays without overwriting scalar attributes. Route WebP EXIF and XMP chunks through the shared decoders. Leave image decoding, ICC handling, and metadata writing with the existing codecs. Update documentation and regression tests. Validation: - OpenMeta unit and WebP tests passed on WSL and Windows. - WSL image I/O tests passed with both runtime backends. - A build without OpenMeta passed its unit test. - An installed Windows static C++17 consumer passed with workarounds for existing dependency-discovery gaps. Assisted-by: Codex / GPT-6 Astra ExtraHigh Signed-off-by: Vlad <shaamaan@gmail.com>
Description
Summary
This PR adds experimental, optional integration with OpenMeta and uses it to decode WebP EXIF and XMP metadata.
The integration is disabled by default and can be enabled with:
-DUSE_OPENMETA=ONOpenMeta 0.4.118 or newer is required.
Motivation
OpenImageIO currently implements metadata parsing separately in individual image plugins. OpenMeta provides a format-aware metadata decoder with a format-independent output model, including richer EXIF and structured XMP support.
WebP is used as the initial integration because its existing reader only decodes EXIF metadata and does not currently expose XMP properties.
Implementation
The integration introduces two layers:
This keeps OpenMeta's C++20 types and ABI requirements out of OpenImageIO's C++17 interfaces.
When OpenMeta is enabled, the WebP reader uses it as the sole EXIF/XMP metadata decoder. The legacy WebP EXIF path remains unchanged in builds without OpenMeta.
Metadata projection uses the following precedence:
For example, a typed EXIF aperture remains available as Exif:ApertureValue, while an XMP property with the same semantic meaning is preserved under its canonical namespace-qualified name.
Build and packaging
Tests
Added:
Scope
This PR only changes WebP metadata reading. WebP pixel decoding and metadata writing are unchanged.
The bridge is intended to allow additional image plugins to adopt OpenMeta incrementally after the WebP integration has established suitable behavior, compatibility, and performance.
Checklist:
and if I used AI coding assistants, I have an
Assisted-by: Codex Sol 5.6 extraHigh and Astra extraHighline in the pull request description above.
behavior.
PR, by pushing the changes to my fork and seeing that the automated CI
passed there. (Exceptions: If most tests pass and you can't figure out why
the remaining ones fail, it's ok to submit the PR and ask for help. Or if
any failures seem entirely unrelated to your change; sometimes things break
on the GitHub runners.)
fixed any problems reported by the clang-format CI test.
corresponding Python bindings. If altering ImageBufAlgo functions, I also
exposed the new functionality as oiiotool options.