feat: Refactor writing color space metadata to unify behavior

This adds new pvt::is_colorspace_srgb, pvt::get_colorspace_rec709_gamma,
pvt::get_colorspace_icc_profile and pvt::get_colorspace_cicp functions to
get colorspace metadata from an ImageSpec, to make logic between file
formats consistent.

This fixes an issue where tga and rla were incorrectly using g22_rec709
and g18_rec709 without _scene suffix, and not handling g24_rec709.

There is existing consistency in that some file formats assume an empty
oiio:ColorSpace to mean sRGB, and some don't. This inconsistency is
preserved.

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

Author: brechtvl

src/dpx.imageio/dpxoutput.cpp

@@ -18,6 +18,8 @@
 #include <OpenImageIO/strutil.h>
 #include <OpenImageIO/typedesc.h>
 
+#include "imageio_pvt.h"
+
 OIIO_PLUGIN_NAMESPACE_BEGIN
 
 
@@ -436,18 +438,14 @@ DPXOutput::prep_subimage(int s, bool allocate)
     m_desc = get_image_descriptor();
 
     // transfer function
-    const ColorConfig& colorconfig = ColorConfig::default_colorconfig();
-    std::string colorspace = spec_s.get_string_attribute("oiio:ColorSpace", "");
-    if (colorconfig.equivalent(colorspace, "lin_rec709_scene"))
-        m_transfer = dpx::kLinear;
-    else if (colorconfig.equivalent(colorspace, "srgb_rec709_scene"))
+    const float gamma = pvt::get_colorspace_rec709_gamma(spec_s);
+    if (pvt::is_colorspace_srgb(spec_s, false))
         m_transfer = dpx::kITUR709;
-    else if (colorconfig.equivalent(colorspace, "g22_rec709_scene")
-             || colorconfig.equivalent(colorspace, "g24_rec709_scene")
-             || colorconfig.equivalent(colorspace, "g18_rec709_scene")
-             || Strutil::istarts_with(colorspace, "Gamma"))
+    else if (gamma == 1.0f)
+        m_transfer = dpx::kLinear;
+    else if (gamma != 0.0f)
         m_transfer = dpx::kUserDefined;
-    else if (colorconfig.equivalent(colorspace, "KodakLog"))
+    else if (spec_s.get_string_attribute("oiio:ColorSpace") == "KodakLog")
         m_transfer = dpx::kLogarithmic;
     else {
         std::string dpxtransfer = spec_s.get_string_attribute("dpx:Transfer",

src/heif.imageio/heifoutput.cpp

@@ -10,6 +10,8 @@
 #include <OpenImageIO/platform.h>
 #include <OpenImageIO/tiffutils.h>
 
+#include "imageio_pvt.h"
+
 #include <libheif/heif_cxx.h>
 
 #define MAKE_LIBHEIF_VERSION(a, b, c, d) \
@@ -250,12 +252,7 @@ HeifOutput::close()
         std::unique_ptr<heif_color_profile_nclx,
                         void (*)(heif_color_profile_nclx*)>
             nclx(heif_nclx_color_profile_alloc(), heif_nclx_color_profile_free);
-        const ColorConfig& colorconfig(ColorConfig::default_colorconfig());
-        const ParamValue* p    = m_spec.find_attribute("CICP",
-                                                       TypeDesc(TypeDesc::INT, 4));
-        string_view colorspace = m_spec.get_string_attribute("oiio:ColorSpace");
-        cspan<int> cicp        = (p) ? p->as_cspan<int>()
-                                     : colorconfig.get_cicp(colorspace);
+        cspan<int> cicp = pvt::get_colorspace_cicp(m_spec);
         if (!cicp.empty()) {
             nclx->color_primaries          = heif_color_primaries(cicp[0]);
             nclx->transfer_characteristics = heif_transfer_characteristics(

src/include/OpenImageIO/imageio.h

@@ -4198,7 +4198,6 @@ OIIO_API void set_colorspace(ImageSpec& spec, string_view name);
 /// @version 3.0
 OIIO_API void set_colorspace_rec709_gamma(ImageSpec& spec, float gamma);
 
-
 /// Are the two named color spaces equivalent, based on the default color
 /// config in effect?
 ///

src/include/imageio_pvt.h

@@ -127,6 +127,31 @@ parallel_convert_from_float(const float* src, void* dst, size_t nvals,
 OIIO_API bool
 check_texture_metadata_sanity(ImageSpec& spec);
 
+/// Returns true if for the purpose of interop, the spec's metadata
+/// specifies a color space that should be encoded as sRGB.
+///
+/// If default_to_srgb is true, the colorspace will be assumed to
+/// be sRGB if no colorspace was specified in the spec.
+OIIO_API bool
+is_colorspace_srgb(const ImageSpec& spec, bool default_to_srgb = true);
+
+/// If the spec's metadata specifies a color space with Rec709 primaries and
+/// gamma transfer function, return the gamma value. If not, return zero.
+OIIO_API float
+get_colorspace_rec709_gamma(const ImageSpec& spec);
+
+// Returns ICC profile from the spec's metadata, either from an ICCProfile
+// attribute or from the colorspace if from_colorspace is true.
+// Returns an empty vector if not found.
+std::vector<uint8_t>
+get_colorspace_icc_profile(const ImageSpec& spec, bool from_colorspace = true);
+
+// Returns ICC profile from the spec's metadata, either from a CICP attribute
+// or from the colorspace if from_colorspace is true.
+// Returns an empty span if not found.
+cspan<int>
+get_colorspace_cicp(const ImageSpec& spec, bool from_colorspace = true);
+
 /// Get the timing report from log_time entries.
 OIIO_API std::string
 timing_report();

src/iv/imageviewer.cpp

@@ -44,22 +44,11 @@
 #include <OpenImageIO/sysutil.h>
 #include <OpenImageIO/timer.h>
 
+#include "imageio_pvt.h"
 
 #include "ivutils.h"
 
 
-namespace {
-
-inline bool
-IsSpecSrgb(const ImageSpec& spec)
-{
-    return equivalent_colorspace(spec.get_string_attribute("oiio:ColorSpace"),
-                                 "srgb_rec709_scene");
-}
-
-}  // namespace
-
-
 // clang-format off
 static const char *s_file_filters = ""
     "Image Files (*.bmp *.cin *.dcm *.dds *.dpx *.fits *.gif *.hdr *.ico *.iff "
@@ -1242,7 +1231,8 @@ ImageViewer::loadCurrentImage(int subimage, int miplevel)
                 //std::cerr << "Loading HALF-FLOAT as FLOAT\n";
                 read_format = TypeDesc::FLOAT;
             }
-            if (IsSpecSrgb(image_spec) && !glwin->is_srgb_capable()) {
+            if (pvt::is_colorspace_srgb(image_spec)
+                && !glwin->is_srgb_capable()) {
                 // If the image is in sRGB, but OpenGL can't load sRGB textures then
                 // we'll need to do the transformation on the CPU after loading the
                 // image. We (so far) can only do this with UINT8 images, so make
@@ -1257,7 +1247,8 @@ ImageViewer::loadCurrentImage(int subimage, int miplevel)
             read_format      = TypeDesc::UINT8;
             allow_transforms = true;
 
-            if (IsSpecSrgb(image_spec) && !glwin->is_srgb_capable())
+            if (pvt::is_colorspace_srgb(image_spec)
+                && !glwin->is_srgb_capable())
                 srgb_transform = true;
         }
 
@@ -1443,7 +1434,7 @@ ImageViewer::exposureMinusOneTenthStop()
     img->exposure(img->exposure() - 0.1);
     if (!glwin->is_glsl_capable()) {
         bool srgb_transform = (!glwin->is_srgb_capable()
-                               && IsSpecSrgb(img->spec()));
+                               && pvt::is_colorspace_srgb(img->spec()));
         img->pixel_transform(srgb_transform, (int)current_color_mode(),
                              current_channel());
         displayCurrentImage();
@@ -1462,7 +1453,7 @@ ImageViewer::exposureMinusOneHalfStop()
     img->exposure(img->exposure() - 0.5);
     if (!glwin->is_glsl_capable()) {
         bool srgb_transform = (!glwin->is_srgb_capable()
-                               && IsSpecSrgb(img->spec()));
+                               && pvt::is_colorspace_srgb(img->spec()));
         img->pixel_transform(srgb_transform, (int)current_color_mode(),
                              current_channel());
         displayCurrentImage();
@@ -1481,7 +1472,7 @@ ImageViewer::exposurePlusOneTenthStop()
     img->exposure(img->exposure() + 0.1);
     if (!glwin->is_glsl_capable()) {
         bool srgb_transform = (!glwin->is_srgb_capable()
-                               && IsSpecSrgb(img->spec()));
+                               && pvt::is_colorspace_srgb(img->spec()));
         img->pixel_transform(srgb_transform, (int)current_color_mode(),
                              current_channel());
         displayCurrentImage();
@@ -1500,7 +1491,7 @@ ImageViewer::exposurePlusOneHalfStop()
     img->exposure(img->exposure() + 0.5);
     if (!glwin->is_glsl_capable()) {
         bool srgb_transform = (!glwin->is_srgb_capable()
-                               && IsSpecSrgb(img->spec()));
+                               && pvt::is_colorspace_srgb(img->spec()));
         img->pixel_transform(srgb_transform, (int)current_color_mode(),
                              current_channel());
         displayCurrentImage();
@@ -1520,7 +1511,7 @@ ImageViewer::gammaMinus()
     img->gamma(img->gamma() - 0.05);
     if (!glwin->is_glsl_capable()) {
         bool srgb_transform = (!glwin->is_srgb_capable()
-                               && IsSpecSrgb(img->spec()));
+                               && pvt::is_colorspace_srgb(img->spec()));
         img->pixel_transform(srgb_transform, (int)current_color_mode(),
                              current_channel());
         displayCurrentImage();
@@ -1539,7 +1530,7 @@ ImageViewer::gammaPlus()
     img->gamma(img->gamma() + 0.05);
     if (!glwin->is_glsl_capable()) {
         bool srgb_transform = (!glwin->is_srgb_capable()
-                               && IsSpecSrgb(img->spec()));
+                               && pvt::is_colorspace_srgb(img->spec()));
         img->pixel_transform(srgb_transform, (int)current_color_mode(),
                              current_channel());
         displayCurrentImage();
@@ -1572,7 +1563,7 @@ ImageViewer::viewChannel(int c, COLOR_MODE colormode)
             IvImage* img = cur();
             if (img) {
                 bool srgb_transform = (!glwin->is_srgb_capable()
-                                       && IsSpecSrgb(img->spec()));
+                                       && pvt::is_colorspace_srgb(img->spec()));
                 img->pixel_transform(srgb_transform, (int)colormode, c);
             }
         } else {

src/iv/ivgl.cpp

@@ -18,6 +18,7 @@
 #    include <QPen>
 #endif
 
+#include "imageio_pvt.h"
 #include "ivutils.h"
 #include <OpenImageIO/strutil.h>
 #include <OpenImageIO/timer.h>
@@ -2198,9 +2199,7 @@ IvGL::typespec_to_opengl(const ImageSpec& spec, int nchannels, GLenum& gltype,
         break;
     }
 
-    bool issrgb
-        = equivalent_colorspace(spec.get_string_attribute("oiio:ColorSpace"),
-                                "srgb_rec709_scene");
+    bool issrgb = pvt::is_colorspace_srgb(spec);
 
     glinternalformat = nchannels;
     if (nchannels == 1) {

src/jpeg.imageio/jpegoutput.cpp

@@ -12,6 +12,8 @@
 #include <OpenImageIO/imageio.h>
 #include <OpenImageIO/tiffutils.h>
 
+#include "imageio_pvt.h"
+
 #include "jpeg_pvt.h"
 
 OIIO_PLUGIN_NAMESPACE_BEGIN
@@ -268,8 +270,7 @@ JpgOutput::open(const std::string& name, const ImageSpec& newspec,
         }
     }
 
-    if (equivalent_colorspace(m_spec.get_string_attribute("oiio:ColorSpace"),
-                              "srgb_rec709_scene"))
+    if (pvt::is_colorspace_srgb(m_spec, false))
         m_spec.attribute("Exif:ColorSpace", 1);
 
     // Write EXIF info
@@ -320,37 +321,32 @@ JpgOutput::open(const std::string& name, const ImageSpec& newspec,
     m_spec.set_format(TypeDesc::UINT8);  // JPG is only 8 bit
 
     // Write ICC profile, if we have anything
-    if (auto icc_profile_parameter = m_spec.find_attribute(ICC_PROFILE_ATTR)) {
-        cspan<unsigned char> icc_profile((unsigned char*)
-                                             icc_profile_parameter->data(),
-                                         icc_profile_parameter->type().size());
-        if (icc_profile.size() && icc_profile.data()) {
-            /* Calculate the number of markers we'll need, rounding up of course */
-            size_t num_markers = icc_profile.size() / MAX_DATA_BYTES_IN_MARKER;
-            if (num_markers * MAX_DATA_BYTES_IN_MARKER
-                != std::size(icc_profile))
-                num_markers++;
-            int curr_marker = 1; /* per spec, count starts at 1*/
-            std::vector<JOCTET> profile(MAX_DATA_BYTES_IN_MARKER
-                                        + ICC_HEADER_SIZE);
-            size_t icc_profile_length = icc_profile.size();
-            while (icc_profile_length > 0) {
-                // length of profile to put in this marker
-                size_t length = std::min(icc_profile_length,
-                                         size_t(MAX_DATA_BYTES_IN_MARKER));
-                icc_profile_length -= length;
-                // Write the JPEG marker header (APP2 code and marker length)
-                strcpy((char*)profile.data(), "ICC_PROFILE");  // NOSONAR
-                profile[11] = 0;
-                profile[12] = curr_marker;
-                profile[13] = (JOCTET)num_markers;
-                OIIO_ASSERT(profile.size() >= ICC_HEADER_SIZE + length);
-                spancpy(make_span(profile), ICC_HEADER_SIZE, icc_profile,
-                        length * (curr_marker - 1), length);
-                jpeg_write_marker(&m_cinfo, JPEG_APP0 + 2, profile.data(),
-                                  ICC_HEADER_SIZE + length);
-                curr_marker++;
-            }
+    std::vector<uint8_t> icc_profile = pvt::get_colorspace_icc_profile(m_spec);
+    if (icc_profile.size()) {
+        /* Calculate the number of markers we'll need, rounding up of course */
+        size_t num_markers = icc_profile.size() / MAX_DATA_BYTES_IN_MARKER;
+        if (num_markers * MAX_DATA_BYTES_IN_MARKER != std::size(icc_profile))
+            num_markers++;
+        int curr_marker = 1; /* per spec, count starts at 1*/
+        std::vector<JOCTET> profile(MAX_DATA_BYTES_IN_MARKER + ICC_HEADER_SIZE);
+        size_t icc_profile_length = icc_profile.size();
+        while (icc_profile_length > 0) {
+            // length of profile to put in this marker
+            size_t length = std::min(icc_profile_length,
+                                     size_t(MAX_DATA_BYTES_IN_MARKER));
+            icc_profile_length -= length;
+            // Write the JPEG marker header (APP2 code and marker length)
+            strcpy((char*)profile.data(), "ICC_PROFILE");  // NOSONAR
+            profile[11] = 0;
+            profile[12] = curr_marker;
+            profile[13] = (JOCTET)num_markers;
+            OIIO_ASSERT(profile.size() >= ICC_HEADER_SIZE + length);
+            spancpy(make_span(profile), ICC_HEADER_SIZE,
+                    cspan<uint8_t>(icc_profile), length * (curr_marker - 1),
+                    length);
+            jpeg_write_marker(&m_cinfo, JPEG_APP0 + 2, profile.data(),
+                              ICC_HEADER_SIZE + length);
+            curr_marker++;
         }
     }
 

src/jpeg2000.imageio/jpeg2000output.cpp

@@ -481,10 +481,10 @@ Jpeg2000Output::create_jpeg2000_image()
     // someboody comes along that desperately needs JPEG2000 and ICC
     // profiles, maybe they will be motivated enough to track down the
     // problem.
-    const ParamValue *icc = m_spec.find_attribute ("ICCProfile");
-    if (icc && icc->type().basetype == TypeDesc::UINT8 && icc->type().arraylen > 0) {
-        m_image->icc_profile_len = icc->type().arraylen;
-        m_image->icc_profile_buf = (unsigned char *) icc->data();
+    std::vector<uint8_t> icc_profile = get_colorspace_icc_profile(m_spec);
+    if (icc_profile.size()) {
+        m_image->icc_profile_len = icc_profile.size();
+        m_image->icc_profile_buf = (unsigned char *) icc_profile.data();
     }
 #endif
 

src/jpegxl.imageio/jxloutput.cpp

@@ -3,6 +3,7 @@
 // https://github.com/AcademySoftwareFoundation/OpenImageIO
 
 #include <cassert>
+#include <cstdint>
 #include <cstdio>
 #include <vector>
 
@@ -11,6 +12,8 @@
 #include <OpenImageIO/imageio.h>
 #include <OpenImageIO/tiffutils.h>
 
+#include "imageio_pvt.h"
+
 #include <jxl/decode.h>
 #include <jxl/encode.h>
 #include <jxl/encode_cxx.h>
@@ -539,18 +542,12 @@ JxlOutput::save_image(const void* data)
     }
 
     // Write the ICC profile, if available
-    const ParamValue* icc_profile_parameter = m_spec.find_attribute(
-        "ICCProfile");
-    if (icc_profile_parameter != nullptr) {
-        unsigned char* icc_profile
-            = (unsigned char*)icc_profile_parameter->data();
-        uint32_t length = icc_profile_parameter->type().size();
-        if (icc_profile && length) {
-            if (JXL_ENC_SUCCESS
-                != JxlEncoderSetICCProfile(m_encoder.get(), icc_profile,
-                                           length)) {
-                errorfmt("JxlEncoderSetICCProfile failed\n");
-            }
+    std::vector<uint8_t> icc_profile = pvt::get_colorspace_icc_profile(m_spec);
+    if (icc_profile.size()) {
+        if (JXL_ENC_SUCCESS
+            != JxlEncoderSetICCProfile(m_encoder.get(), icc_profile.data(),
+                                       icc_profile.size())) {
+            errorfmt("JxlEncoderSetICCProfile failed\n");
         }
     }