Skip to content

Commit

Permalink
fix(oiiotool): --autocc bugfix and color config inventory cleanup (#4060
Browse files Browse the repository at this point in the history 
)

The important bug fix is in oiiotool image input when autocc is used,
where we reversed the sense of a test and did the autocc-upon-input if
the file's color space was equivalent to the scene_linear space
(pointlessly), and skipped the conversion if they were different (oh no,
big bug!).

Along the way:

* Add missing try/catch around OCIO call that should have had it.

* Some very ugly SPI-specific recognize-by-name color space heuristics.
I hate this, sorry, but it solves a really important problem for me.

* A bunch of additional debugging messages during color space inventory,
enabled only when debugging, so nobody should see these but me.

* A couple places where we canonicalize the spelling of
"oiio:ColorSpace".

Note that there is still an issue with the color space classification
using OCIO 2.3+'s identification of built-in equivalents by name. These
are shockingly expensive. I will return to this later.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
  • Loading branch information
@lgritz
lgritz authored Nov 24, 2023
1 parent b0e6530 commit b8723ec
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 29 deletions.
5 changes: 5 additions & 0 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ if (${PROJ_NAME}_NAMESPACE_INCLUDE_PATCH)
endif ()
message(STATUS "Setting Namespace to: ${PROJ_NAMESPACE_V}")

set (OIIO_SITE "" CACHE STRING "Site name for customization")
if (OIIO_SITE)
add_definitions (-DOIIO_SITE_${OIIO_SITE})
endif ()

# Define OIIO_INTERNAL symbol only when building OIIO itself, will not be
# defined for downstream projects using OIIO.
add_definitions (-DOIIO_INTERNAL=1)
Expand Down
78 changes: 61 additions & 17 deletions src/libOpenImageIO/color_ocio.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ static const Imath::C3f test_colors[n_test_colors]
} // namespace


#if 0 /* allow color configuration debugging */
#if 0 || !defined(NDEBUG) /* allow color configuration debugging */
static bool colordebug = Strutil::stoi(Sysutil::getenv("OIIO_COLOR_DEBUG"));
# define DBG(...) \
if (colordebug) \
Expand Down Expand Up @@ -403,6 +403,13 @@ class ColorConfig::Impl {
}
}

void debug_print_aliases()
{
DBG("Aliases: scene_linear={} lin_srgb={} srgb={} ACEScg={} Rec709={}\n",
scene_linear_alias, lin_srgb_alias, srgb_alias, ACEScg_alias,
Rec709_alias);
}

// For OCIO 2.3+, we can ask for the equivalent of some built-in
// color spaces.
void identify_builtin_equivalents();
Expand Down Expand Up @@ -490,10 +497,15 @@ OCIO::ConstCPUProcessorRcPtr
ColorConfig::Impl::get_to_builtin_cpu_proc(const char* my_from,
const char* builtin_to) const
{
auto proc = OCIO::Config::GetProcessorToBuiltinColorSpace(config_, my_from,
builtin_to);
return proc ? proc->getDefaultCPUProcessor()
: OCIO::ConstCPUProcessorRcPtr();
try {
auto proc = OCIO::Config::GetProcessorToBuiltinColorSpace(config_,
my_from,
builtin_to);
return proc ? proc->getDefaultCPUProcessor()
: OCIO::ConstCPUProcessorRcPtr();
} catch (...) {
return {};
}
}

#endif
Expand Down Expand Up @@ -592,6 +604,16 @@ ColorConfig::Impl::classify_by_name(CSInfo& cs)
cs.setflag(CSInfo::is_ACEScg | CSInfo::is_linear_response,
ACEScg_alias);
}
#ifdef OIIO_SITE_spi
// Ugly SPI-specific hacks, so sorry
else if (Strutil::starts_with(cs.name, "cgln")) {
cs.setflag(CSInfo::is_ACEScg | CSInfo::is_linear_response,
ACEScg_alias);
} else if (cs.name == "srgbf" || cs.name == "srgbh" || cs.name == "srgb16"
|| cs.name == "srgb8") {
cs.setflag(CSInfo::is_srgb, srgb_alias);
}
#endif

// Set up some canonical names
if (cs.flags() & CSInfo::is_srgb)
Expand All @@ -602,13 +624,19 @@ ColorConfig::Impl::classify_by_name(CSInfo& cs)
cs.canonical = "lin_srgb";
else if (cs.flags() & CSInfo::is_ACEScg)
cs.canonical = "ACEScg";
if (cs.canonical.size()) {
DBG("classify by name identified '{}' as canonical {}\n", cs.name,
cs.canonical);
cs.examined = true;
}
}



void
ColorConfig::Impl::classify_by_conversions(CSInfo& cs)
{
DBG("classifying by conversions {}\n", cs.name);
if (cs.examined)
return; // Already classified

Expand Down Expand Up @@ -698,26 +726,36 @@ void
ColorConfig::Impl::identify_builtin_equivalents()
{
#if OCIO_VERSION_HEX >= MAKE_OCIO_VERSION_HEX(2, 3, 0)
Timer timer;
if (auto n = IdentifyBuiltinColorSpace("srgb_tx")) {
if (CSInfo* cs = find(n)) {
cs->setflag(CSInfo::is_srgb, srgb_alias);
DBG("Identified {} = builtin '{}'\n", "srgb", cs->name);
}
} else {
DBG("No config space identified as srgb\n");
}
DBG("identify_builtin_equivalents srgb took {:0.2f}s\n", timer.lap());
if (auto n = IdentifyBuiltinColorSpace("lin_srgb")) {
if (CSInfo* cs = find(n)) {
cs->setflag(CSInfo::is_lin_srgb | CSInfo::is_linear_response,
lin_srgb_alias);
DBG("Identified {} = builtin '{}'\n", "lin_srgb", cs->name);
}
} else {
DBG("No config space identified as lin_srgb\n");
}
DBG("identify_builtin_equivalents lin_srgb took {:0.2f}s\n", timer.lap());
if (auto n = IdentifyBuiltinColorSpace("ACEScg")) {
if (CSInfo* cs = find(n)) {
cs->setflag(CSInfo::is_ACEScg | CSInfo::is_linear_response,
ACEScg_alias);
DBG("Identified {} = builtin '{}'\n", "ACEScg", cs->name);
}
} else {
DBG("No config space identified as acescg\n");
}
DBG("identify_builtin_equivalents acescg took {:0.2f}s\n", timer.lap());
#endif
}

Expand Down Expand Up @@ -798,23 +836,26 @@ ColorConfig::Impl::init(string_view filename)

ok = config_.get() != nullptr;

if (timer() > 0.1f)
DBG("OCIO config {} loaded in {:0.2f} seconds\n", filename,
timer.lap());
DBG("OCIO config {} loaded in {:0.2f} seconds\n", filename, timer.lap());
#endif

inventory();
identify_builtin_equivalents();
// NOTE: inventory already does classify_by_name

#if OCIO_VERSION_HEX < MAKE_OCIO_VERSION_HEX(2, 2, 0)
// Prior to 2.2, there are some other heuristics we use
for (auto&& cs : colorspaces)
reclassify_heuristics(cs);
#endif
if (timer() > 0.1f)
DBG("OCIO config classified in {:0.2f} seconds\n", timer.lap());
#if OCIO_VERSION_HEX >= MAKE_OCIO_VERSION_HEX(2, 3, 0)
DBG("\nIDENTIFY BUILTIN EQUIVALENTS\n");
identify_builtin_equivalents(); // OCIO 2.3+ only
DBG("OCIO 2.3+ builtin equivalents in {:0.2f} seconds\n", timer.lap());
#endif

#if 1
for (auto&& cs : colorspaces) {
// examine(&cs);
DBG("Color space '{}':\n", cs.name);
if (cs.flags() & CSInfo::is_srgb)
DBG("'{}' is srgb\n", cs.name);
Expand All @@ -832,9 +873,9 @@ ColorConfig::Impl::init(string_view filename)
DBG("\n");
}
#endif
DBG("Aliases: scene_linear={} lin_srgb={} srgb={} ACEScg={} Rec709={}\n",
scene_linear_alias, lin_srgb_alias, srgb_alias, ACEScg_alias,
Rec709_alias);
debug_print_aliases();
DBG("OCIO config {} classified in {:0.2f} seconds\n", filename,
timer.lap());

return ok;
}
Expand All @@ -845,7 +886,10 @@ bool
ColorConfig::reset(string_view filename)
{
pvt::LoggedTimer logtime("ColorConfig::reset");
if (m_impl && filename == getImpl()->configname()) {
if (m_impl
&& (filename == getImpl()->configname()
|| (filename == ""
&& getImpl()->configname() == "ocio://default"))) {
// Request to reset to the config we're already using. Just return,
// don't do anything expensive.
return true;
Expand Down Expand Up @@ -1063,8 +1107,8 @@ ColorConfig::getColorSpaceNameByRole(string_view role) const
using Strutil::print;
OCIO::ConstColorSpaceRcPtr c = getImpl()->config_->getColorSpace(
std::string(role).c_str());
// print("looking first for named color space {} -> {}\n", role,
// c ? c->getName() : "not found");
// DBG("looking first for named color space {} -> {}\n", role,
// c ? c->getName() : "not found");
// Catch special case of obvious name synonyms
if (!c
&& (Strutil::iequals(role, "RGB")
Expand Down
19 changes: 10 additions & 9 deletions src/oiiotool/oiiotool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5381,30 +5381,31 @@ input_file(Oiiotool& ot, cspan<const char*> argv)
std::string colorspace(
ot.colorconfig.getColorSpaceFromFilepath(filename));
if (colorspace.size() && ot.debug)
std::cout << " From " << filename
<< ", we deduce color space \"" << colorspace
<< "\"\n";
print(" From {}, we deduce color space \"{}\"\n", filename,
colorspace);
if (colorspace.empty()) {
ot.read();
colorspace = ot.curimg->spec()->get_string_attribute(
"oiio:ColorSpace");
if (ot.debug)
std::cout << " Metadata of " << filename
<< " indicates color space \"" << colorspace
<< "\"\n";
print(" Metadata of {} indicates color space \"{}\"\n",
colorspace, filename);
}
std::string linearspace = ot.colorconfig.resolve("linear");
if (colorspace.size()
&& ot.colorconfig.equivalent(colorspace, linearspace)) {
&& !ot.colorconfig.equivalent(colorspace, linearspace)) {
std::string cmd = "colorconvert:strict=0";
if (autoccunpremult)
cmd += ":unpremult=1";
const char* argv[] = { cmd.c_str(), colorspace.c_str(),
linearspace.c_str() };
if (ot.debug)
std::cout << " Converting " << filename << " from "
<< colorspace << " to " << linearspace << "\n";
print(" Converting {} from {} to {}\n", filename,
colorspace, linearspace);
action_colorconvert(ot, argv);
} else if (ot.debug) {
print(" no auto conversion necessary for {}->{}\n", colorspace,
linearspace);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/raw.imageio/rawinput.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -662,8 +662,8 @@ RawInput::open_raw(bool unpack, const std::string& name,
m_spec.attribute("raw:FilterPattern", filter);

// Also, any previously set demosaicing options are void, so remove them
m_spec.erase_attribute("oiio:Colorspace");
m_spec.erase_attribute("raw:Colorspace");
m_spec.erase_attribute("oiio:ColorSpace");
m_spec.erase_attribute("raw:ColorSpace");
m_spec.erase_attribute("raw:Exposure");
} else {
errorf("raw:Demosaic set to unknown value");
Expand Down
2 changes: 1 addition & 1 deletion src/term.imageio/termoutput.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ bool
TermOutput::output()
{
// Color convert in place to sRGB, or it won't look right
std::string cspace = m_buf.spec()["oiio:colorspace"].get();
std::string cspace = m_buf.spec()["oiio:ColorSpace"].get();
ImageBufAlgo::colorconvert(m_buf, m_buf, cspace, "sRGB");

string_view method(m_method);
Expand Down

0 comments on commit b8723ec

Please sign in to comment.