feat(exr): Follow CIF for color interop ID in multi-part files by brechtvl · Pull Request #5422 · AcademySoftwareFoundation/OpenImageIO

brechtvl · 2026-08-25T12:56:19Z

Description

On read, a part without a colorInteropID now gets it from the first part, if there is any. There is no validation, EXR files that do not follow the CIF recommendation are still accepted.

On write with openexr:ColorInteropIDPolicy set to "strict" in the first subimage, give an error when the CIF recommendation is not followed. That is, when the colorInteropID in later parts is different than the first part, except if is missing or set to "data".

For example writing parts like these gives an error with the strict policy:

"lin_ap0_scene" "lin_ap1_scene"
"data" "lin_ap0_scene"
missing "lin_ap0_scene"

I considered automatically reordering headers to accept cases where a "data" part happens to come first. However attributes in the first header have a special meaning, and reordering attributes other than colorInteropID to later headers would be problematic.

Ref AcademySoftwareFoundation/openexr#2560

Tests

New test added for this.

Checklist:

I have read the guidelines on contributions and code review procedures.
I have read the Policy on AI Coding Assistants
and if I used AI coding assistants, I have an Assisted-by: TOOL / MODEL
line in the pull request description above.
I have updated the documentation if my PR adds features or changes
behavior.
I am sure that this PR's changes are tested in the testsuite.
I have run and passed the testsuite in CI before submitting the
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.)
My code follows the prevailing code style of this project and I
fixed any problems reported by the clang-format CI test.
If I added or modified a public C++ API call, I have also amended the
corresponding Python bindings. If altering ImageBufAlgo functions, I also
exposed the new functionality as oiiotool options.

On read, a part without a colorInteropID now gets it from the first part, if there is any. There is no validation, EXR files that do not follow the CIF recommendation are still accepted. On write, give an error when the colorInteropID in later parts is different than the first part, except if is missing or set to "data". Upcoming versions of OpenEXR will throw an exception in this case, this applies the same logic for existing versions of OpenEXR already. Note this also means writing parts in order like this is now an error: * "lin_ap0_scene", "lin_ap1_scene" * "data", "lin_ap0_scene" * missing, "lin_ap0_scene" It also means "data" <missing> becomes "data" "data" on read. I considered automatically reordering headers to accept cases where a "data" part happens to come first. However attributes in the first header have a special meaning, and reordering attributes other than colorInteropID to later headers would be problematic. Ref AcademySoftwareFoundation/openexr#2560 Signed-off-by: Brecht Van Lommel <brecht@blender.org>

brechtvl · 2026-08-25T12:58:56Z

CC @doug-walker @zachlewis

brechtvl · 2026-08-25T12:59:32Z

This is more strict than I would have chosen myself, as I explained in the OpenEXR PR. But even if the rules were relaxed a bit, it still remains a breaking change that should probably be done sooner than later in 3.2.

Signed-off-by: Brecht Van Lommel <brecht@blender.org>

doug-walker

This looks good Brecht, it seems to follow the CIF Recommendation.

You have raised a valid point that this will potentially refuse to write some files that would have been written before. Personally, I don't think that's undesirable since the parts would not be well-organized, but I'd be interested to hear from OIIO users if this is problematic for them.

brechtvl · 2026-08-26T10:57:15Z

Converting to a draft for now since discussion in the OpenEXR PR is still ongoing, so best not to land this before a decision is made there.

Signed-off-by: Brecht Van Lommel <brecht@blender.org>

brechtvl · 2026-09-05T21:17:37Z

I now made the validation on write optional with openexr:ColorInteropIDPolicy, like the existing openexr:ACESContainerPolicy.

I considered adding a global imageoutput:strict instead (to go along with the existing imageput:strict), since that might be more convenient if more of these things get added. Both could also coexist by making imageoutput:strict affect the default value of such policy attributes.

I don't personally have a use case for this validation, so don't really have a strong opinion if and how it should be done.

lgritz · 2026-09-19T00:24:03Z

Sorry, it's clearly my fault for getting distracted with releases and Dev Days and letting this grow stale for 2 weeks, but I'm afraid that this has some merge conflicts. (The most serious being that the .rst files for docs have all been changed to .md.)

I can try to fix them on my end if you would prefer.

brechtvl · 2026-09-19T01:35:55Z

No problem at all, merged now.

lgritz · 2026-09-19T05:16:40Z

The wheel failures are not due to this PR. I will investigate those separately.

lgritz · 2026-09-20T03:09:55Z

+   * - ``openexr:ColorInteropIDPolicy``
+     - string
+     - One of `none` (default) or `strict`, taken from the first subimage.


Do you think that the right place to set this is in the spec of every output file when it's opened? Is it likely that people will want to use different policies for different files within an app invocation?

Or would it be more convenient for most people to set once per execution for an app-wide policy, like we do for "openexr:core"?

I was following openexr:ACESContainerPolicy which is per file, but personally I have no need for that or this new policy to be that granular.

The main use cases I can think of involve oiiotool, and a new global imageoutput:strict attribute to match imageinput:strict seems most convenient to me. Since then it's only a single option to remember for all types of validation and file formats.

For a DCC, global attributes can be problematic because there's multi-threading and plug-ins and so you have to be very careful with what you enable to avoid breaking other code. But at least in Blender I have no intent of using this and would try to make EXRs valid by construction rather than trying to validate.

I'd be happy to make it either a global openexr:ColorInteropIDPolicy or imageoutput:strict attribute (or both) instead.

I don't have a real preference between those two. I was mostly concerned with whether it would be easier to use as a global versus per-file.

lgritz

I raised several first-principles concerns inline.

If this is all spelled out by OpenEXR or CIF policies that I'm not up to speed on, then apologies. I think I was so busy in the last month with bug fixes and preparing releases, that on both the OpenEXR side as well as on this PR, I just trusted others to figure out what to do here. And I do trust you -- I'm voicing these things in case any choices were unintentional (and I consider it very possible that I just misunderstand what's happening). Leave or change, up to you. Just read my comments and decide whether this should stand as-is and be merged or if changes are needed.
@brechtvl @doug-walker @zachlewis I'll sign off on whatever you think is right.

lgritz · 2026-09-20T05:28:00Z

+    } else {
+        // Follow the color interop forum recommendation for OpenEXR files,
+        // inheriting the colorInteropID from the first part.
+        string_view interop_id = spec.get_string_attribute("colorInteropID");
+        if (!interop_id.empty()) {
+            spec.set_colorspace(interop_id);
+        } else if (!in->m_file_color_interop_id.empty()) {
+            spec.set_colorspace(in->m_file_color_interop_id);
+        }


Maybe I don't understand what's going on here. The comment says "inherit from the first part", but I think what the code is doing is "use the colorInteropID of this part, but if it doesn't exist, then use the one from the first part."

Also, none of this comes into play if acesImageContainerFlag is 1. Should that also inherit (conditionally or unconditionally?) from the first part? What's supposed to happen for a file that inconsistently says it's an aces container but also has a colorInteropID that is something other than lin_ap0_scene?

Maybe inherit is a poor choice of words, I will more explicitly write it out in the comment the way you did.

An ACES image container only has a single part according to the specification. OpenEXR has special handling for attributes like colorInteropID, chromaticities and displayWindow in multiple parts, but nothing for acesImageContainerFlag. So I didn't do anything here either, but I don't really have an opinion on what is best here.

The CIF OpenEXR recommendation does say that the ACES container flag has precedence over the color interop ID, so that's what I implemented.

As Brecht wrote, the CIF Recommendation does give the ACES container flag precedence, and likewise the inheriting from the first part is following the recommendation.

If the file says it's an ACES container but has a different colorInteropID, that is something that would be flagged by the OpenEXR checkColorMetadata function. I'm not sure if OIIO has some kind of validation function on images, but something like that could be added there as well. On the OpenEXR side, we decided not to have errors like this block reading the file, but if exrinput has something analogous to the strict mode in exroutput, that might be a useful option.

Oh yes, I do understand that the ACES container flag has precedence. I was referring to whether that, too, should also have the "first part has precedence over all others" property that the interop id has.

Oh right, true ACES containers are only one part anyway. Never mind.

lgritz · 2026-09-20T05:31:12Z

+
+    if (policy != "strict") {
+        errorfmt("Unknown openexr:ColorInteropIDPolicy \"{}\"", policy);
+        return false;
+    }


Merits of treating the policy request that is anything other than case-sensitive "none" and "strict" as a hard error that prevents reading the file? Or should an unknown string just silently be the default "none" behavior?

Personally I find it helpful for it to detect when I've made a typo. But I see openexr:ACESContainerPolicy doesn't check this, so I'm fine removing it too for consistency.

A case could be plausibly made for being quite strict on all hint names and values, but since the status quo is usually to be as forgiving as possible and only fail if there is no reasonable interpretation of how to proceed, I think that making this one more strict than almost everything else just will tend to trip people up.

lgritz · 2026-09-20T05:35:13Z

+    if (!validate_color_interop_ids())
+        return false;
+


I think this means that the default "none" policy will just have no complaints if the parts (invalidly?) give and propagate different color spaces.

Is there merit to having the purpose of the policy to dictate whether we issue an error versus silently fix the problem? That is, should "none" still result in all parts inheriting the color space of the first part?

I don't think there is a good way to silently fix the problem. If you have invalid parts like this:

"lin_ap0_scene" "lin_ap1_scene"

"data" "lin_ap0_scene"

Then making all parts have lin_ap0_scene or data respectively seems more damaging than helpful to me.

For the chromaticities attribute, OpenEXR does have a mode where the user may request that the chromaticities of later parts are changed to match the first part, and my first commits when adding interop ID support to OpenEXR did likewise. But I removed this at the suggestion of Peter. As Brecht wrote, silently overwriting a part's ID is problematic and seem inappropriate for the "none" option. Though perhaps it would be useful in a "relaxed" or other option as a convenience for people to avoid needing to manually iterate through the parts to fix errors in cases where they know some of the parts are wrong for some reason (I think that was the motivation for OpenEXR's chromaticities mode).

Ok, this is fine, then. The differences only matter if you're trying to write invalid files, so I'm not inclined to care much, and we can adjust later if this is causing problems.

If I'm being honest, I would have said all along that it was perfectly fine with me for different parts to have different color spaces. OIIO's conceptual view of multi-image file is that the subimages they can be arbitrary images that just happen to be in one file container, and it's a peculiarity of OpenEXR that it's more restrictive than that. So the details of exactly how OpenEXR wants to be restrictive is not something I am going to worry much about, we just go along with what the file format says.

lgritz · 2026-09-20T21:12:17Z

Alright, I've made all the comments I'm inclinded to. Brecht, you can decide if you want to change anything, and just let me know when to merge. The code looks fine, and I'll look at any changes (if there are any) before merge.

Improve test naming and failureok usage

391bfaf

Signed-off-by: Brecht Van Lommel <brecht@blender.org>

doug-walker reviewed Aug 25, 2026

View reviewed changes

Comment thread src/openexr.imageio/exroutput.cpp Outdated

brechtvl marked this pull request as draft August 26, 2026 10:54

Make write validation optional with openexr:ColorInteropIDPolicy

6f87cc0

Signed-off-by: Brecht Van Lommel <brecht@blender.org>

brechtvl marked this pull request as ready for review September 5, 2026 21:17

brechtvl changed the title ~~feat(openexr): Follow CIF for color interop ID in multi-part files~~ feat(exr): Follow CIF for color interop ID in multi-part files Sep 5, 2026

Merge branch 'main' into exr-multipart-colorspace

7de64ab

lgritz reviewed Sep 20, 2026

View reviewed changes

Conversation

brechtvl commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Tests

Checklist:

Uh oh!

brechtvl commented Aug 25, 2026

Uh oh!

brechtvl commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

doug-walker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brechtvl commented Aug 26, 2026

Uh oh!

brechtvl commented Sep 5, 2026

Uh oh!

lgritz commented Sep 19, 2026

Uh oh!

brechtvl commented Sep 19, 2026

Uh oh!

lgritz commented Sep 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brechtvl Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgritz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brechtvl Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgritz commented Sep 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

brechtvl commented Aug 25, 2026 •

edited

Loading

brechtvl commented Aug 25, 2026 •

edited

Loading

brechtvl Sep 20, 2026 •

edited

Loading

brechtvl Sep 20, 2026 •

edited

Loading