fix(exr): allow an empty "name" metadata to be read #4528
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When the exr "name" metadata (used for the names of subimages/parts) existed but was the empty string, we were treating it like it didn't exist (and upon write substituted our own "subimageXY" name, as we are wont to do for unnamed parts). But now we've seen multi-part exrs where the name is "" (exists, but empty string) and we want a read-write cycle to preserve that. Even though it's not a very sensible or helpful part name, it's technically legal, so we should honor it. Signed-off-by: Larry Gritz <[email protected]>
|
Does anybody object to this? In short, the change allows multi-part exr files to have a part whose name is the empty string "", which is technically allowed by OpenEXR itself. Our old behavior was to treat an empty string part name as if there was no name at all (in which case we substituted something else). |
|
N.B. I always say "does anybody object" but what I really mean is "somebody PLEASE review and approve (or critique) this, because it's bad open source practice to merge PRs with no approval by somebody who isn't the author of the patch and I don't want to do that except in cases of real emergencies." |
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Nov 9, 2024
…oundation#4528) When the exr "name" metadata (used for the names of subimages/parts) existed but was the empty string, we were treating it like it didn't exist (and upon write substituted our own "subimageXY" name, as we are wont to do for unnamed parts). But now we've seen multi-part exrs where the name is "" (exists, but empty string) and we want a read-write cycle to preserve that. Even though it's not a very sensible or helpful part name, it's technically legal, so we should honor it. Signed-off-by: Larry Gritz <[email protected]>
|
For what it's worth, in the OpenEXR subset documented in SMPTE's ACES Container (ST 2065-4:2023 ACES Image Container File Layout) zero-length attributes are everywhere -- unless we missed a spot -- disallowed. Point of information only. As for the change itself, LGTM |
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Nov 9, 2024
…oundation#4528) When the exr "name" metadata (used for the names of subimages/parts) existed but was the empty string, we were treating it like it didn't exist (and upon write substituted our own "subimageXY" name, as we are wont to do for unnamed parts). But now we've seen multi-part exrs where the name is "" (exists, but empty string) and we want a read-write cycle to preserve that. Even though it's not a very sensible or helpful part name, it's technically legal, so we should honor it. Signed-off-by: Larry Gritz <[email protected]>
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Nov 12, 2024
Continuation of PR AcademySoftwareFoundation#4528. I forgot to change the spot in the OpenEXR "core" API as well.
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Nov 12, 2024
Continuation of PR AcademySoftwareFoundation#4528. I forgot to change the spot in the OpenEXR "core" API as well.
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Nov 12, 2024
Continuation of PR AcademySoftwareFoundation#4528. I forgot to change the spot in the OpenEXR "core" API as well. Signed-off-by: Larry Gritz <[email protected]>
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Nov 12, 2024
Continuation of PR AcademySoftwareFoundation#4528. I forgot to change the spot in the OpenEXR "core" API as well. Signed-off-by: Larry Gritz <[email protected]>
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Nov 12, 2024
…oundation#4536) Continuation of PR AcademySoftwareFoundation#4528. I forgot to change the spot in the OpenEXR "core" API as well. Signed-off-by: Larry Gritz <[email protected]>
lgritz
added a commit
that referenced
this pull request
Nov 15, 2024
Continuation of PR #4528. I forgot to change the spot in the OpenEXR "core" API as well. Signed-off-by: Larry Gritz <[email protected]>
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Nov 15, 2024
…oundation#4536) Continuation of PR AcademySoftwareFoundation#4528. I forgot to change the spot in the OpenEXR "core" API as well. Signed-off-by: Larry Gritz <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the exr "name" metadata (used for the names of subimages/parts) existed but was the empty string, we were treating it like it didn't exist (and upon write substituted our own "subimageXY" name, as we are wont to do for unnamed parts). But now we've seen multi-part exrs where the name is "" (exists, but empty string) and we want a read-write cycle to preserve that. Even though it's not a very sensible or helpful part name, it's technically legal, so we should honor it.