feat: Refactor writing color space metadata to unify behavior

This adds new functions to get and set colorspaces in a ImageSpace, to
make logic between file formats more consistent.
* pvt::set_colorspace_srgb and pvt::is_colorspace_srgb
* pvt::get_colorspace_rec709_gamma
* pvt::get_colorspace_icc_profile
* pvt::set_colorspace_cicp and pvt::get_colorspace_cicp

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 <[email protected]>

Author: brechtvl

src/bmp.imageio/bmpinput.cpp

@@ -8,6 +8,8 @@
 #include <OpenImageIO/fmath.h>
 #include <OpenImageIO/imageio.h>
 
+#include "imageio_pvt.h"
+
 #include "bmp_pvt.h"
 
 OIIO_PLUGIN_NAMESPACE_BEGIN
@@ -262,7 +264,8 @@ BmpInput::open(const std::string& name, ImageSpec& newspec,
     // Default presumption is that a BMP file is meant to look reasonable on a
     // display, so assume it's sRGB. This is not really correct -- see the
     // comments below.
-    m_spec.attribute("oiio:ColorSpace", "srgb_rec709_scene");
+    const bool erase_other_attributes = false;
+    pvt::set_colorspace_srgb(m_spec, erase_other_attributes);
 #if 0
     if (m_dib_header.size >= WINDOWS_V4
         && m_dib_header.cs_type == CSType::CalibratedRGB) {

src/dds.imageio/ddsinput.cpp

@@ -15,6 +15,8 @@
 #include <OpenImageIO/parallel.h>
 #include <OpenImageIO/typedesc.h>
 
+#include "imageio_pvt.h"
+
 #include "dds_pvt.h"
 #define BCDEC_IMPLEMENTATION
 #include "bcdec.h"
@@ -828,7 +830,7 @@ DDSInput::seek_subimage(int subimage, int miplevel)
     if (bpp != 0)
         m_spec.attribute("oiio:BitsPerSample", bpp);
 
-    const char* colorspace = nullptr;
+    bool is_srgb = false;
 
     if (m_dds.fmt.fourCC == DDS_4CC_DX10) {
         switch (m_dx10.dxgiFormat) {
@@ -838,18 +840,17 @@ DDSInput::seek_subimage(int subimage, int miplevel)
         case DDS_FORMAT_BC7_UNORM_SRGB:
         case DDS_FORMAT_R8G8B8A8_UNORM_SRGB:
         case DDS_FORMAT_B8G8R8A8_UNORM_SRGB:
-        case DDS_FORMAT_B8G8R8X8_UNORM_SRGB:
-            colorspace = "srgb_rec709_scene";
-            break;
+        case DDS_FORMAT_B8G8R8X8_UNORM_SRGB: is_srgb = true; break;
         }
     }
 
-    // linear color space for HDR-ish images
-    if (colorspace == nullptr
-        && (basetype == TypeDesc::HALF || basetype == TypeDesc::FLOAT))
-        colorspace = "lin_rec709_scene";
-
-    m_spec.set_colorspace(colorspace);
+    if (is_srgb) {
+        pvt::set_colorspace_srgb(m_spec);
+    } else if (!is_srgb
+               && (basetype == TypeDesc::HALF || basetype == TypeDesc::FLOAT)) {
+        // linear color space for HDR-ish images
+        m_spec.set_colorspace("lin_rec709_scene");
+    }
 
     m_spec.default_channel_names();
     // Special case: if a 2-channel DDS RG or YA?

src/dpx.imageio/dpxinput.cpp

@@ -29,6 +29,8 @@
 #include <OpenImageIO/strutil.h>
 #include <OpenImageIO/typedesc.h>
 
+#include "imageio_pvt.h"
+
 OIIO_PLUGIN_NAMESPACE_BEGIN
 
 
@@ -314,7 +316,7 @@ DPXInput::seek_subimage(int subimage, int miplevel)
     switch (m_dpx.header.Transfer(subimage)) {
     case dpx::kLinear: m_spec.set_colorspace("lin_rec709_scene"); break;
     case dpx::kLogarithmic: m_spec.set_colorspace("KodakLog"); break;
-    case dpx::kITUR709: m_spec.set_colorspace("srgb_rec709_scene"); break;
+    case dpx::kITUR709: pvt::set_colorspace_srgb(m_spec); break;
     case dpx::kUserDefined:
         if (!std::isnan(m_dpx.header.Gamma()) && m_dpx.header.Gamma() != 0) {
             set_colorspace_rec709_gamma(m_spec, float(m_dpx.header.Gamma()));

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/ffmpeg.imageio/ffmpeginput.cpp

@@ -71,6 +71,7 @@ receive_frame(AVCodecContext* avctx, AVFrame* picture, AVPacket* avpkt)
 
 
 
+#include "imageio_pvt.h"
 #include <OpenImageIO/color.h>
 #include <OpenImageIO/imageio.h>
 #include <iostream>
@@ -549,11 +550,7 @@ FFmpegInput::open(const std::string& name, ImageSpec& spec)
         = { m_codec_context->color_primaries, m_codec_context->color_trc,
             m_codec_context->colorspace,
             m_codec_context->color_range == AVCOL_RANGE_MPEG ? 0 : 1 };
-    m_spec.attribute("CICP", TypeDesc(TypeDesc::INT, 4), cicp);
-    const ColorConfig& colorconfig(ColorConfig::default_colorconfig());
-    string_view interop_id = colorconfig.get_color_interop_id(cicp);
-    if (!interop_id.empty())
-        m_spec.attribute("oiio:ColorSpace", interop_id);
+    pvt::set_colorspace_cicp(m_spec, cicp);
 
     m_nsubimages = m_frames;
     spec         = m_spec;

src/gif.imageio/gifinput.cpp

@@ -12,6 +12,8 @@
 #include <OpenImageIO/imageio.h>
 #include <OpenImageIO/thread.h>
 
+#include "imageio_pvt.h"
+
 // GIFLIB:
 // http://giflib.sourceforge.net/
 // Format description:
@@ -259,7 +261,7 @@ GIFInput::read_subimage_metadata(ImageSpec& newspec)
     newspec.nchannels = 4;
     newspec.default_channel_names();
     newspec.alpha_channel = 4;
-    newspec.set_colorspace("srgb_rec709_scene");
+    pvt::set_colorspace_srgb(newspec);
 
     m_previous_disposal_method = m_disposal_method;
     m_disposal_method          = DISPOSAL_UNSPECIFIED;

src/heif.imageio/heifinput.cpp

@@ -9,6 +9,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) \
@@ -273,7 +275,7 @@ HeifInput::seek_subimage(int subimage, int miplevel)
     if (m_bitdepth > 8) {
         m_spec.attribute("oiio:BitsPerSample", m_bitdepth);
     }
-    m_spec.set_colorspace("srgb_rec709_scene");
+    pvt::set_colorspace_srgb(m_spec);
 
 #if LIBHEIF_HAVE_VERSION(1, 9, 0)
     // Read CICP. Have to use the C API to get it from the image handle,
@@ -292,12 +294,7 @@ HeifInput::seek_subimage(int subimage, int miplevel)
                                       int(nclx->transfer_characteristics),
                                       int(nclx->matrix_coefficients),
                                       int(nclx->full_range_flag ? 1 : 0) };
-                m_spec.attribute("CICP", TypeDesc(TypeDesc::INT, 4), cicp);
-                const ColorConfig& colorconfig(
-                    ColorConfig::default_colorconfig());
-                string_view interop_id = colorconfig.get_color_interop_id(cicp);
-                if (!interop_id.empty())
-                    m_spec.attribute("oiio:ColorSpace", interop_id);
+                pvt::set_colorspace_cicp(m_spec, cicp);
             }
             heif_nclx_color_profile_free(nclx);
         }

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/iconvert/iconvert.cpp

@@ -18,6 +18,8 @@
 #include <OpenImageIO/strutil.h>
 #include <OpenImageIO/sysutil.h>
 
+#include "imageio_pvt.h"
+
 
 using namespace OIIO;
 
@@ -224,7 +226,7 @@ adjust_spec(ImageInput* in, ImageOutput* out, const ImageSpec& inspec,
     if (gammaval != 1.0f)
         outspec.attribute("oiio:Gamma", gammaval);
     if (sRGB) {
-        outspec.set_colorspace("srgb_rec709_scene");
+        pvt::set_colorspace_srgb(outspec);
         if (!strcmp(in->format_name(), "jpeg")
             || outspec.find_attribute("Exif:ColorSpace"))
             outspec.attribute("Exif:ColorSpace", 1);

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?
 ///