-
Notifications
You must be signed in to change notification settings - Fork 649
feat: oiiotool --mergemeta pt 2: fold into --pastemeta #4674
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or 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
After merging 4672, we noticed some things that could be improved, so this is a further revision (and partial retraction). There was already an oiiotool flag, `--metamerge`, which in retrospect make it seem unnecessarily confusing to add --mergemeta. So this PR removes this extra new command, instead simply piggy-backing on the existing --pastemeta by adding to it optional modifiers: - `:merge=` controls the merge strategy, 0 (default) means discard the existing metadata entirely and use only the pasted, matching the original --pastemeta behavior; 1 means merge nondestructively (no existing metadata will be altered or removed); 2 means destructive merge, where pasted metadata will overwrite existing metadata having the same name, but conflict-free existing metadata will remain. - `:pattern=` regex selects only some metadata. Additionally, we noticed an interesting corner case that surprised users when testing the new metadata merging feature from the last PR: What happens if the metadata source image has fewer subimages than the pixel source images? The behavior generally for oiiotool is that when binary operations (those taking two images as input) are on images with differing numbers of subimages, the output gets the number of subimges as the FIRST input (assuming -a instructed us to consider subimages at all). But that's not what we want here! If the metadata source has one subimage and the pixel source has many, clearly the intuitive result is for that one metadata set to be pasted into all of the subimages from the pixel-supplying image. So we fix that so that for --pastemeta, it's the second input image (not the first) that takes the lead in determining the number of subimages we will process. This in turn took a bit of extra infrastructure fixing in the OiiotoolOp class that controls this logic. Signed-off-by: Larry Gritz <[email protected]>
Collaborator
Author
|
This is working with images with different subimage counts and improve the metadata handling functionality in oiiotool. Looks good to me! Thanks! |
Collaborator
Author
|
The CI failure appears to be a false positive or some kind of abi checker confusion. It happens sometimes for things that aren't really ABI breaks, I think it gets confused by certain templates. |
1a34ef7
into
AcademySoftwareFoundation:main
27 of 28 checks passed
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Mar 22, 2025
…reFoundation#4674, AcademySoftwareFoundation#4676) After merging AcademySoftwareFoundation#4672, we noticed some things that could be improved, so this is a further revision (and partial retraction). There was already an oiiotool flag, `--metamerge`, which in retrospect make it seem unnecessarily confusing to add --mergemeta. So this PR removes this extra new command, instead simply piggy-backing on the existing --pastemeta by adding to it optional modifiers: - `:merge=` controls the merge strategy, 0 (default) means discard the existing metadata entirely and use only the pasted, matching the original --pastemeta behavior; 1 means merge nondestructively (no existing metadata will be altered or removed); 2 means destructive merge, where pasted metadata will overwrite existing metadata having the same name, but conflict-free existing metadata will remain. - `:pattern=` regex selects only some metadata. Additionally, we noticed an interesting corner case that surprised users when testing the new metadata merging feature from the last PR: What happens if the metadata source image has fewer subimages than the pixel source images? The behavior generally for oiiotool is that when binary operations (those taking two images as input) are on images with differing numbers of subimages, the output gets the number of subimges as the FIRST input (assuming -a instructed us to consider subimages at all). But that's not what we want here! If the metadata source has one subimage and the pixel source has many, clearly the intuitive result is for that one metadata set to be pasted into all of the subimages from the pixel-supplying image. So we fix that so that for --pastemeta, it's the second input image (not the first) that takes the lead in determining the number of subimages we will process. This in turn took a bit of extra infrastructure fixing in the OiiotoolOp class that controls this logic. Signed-off-by: Larry Gritz <[email protected]>
scott-wilson
pushed a commit
to scott-wilson/OpenImageIO
that referenced
this pull request
May 17, 2025
…reFoundation#4674) After merging AcademySoftwareFoundation#4672, we noticed some things that could be improved, so this is a further revision (and partial retraction). There was already an oiiotool flag, `--metamerge`, which in retrospect make it seem unnecessarily confusing to add --mergemeta. So this PR removes this extra new command, instead simply piggy-backing on the existing --pastemeta by adding to it optional modifiers: - `:merge=` controls the merge strategy, 0 (default) means discard the existing metadata entirely and use only the pasted, matching the original --pastemeta behavior; 1 means merge nondestructively (no existing metadata will be altered or removed); 2 means destructive merge, where pasted metadata will overwrite existing metadata having the same name, but conflict-free existing metadata will remain. - `:pattern=` regex selects only some metadata. Additionally, we noticed an interesting corner case that surprised users when testing the new metadata merging feature from the last PR: What happens if the metadata source image has fewer subimages than the pixel source images? The behavior generally for oiiotool is that when binary operations (those taking two images as input) are on images with differing numbers of subimages, the output gets the number of subimges as the FIRST input (assuming -a instructed us to consider subimages at all). But that's not what we want here! If the metadata source has one subimage and the pixel source has many, clearly the intuitive result is for that one metadata set to be pasted into all of the subimages from the pixel-supplying image. So we fix that so that for --pastemeta, it's the second input image (not the first) that takes the lead in determining the number of subimages we will process. This in turn took a bit of extra infrastructure fixing in the OiiotoolOp class that controls this logic. Signed-off-by: Larry Gritz <[email protected]> Signed-off-by: Scott Wilson <[email protected]>
scott-wilson
pushed a commit
to scott-wilson/OpenImageIO
that referenced
this pull request
May 18, 2025
…reFoundation#4674) After merging AcademySoftwareFoundation#4672, we noticed some things that could be improved, so this is a further revision (and partial retraction). There was already an oiiotool flag, `--metamerge`, which in retrospect make it seem unnecessarily confusing to add --mergemeta. So this PR removes this extra new command, instead simply piggy-backing on the existing --pastemeta by adding to it optional modifiers: - `:merge=` controls the merge strategy, 0 (default) means discard the existing metadata entirely and use only the pasted, matching the original --pastemeta behavior; 1 means merge nondestructively (no existing metadata will be altered or removed); 2 means destructive merge, where pasted metadata will overwrite existing metadata having the same name, but conflict-free existing metadata will remain. - `:pattern=` regex selects only some metadata. Additionally, we noticed an interesting corner case that surprised users when testing the new metadata merging feature from the last PR: What happens if the metadata source image has fewer subimages than the pixel source images? The behavior generally for oiiotool is that when binary operations (those taking two images as input) are on images with differing numbers of subimages, the output gets the number of subimges as the FIRST input (assuming -a instructed us to consider subimages at all). But that's not what we want here! If the metadata source has one subimage and the pixel source has many, clearly the intuitive result is for that one metadata set to be pasted into all of the subimages from the pixel-supplying image. So we fix that so that for --pastemeta, it's the second input image (not the first) that takes the lead in determining the number of subimages we will process. This in turn took a bit of extra infrastructure fixing in the OiiotoolOp class that controls this logic. Signed-off-by: Larry Gritz <[email protected]> Signed-off-by: Scott Wilson <[email protected]>
zachlewis
pushed a commit
to zachlewis/OpenImageIO
that referenced
this pull request
Sep 1, 2025
…reFoundation#4674) After merging AcademySoftwareFoundation#4672, we noticed some things that could be improved, so this is a further revision (and partial retraction). There was already an oiiotool flag, `--metamerge`, which in retrospect make it seem unnecessarily confusing to add --mergemeta. So this PR removes this extra new command, instead simply piggy-backing on the existing --pastemeta by adding to it optional modifiers: - `:merge=` controls the merge strategy, 0 (default) means discard the existing metadata entirely and use only the pasted, matching the original --pastemeta behavior; 1 means merge nondestructively (no existing metadata will be altered or removed); 2 means destructive merge, where pasted metadata will overwrite existing metadata having the same name, but conflict-free existing metadata will remain. - `:pattern=` regex selects only some metadata. Additionally, we noticed an interesting corner case that surprised users when testing the new metadata merging feature from the last PR: What happens if the metadata source image has fewer subimages than the pixel source images? The behavior generally for oiiotool is that when binary operations (those taking two images as input) are on images with differing numbers of subimages, the output gets the number of subimges as the FIRST input (assuming -a instructed us to consider subimages at all). But that's not what we want here! If the metadata source has one subimage and the pixel source has many, clearly the intuitive result is for that one metadata set to be pasted into all of the subimages from the pixel-supplying image. So we fix that so that for --pastemeta, it's the second input image (not the first) that takes the lead in determining the number of subimages we will process. This in turn took a bit of extra infrastructure fixing in the OiiotoolOp class that controls this logic. 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.
After merging #4672, we noticed some things that could be improved, so this is a further revision (and partial retraction).
There was already an oiiotool flag,
--metamerge, which in retrospect make it seem unnecessarily confusing to add --mergemeta. So this PR removes this extra new command, instead simply piggy-backing on the existing --pastemeta by adding to it optional modifiers::merge=controls the merge strategy, 0 (default) means discard the existing metadata entirely and use only the pasted, matching the original --pastemeta behavior; 1 means merge nondestructively (no existing metadata will be altered or removed); 2 means destructive merge, where pasted metadata will overwrite existing metadata having the same name, but conflict-free existing metadata will remain.:pattern=regex selects only some metadata.Additionally, we noticed an interesting corner case that surprised users when testing the new metadata merging feature from the last PR: What happens if the metadata source image has fewer subimages than the pixel source images? The behavior generally for oiiotool is that when binary operations (those taking two images as input) are on images with differing numbers of subimages, the output gets the number of subimges as the FIRST input (assuming -a instructed us to consider subimages at all). But that's not what we want here! If the metadata source has one subimage and the pixel source has many, clearly the intuitive result is for that one metadata set to be pasted into all of the subimages from the pixel-supplying image.
So we fix that so that for --pastemeta, it's the second input image (not the first) that takes the lead in determining the number of subimages we will process. This in turn took a bit of extra infrastructure fixing in the OiiotoolOp class that controls this logic.