feat(jpeg): Support encoding/decoding arbitrary metadata as comments #4430
Conversation
This is needed to port Blender's current JPEG IO code to using OIIO, but is also a useful feature to have in general. For reading, the code tries to parse comments as colon-separated key-value pairs and sets metadata accordingly. For writing, this needs to be explicitly enabled by setting jpeg:com_attributes to 1 in order to avoid accidentally bloating files for existing applications. Signed-off-by: Lukas Stockner <[email protected]>
lukasstockner
force-pushed
the
jpeg-metadata
branch
from
3521e6c
to
ba93c68
Compare
|
The Mac failures are unrelated and fixed by a different PR that has already been merged. |
| std::string((const char*)m->data, | ||
| m->data_length)); | ||
| m_spec.attribute("ImageDescription", data); | ||
| // Additional string metadata can be stored in JPEG files as |
There was a problem hiding this comment.
Maybe it's worth explicitly starting by spelling out:
// The first COM block encountered will be interpreted as the image description.
// Subsequent COM blocks, if in the form "key:value", ... blah blah
By the way, is this exactly what we want? What if the first COM looks like "key:value", should that always be slotted into ImageDescription?
There was a problem hiding this comment.
I would prefer only setting ImageDescription if the parsing fails, but I figured that's a potentially breaking change so I kept it safe for now.
There was a problem hiding this comment.
I bet that most traditional JPEG COM blocks that are true "image comments" are unlikely to have the specific form of "[ident:]string1:string2" where the optional ident (namespace prefix) follows C identifier rules and string1 won't start or end with whitespace. If we interpret only that pattern as metadata and the first COM that doesn't follow the pattern is "ImageDescription".
I'm willing to risk that an occasional "comment" with a quirky format might be incorrectly interpreted as metadata. Especially if there is some kind of OIIO global option that lets you revert to the old behavior (first COM is always ImageDescription), so somebody can get out of a bind if they have a pile of images with the ambiguous formatting of their COM blocks.
There was a problem hiding this comment.
What's the verdict here, @lukasstockner? Do you want to make any more changes, or do you want to keep the logic as-is and we can always revise later if it causes trouble?
There was a problem hiding this comment.
If it's fine with you, I'll go ahead and add some more logic to only set the ImageDescription if a global option is set and/or the matched key fails a heuristic.
|
@lukasstockner Is this ready to merge, or is there more you wanted to do at this stage? |
I still want to implement the ImageDescription logic discussed above. I didn't find time yet, but hope to do so tomorrow. |
|
@lukasstockner Just a reminder, you were going to make a minor adjustment here, and then I think we are ready to merge. You may also need to rebase on the current main and resolve some very minor conflicts that have occurred since you started this PR. |
|
Ping @lukasstockner |
lukasstockner
force-pushed
the
jpeg-metadata
branch
from
0b4142e
to
182bf07
Compare
Signed-off-by: Lukas Stockner <[email protected]>
…arter Signed-off-by: Lukas Stockner <[email protected]>
lukasstockner
force-pushed
the
jpeg-metadata
branch
from
182bf07
to
06ad8d5
Compare
|
So sorry for the delay here, I've finally implemented the improvement we discussed. |
Signed-off-by: Lukas Stockner <[email protected]>
…cademySoftwareFoundation#4430) This is needed to port Blender's current JPEG IO code to using OIIO, but is also a useful feature to have in general. For reading, the code tries to parse comments as colon-separated key-value pairs and sets metadata accordingly. For writing, this needs to be explicitly enabled by setting jpeg:com_attributes to 1 in order to avoid accidentally bloating files for existing applications. Tests: I've added a small (~10KB) JPEG file containing Blender metadata and a basic test that parses it, checks that the metadata was read correctly, writes it twice (once with and once without `jpeg:com_attributes`), and then checks that those files are also parsed as expected. In case you're wondering why the info for "no-attribs.jpg" still contains one Blender attribute - that's because the first COM field is still put into `ImageDescription` just like before, so even without `jpeg:com_attributes` it ends up being written to the output file and recognized during parsing. --------- Signed-off-by: Lukas Stockner <[email protected]>
…cademySoftwareFoundation#4430) This is needed to port Blender's current JPEG IO code to using OIIO, but is also a useful feature to have in general. For reading, the code tries to parse comments as colon-separated key-value pairs and sets metadata accordingly. For writing, this needs to be explicitly enabled by setting jpeg:com_attributes to 1 in order to avoid accidentally bloating files for existing applications. Tests: I've added a small (~10KB) JPEG file containing Blender metadata and a basic test that parses it, checks that the metadata was read correctly, writes it twice (once with and once without `jpeg:com_attributes`), and then checks that those files are also parsed as expected. In case you're wondering why the info for "no-attribs.jpg" still contains one Blender attribute - that's because the first COM field is still put into `ImageDescription` just like before, so even without `jpeg:com_attributes` it ends up being written to the output file and recognized during parsing. --------- Signed-off-by: Lukas Stockner <[email protected]>
…cademySoftwareFoundation#4430) This is needed to port Blender's current JPEG IO code to using OIIO, but is also a useful feature to have in general. For reading, the code tries to parse comments as colon-separated key-value pairs and sets metadata accordingly. For writing, this needs to be explicitly enabled by setting jpeg:com_attributes to 1 in order to avoid accidentally bloating files for existing applications. Tests: I've added a small (~10KB) JPEG file containing Blender metadata and a basic test that parses it, checks that the metadata was read correctly, writes it twice (once with and once without `jpeg:com_attributes`), and then checks that those files are also parsed as expected. In case you're wondering why the info for "no-attribs.jpg" still contains one Blender attribute - that's because the first COM field is still put into `ImageDescription` just like before, so even without `jpeg:com_attributes` it ends up being written to the output file and recognized during parsing. --------- Signed-off-by: Lukas Stockner <[email protected]>
…cademySoftwareFoundation#4430) This is needed to port Blender's current JPEG IO code to using OIIO, but is also a useful feature to have in general. For reading, the code tries to parse comments as colon-separated key-value pairs and sets metadata accordingly. For writing, this needs to be explicitly enabled by setting jpeg:com_attributes to 1 in order to avoid accidentally bloating files for existing applications. Tests: I've added a small (~10KB) JPEG file containing Blender metadata and a basic test that parses it, checks that the metadata was read correctly, writes it twice (once with and once without `jpeg:com_attributes`), and then checks that those files are also parsed as expected. In case you're wondering why the info for "no-attribs.jpg" still contains one Blender attribute - that's because the first COM field is still put into `ImageDescription` just like before, so even without `jpeg:com_attributes` it ends up being written to the output file and recognized during parsing. --------- Signed-off-by: Lukas Stockner <[email protected]>
This is needed to port Blender's current JPEG IO code to using OIIO, but is also a useful feature to have in general.
For reading, the code tries to parse comments as colon-separated key-value pairs and sets metadata accordingly. For writing, this needs to be explicitly enabled by setting jpeg:com_attributes to 1 in order to avoid accidentally bloating files for existing applications.
Tests
I've added a small (~10KB) JPEG file containing Blender metadata and a basic test that parses it, checks that the metadata was read correctly, writes it twice (once with and once without
jpeg:com_attributes), and then checks that those files are also parsed as expected.In case you're wondering why the info for "no-attribs.jpg" still contains one Blender attribute - that's because the first COM field is still put into
ImageDescriptionjust like before, so even withoutjpeg:com_attributesit ends up being written to the output file and recognized during parsing.Checklist:
(adding new test cases if necessary).
corresponding Python bindings (and if altering ImageBufAlgo functions, also
exposed the new functionality as oiiotool options).
already run clang-format before submitting, I definitely will look at the CI
test that runs clang-format and fix anything that it highlights as being
nonconforming.