Skip to content
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

feat(jpeg): Support encoding/decoding arbitrary metadata as comments #4430

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cmake/testing.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ macro (oiio_add_all_tests)
oiiotool-demosaic
diff
dither dup-channels
jpeg-corrupt
jpeg-corrupt jpeg-metadata
maketx oiiotool-maketx
misnamed-file
missingcolor
Expand Down
8 changes: 8 additions & 0 deletions src/doc/builtinplugins.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1037,6 +1037,10 @@ anywhere near the acceptance of the original JPEG/JFIF format.
reader/writer, and you should assume that nearly everything described
Appendix :ref:`chap-stdmetadata` is properly translated when using
JPEG files.
* - *other*
-
- Extra attributes will be read from comment blocks in the JPEG file,
and can optionally be written if ``jpeg:com_attributes`` is enabled.

**Configuration settings for JPEG input**

Expand Down Expand Up @@ -1084,6 +1088,10 @@ control aspects of the writing itself:
* - ``jpeg:progressive``
- int
- If nonzero, will write a progressive JPEG file.
* - ``jpeg:com_attributes``
- int
- If nonzero, extra attributes will be written into the file as comment
blocks.


**Custom I/O Overrides**
Expand Down
20 changes: 17 additions & 3 deletions src/jpeg.imageio/jpeginput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,10 +284,24 @@ JpgInput::open(const std::string& name, ImageSpec& newspec)
&& !strcmp((const char*)m->data, "Photoshop 3.0"))
jpeg_decode_iptc((unsigned char*)m->data);
else if (m->marker == JPEG_COM) {
std::string data((const char*)m->data, m->data_length);
if (!m_spec.find_attribute("ImageDescription", TypeDesc::STRING))
m_spec.attribute("ImageDescription",
std::string((const char*)m->data,
m->data_length));
m_spec.attribute("ImageDescription", data);
// Additional string metadata can be stored in JPEG files as
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me.

// comment markers in the form "key:value".
// Since the key might also commonly contain a colon, the
// heuristic to separate them here is that if there are multiple
// colons, the first one belongs to the key, the second one is the
// separator, and any further ones belong to the value.
auto separator = data.find(':');
if (separator != std::string::npos && separator > 0) {
auto second_separator = data.find(':', separator + 1);
if (second_separator != std::string::npos)
separator = second_separator;
std::string key = data.substr(0, separator);
if (!m_spec.find_attribute(key, TypeDesc::STRING))
m_spec.attribute(key, data.substr(separator + 1));
}
}
}

Expand Down
39 changes: 39 additions & 0 deletions src/jpeg.imageio/jpegoutput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <cassert>
#include <cstdio>
#include <set>
#include <vector>

#include <OpenImageIO/filesystem.h>
Expand Down Expand Up @@ -117,6 +118,14 @@ OIIO_PLUGIN_EXPORTS_END



static std::set<std::string> metadata_include { "oiio:ConstantColor",
"oiio:AverageColor",
"oiio:SHA-1" };
static std::set<std::string> metadata_exclude {
"XResolution", "YResolution", "PixelAspectRatio",
"ResolutionUnit", "Orientation", "ImageDescription"
};

bool
JpgOutput::open(const std::string& name, const ImageSpec& newspec,
OpenMode mode)
Expand Down Expand Up @@ -229,6 +238,36 @@ JpgOutput::open(const std::string& name, const ImageSpec& newspec,
comment.size() + 1);
}

// Write other metadata as JPEG comments if requested
if (m_spec.get_int_attribute("jpeg:com_attributes")) {
for (const auto& p : m_spec.extra_attribs) {
std::string name = p.name().string();
auto colon = name.find(':');
if (metadata_include.count(name)) {
// Allow explicitly included metadata
} else if (metadata_exclude.count(name))
continue; // Suppress metadata that is processed separately
else if (Strutil::istarts_with(name, "ICCProfile"))
continue; // Suppress ICC profile, gets written separately
else if (colon != ustring::npos) {
auto prefix = p.name().substr(0, colon);
if (Strutil::iequals(prefix, "oiio"))
continue; // Suppress internal metadata
else if (Strutil::iequals(prefix, "exif")
|| Strutil::iequals(prefix, "GPS")
|| Strutil::iequals(prefix, "XMP"))
continue; // Suppress EXIF metadata, gets written separately
else if (Strutil::iequals(prefix, "iptc"))
continue; // Suppress IPTC metadata
else if (is_imageio_format_name(prefix))
continue; // Suppress format-specific metadata
}
auto data = p.name().string() + ":" + p.get_string();
jpeg_write_marker(&m_cinfo, JPEG_COM, (JOCTET*)data.c_str(),
data.size());
}
}

if (Strutil::iequals(m_spec.get_string_attribute("oiio:ColorSpace"), "sRGB"))
m_spec.attribute("Exif:ColorSpace", 1);

Expand Down
62 changes: 62 additions & 0 deletions testsuite/jpeg-metadata/ref/out.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
Reading src/blender-render.jpg
src/blender-render.jpg : 640 x 480, 3 channel, uint8 jpeg
SHA-1: A60D05FC42FDEE2FC8907531E3641C17D7C1E3AB
channel list: R, G, B
ImageDescription: "Blender:File:<untitled>"
Blender:Camera: "Camera"
Blender:Date: "2024/09/17 15:50:17"
Blender:File: "<untitled>"
Blender:Frame: "001"
Blender:RenderTime: "00:03.49"
Blender:Scene: "Scene"
Blender:Time: "00:00:00:01"
jpeg:subsampling: "4:2:0"
oiio:ColorSpace: "sRGB"
Comparing "src/blender-render.jpg" and "no-attribs.jpg"
PASS
Reading no-attribs.jpg
no-attribs.jpg : 640 x 480, 3 channel, uint8 jpeg
SHA-1: 329B449C07E6649023504E2C8E5130B41985CF7F
channel list: R, G, B
ImageDescription: "Blender:File:<untitled>"
Blender:File: "<untitled>"
Exif:ColorSpace: 1
Exif:ExifVersion: "0230"
Exif:FlashPixVersion: "0100"
IPTC:Caption: "Blender:File:<untitled>"
jpeg:subsampling: "4:2:0"
oiio:ColorSpace: "sRGB"
Reading src/blender-render.jpg
src/blender-render.jpg : 640 x 480, 3 channel, uint8 jpeg
SHA-1: A60D05FC42FDEE2FC8907531E3641C17D7C1E3AB
channel list: R, G, B
ImageDescription: "Blender:File:<untitled>"
Blender:Camera: "Camera"
Blender:Date: "2024/09/17 15:50:17"
Blender:File: "<untitled>"
Blender:Frame: "001"
Blender:RenderTime: "00:03.49"
Blender:Scene: "Scene"
Blender:Time: "00:00:00:01"
jpeg:subsampling: "4:2:0"
oiio:ColorSpace: "sRGB"
Comparing "src/blender-render.jpg" and "with-attribs.jpg"
PASS
Reading with-attribs.jpg
with-attribs.jpg : 640 x 480, 3 channel, uint8 jpeg
SHA-1: 329B449C07E6649023504E2C8E5130B41985CF7F
channel list: R, G, B
ImageDescription: "Blender:File:<untitled>"
Blender:Camera: "Camera"
Blender:Date: "2024/09/17 15:50:17"
Blender:File: "<untitled>"
Blender:Frame: "001"
Blender:RenderTime: "00:03.49"
Blender:Scene: "Scene"
Blender:Time: "00:00:00:01"
Exif:ColorSpace: 1
Exif:ExifVersion: "0230"
Exif:FlashPixVersion: "0100"
IPTC:Caption: "Blender:File:<untitled>"
jpeg:subsampling: "4:2:0"
oiio:ColorSpace: "sRGB"
23 changes: 23 additions & 0 deletions testsuite/jpeg-metadata/run.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#!/usr/bin/env python

# Copyright Contributors to the OpenImageIO project.
# SPDX-License-Identifier: Apache-2.0
# https://github.com/AcademySoftwareFoundation/OpenImageIO


redirect = ' >> out.txt 2>&1 '

# This file was rendered and saved in Blender, and therefore contains metadata
# in the form of comments.

# Check if the comments are correctly decoded as attributes, and that writing
# to a new JPEG does not include them by default.
command += rw_command ("src", "blender-render.jpg", use_oiiotool=1,
output_filename="no-attribs.jpg")
command += info_command ("no-attribs.jpg", safematch=True)

# Check that, when jpeg:com_attributes is set, the attributes are preserved.
command += rw_command ("src", "blender-render.jpg", use_oiiotool=1,
output_filename="with-attribs.jpg",
extraargs="--attrib:type=int jpeg:com_attributes 1")
command += info_command ("with-attribs.jpg", safematch=True)
Binary file added testsuite/jpeg-metadata/src/blender-render.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading