- replacing getAMFTransformIDs and getICCProfileName with more generic getInterchangeAttribute function. Same with the setters

- getInterchangeAttribute() and setInterchangeAttribute() functions currently knows and accepts "amf_transform_ids" and "icc_profile_name" keys. other keys will throw.
- ColorSpace serializer will list all of the key/value pairs.
- InteropID and interchangeAttributes are now allowed in earlier config versions down to 2.0 too. Config::checkVersionConsistency() will throw only for version smaller than 2.0.
- YAML loader and saver now supports the "interchange" section. Any unknown keys under that section will be ignored upon load and will generate a warning but won't throw.
- Generic str key/value loader/saver is extended for re usability (and fixed incorrect throw message).
- since the amf keywords are separated by newline characters, the newline sanitizer that was used for the description is also generalized and applied to all of the current and possible future interchange fields.
- ColorSpace_test.cpp file updated to test the current state and behavior of the API.
- ComputeValues function in the CPUProcessor_tests.cpp was taking the linenumber as as template parameter. This was a very bad idea as that means each invocation of the function would create a separate copy of the function which was causing all sorts of issues in the debugger and failing to compile when the line numbers are not constants (happens in JIT debug sessions). Fixed by making the line number a normal parameter.
- Python ColorSpace object no longer takes the amf of icc parameters in the constructor. User needs to set them explicitely later on.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

Author: cozdas

include/OpenColorIO/OpenColorIO.h

@@ -13,6 +13,7 @@
 #include <fstream>
 #include <vector>
 #include <cstdint>
+#include <map>
 
 #include "OpenColorABI.h"
 #include "OpenColorTypes.h"
@@ -1992,23 +1993,25 @@ class OCIOEXPORT ColorSpace
     void setInteropID(const char * interopID);
 
     /**
-     * Get/Set the AMF transform IDs for the color space.
-     * 
-     * The AMF transform IDs are used to identify specific transforms in the ACES Metadata File.
-     * Multiple transform IDs can be specified in a newline-separated string.
-     */
-    const char * getAMFTransformIDs() const noexcept;
-    void setAMFTransformIDs(const char * amfTransformIDs);
-
-    /**
-     * Get/Set the ICC profile name for the color space.
-     * 
-     * The ICC profile name identifies the ICC color profile associated with this color space.
-     * This can be used to link OCIO color spaces with corresponding ICC profiles for
-     * applications that need to work with both color management systems.
+     * Get/Set the interchange attributes.
+     *
+     * Currently supported attribute names are "amf_transform_ids" and
+     * "icc_profile_name". Using any other name will throw. If the attribute is
+     * not defined, it'll return empty string. Similarly setting it to empty
+     * string will effectively delete the attribute.
+     *
+     * The AMF transform IDs are used to identify specific transforms in the
+     * ACES Metadata File. Multiple transform IDs can be specified in a
+     * newline-separated string.
+     *
+     * The ICC profile name identifies the ICC color profile associated with
+     * this color space. This can be used to link OCIO color spaces with
+     * corresponding ICC profiles for applications that need to work with both
+     * color management systems.
      */
-    const char * getICCProfileName() const noexcept;
-    void setICCProfileName(const char * iccProfileName);
+    const char *getInterchangeAttribute(const char *attrName) const;
+    void setInterchangeAttribute(const char* attrName, const char *value);
+    std::map<std::string, std::string> getInterchangeAttributes() const noexcept;
 
     BitDepth getBitDepth() const noexcept;
     void setBitDepth(BitDepth bitDepth);

src/OpenColorIO/ColorSpace.cpp

@@ -4,6 +4,7 @@
 #include <cstring>
 #include <sstream>
 #include <vector>
+#include <map>
 
 #include <OpenColorIO/OpenColorIO.h>
 
@@ -13,6 +14,13 @@
 #include "utils/StringUtils.h"
 
 
+namespace
+{
+const std::array<const std::string, 2> knownInterchangeNames = {
+    "amf_transform_ids",
+    "icc_profile_name" };
+}
+
 namespace OCIO_NAMESPACE
 {
 
@@ -25,9 +33,8 @@ class ColorSpace::Impl
     std::string m_description;
     std::string m_encoding;
     std::string m_interopID;
-    std::string m_AMFTransformIDs;
-    std::string m_ICCProfileName;
     StringUtils::StringVec m_aliases;
+    std::map<std::string, std::string> m_interchangeAttribs;
 
     BitDepth m_bitDepth{ BIT_DEPTH_UNKNOWN };
     bool m_isData{ false };
@@ -66,8 +73,7 @@ class ColorSpace::Impl
             m_description = rhs.m_description;
             m_encoding = rhs.m_encoding;
             m_interopID = rhs.m_interopID;
-            m_AMFTransformIDs = rhs.m_AMFTransformIDs;
-            m_ICCProfileName = rhs.m_ICCProfileName;
+            m_interchangeAttribs= rhs.m_interchangeAttribs;
             m_bitDepth = rhs.m_bitDepth;
             m_isData = rhs.m_isData;
             m_referenceSpaceType = rhs.m_referenceSpaceType;
@@ -277,27 +283,60 @@ void ColorSpace::setInteropID(const char * interopID)
     getImpl()->m_interopID = id;
 }
 
-const char * ColorSpace::getAMFTransformIDs() const noexcept
+const char * ColorSpace::getInterchangeAttribute(const char* attrName) const
 {
-    return getImpl()->m_AMFTransformIDs.c_str();
-}
+    std::string nameTrimmed = StringUtils::Trim(attrName);
+    for (auto& key : knownInterchangeNames)
+    {
+        // do case-insensitive comparison.
+        if (StringUtils::Compare(key, nameTrimmed))
+        {
+            auto it = m_impl->m_interchangeAttribs.find(key);
+            if (it != m_impl->m_interchangeAttribs.end())
+            {
+                return it->second.c_str();
+            }
+            return "";
+        }
+    }
 
-void ColorSpace::setAMFTransformIDs(const char * amfTransformIDs)
-{
-    getImpl()->m_AMFTransformIDs = amfTransformIDs ? amfTransformIDs : "";
+    std::ostringstream oss;
+    oss << "Unknown attribute name '" << attrName << "'.";
+    throw Exception(oss.str().c_str());
 }
 
-const char * ColorSpace::getICCProfileName() const noexcept
+void ColorSpace::setInterchangeAttribute(const char* attrName, const char* value)
 {
-    return getImpl()->m_ICCProfileName.c_str();
+    std::string nameTrimmed = StringUtils::Trim(attrName);
+    for (auto& key : knownInterchangeNames)
+    {
+        // Do case-insensitive comparison.
+        if (StringUtils::Compare(key, nameTrimmed))
+        {
+            // use key instead of nameTrim for storing in correct capitalization.
+            if (!value || !*value)
+            {
+                m_impl->m_interchangeAttribs.erase(key);
+            } 
+            else
+            {
+                m_impl->m_interchangeAttribs[key] = value;
+            }
+            return;
+        }
+    }
+
+    std::ostringstream oss;
+    oss << "Unknown attribute name '" << attrName << "'.";
+    throw Exception(oss.str().c_str());
 }
 
-void ColorSpace::setICCProfileName(const char * iccProfileName)
+std::map<std::string, std::string> ColorSpace::getInterchangeAttributes() const noexcept
 {
-    getImpl()->m_ICCProfileName = iccProfileName ? iccProfileName : "";
+    return m_impl->m_interchangeAttribs;
 }
 
-BitDepth ColorSpace::getBitDepth() const noexcept
+BitDepth ColorSpace::ColorSpace::getBitDepth() const noexcept
 {
     return getImpl()->m_bitDepth;
 }
@@ -516,15 +555,10 @@ std::ostream & operator<< (std::ostream & os, const ColorSpace & cs)
     {
         os << ", description=" << str;
     }
-    str = cs.getAMFTransformIDs();
-    if (!str.empty())
-    {
-        os << ", amf_transform_ids=" << str;
-    }
-    str = cs.getICCProfileName();
-    if (!str.empty())
+    // TODO: Do we need to output the section name too?
+    for (const auto& attr : cs.getInterchangeAttributes())
     {
-        os << ", icc_profile_name=" << str;
+        os << ", " << attr.first << "=" << attr.second;
     }
     if(cs.getTransform(COLORSPACE_DIR_TO_REFERENCE))
     {

src/OpenColorIO/Config.cpp

@@ -5675,27 +5675,20 @@ void Config::Impl::checkVersionConsistency() const
             }
         } 
 
-        if (hexVersion < 0x02050000) // Version 2.5
+        if (m_majorVersion < 2) 
         {
             if (*cs->getInteropID())
             {
                 std::ostringstream os;
                 os << "Config failed validation. The color space '" << cs->getName() << "' ";
-                os << "has non-empty InteropID and config version is less than 2.5.";
-                throw Exception(os.str().c_str());
-            }
-            if (*cs->getAMFTransformIDs())
-            {
-                std::ostringstream os;
-                os << "Config failed validation. The color space '" << cs->getName() << "' ";
-                os << "has non-empty AMFTransformIDs and config version is less than 2.5.";
+                os << "has non-empty InteropID and config version is less than 2.0.";
                 throw Exception(os.str().c_str());
             }
-            if (*cs->getICCProfileName())
+            if (cs->getInterchangeAttributes().size()>0)
             {
                 std::ostringstream os;
                 os << "Config failed validation. The color space '" << cs->getName() << "' ";
-                os << "has non-empty ICCProfileName and config version is less than 2.5.";
+                os << "has non-empty interchange attributes and config version is less than 2.0.";
                 throw Exception(os.str().c_str());
             }
         }