From 5ce78b6fbfd036dc1a11322d2189bdae233df446 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Sat, 18 May 2024 16:06:36 -0700 Subject: [PATCH] cleanup: change raw cerr stream output to print (#4258) A small piece of the ongoing switch to std::format/print style text output. This part is for error and debugging output when reading/writing exr images. I also switched conditional compilation to the DBG macro idiom that I've used elsewhere, where it takes a combination of DEBUG compile and setting of environment variable OIIO_DEBUG_OPENEXR to trigger the printf style debugging (the first use case of this was in our color management code, which I also modify slightly here to conform). Also opportunistically simplify gcd declaration now that we're C++17 minimum. Signed-off-by: Larry Gritz --- src/libOpenImageIO/color_ocio.cpp | 3 +- src/openexr.imageio/exr_pvt.h | 21 +++++++++- src/openexr.imageio/exrinput.cpp | 14 +++---- src/openexr.imageio/exrinput_c.cpp | 65 +++++++++++------------------- 4 files changed, 50 insertions(+), 53 deletions(-) diff --git a/src/libOpenImageIO/color_ocio.cpp b/src/libOpenImageIO/color_ocio.cpp index 9d3a44efda..4373ceb43d 100644 --- a/src/libOpenImageIO/color_ocio.cpp +++ b/src/libOpenImageIO/color_ocio.cpp @@ -50,7 +50,8 @@ static const Imath::C3f test_colors[n_test_colors] #if 0 || !defined(NDEBUG) /* allow color configuration debugging */ -static bool colordebug = Strutil::stoi(Sysutil::getenv("OIIO_COLOR_DEBUG")); +static bool colordebug = Strutil::stoi(Sysutil::getenv("OIIO_DEBUG_COLOR")) + || Strutil::stoi(Sysutil::getenv("OIIO_DEBUG_ALL")); # define DBG(...) \ if (colordebug) \ Strutil::print(__VA_ARGS__) diff --git a/src/openexr.imageio/exr_pvt.h b/src/openexr.imageio/exr_pvt.h index df74c21acd..4e18585c7b 100644 --- a/src/openexr.imageio/exr_pvt.h +++ b/src/openexr.imageio/exr_pvt.h @@ -10,6 +10,8 @@ #include #include #include +#include +#include #include #include @@ -33,11 +35,25 @@ # define OPENEXR_HAS_FLOATVECTOR 0 #endif -#define ENABLE_READ_DEBUG_PRINTS 0 - +#define ENABLE_EXR_DEBUG_PRINTS 0 OIIO_PLUGIN_NAMESPACE_BEGIN +// Lots of debugging printf turned on for DEBUG builds or if you define +// ENABLE_EXR_DEBUG_PRINTS above, *AND* the "OIIO_DEBUG_OPENEXR" environment +// variable is set to something numerically non-zero. +#if ENABLE_EXR_DEBUG_PRINTS || !defined(NDEBUG) /* allow debugging */ +static bool exrdebug = Strutil::stoi(Sysutil::getenv("OIIO_DEBUG_OPENEXR")) + || Strutil::stoi(Sysutil::getenv("OIIO_DEBUG_ALL")); +# define DBGEXR(...) \ + if (exrdebug) \ + Strutil::print(__VA_ARGS__) +#else +# define DBGEXR(...) +#endif + + + #if OIIO_CPLUSPLUS_VERSION >= 17 || defined(__cpp_lib_gcd_lcm) using std::gcd; #else @@ -55,6 +71,7 @@ gcd(M a, N b) #endif + // Split a full channel name into layer and suffix. inline void split_name(string_view fullname, string_view& layer, string_view& suffix) diff --git a/src/openexr.imageio/exrinput.cpp b/src/openexr.imageio/exrinput.cpp index 7c2eb343a3..37c20b7163 100644 --- a/src/openexr.imageio/exrinput.cpp +++ b/src/openexr.imageio/exrinput.cpp @@ -598,7 +598,8 @@ OpenEXRInput::PartInfo::parse_header(OpenEXRInput* in, } } else { #if 0 - std::cerr << " unknown attribute " << type << ' ' << name << "\n"; + print(std::cerr, " unknown attribute '{}' name '{}'\n", + type, name); #endif } } @@ -1128,8 +1129,8 @@ OpenEXRInput::read_native_scanlines(int subimage, int miplevel, int ybegin, if (!seek_subimage(subimage, miplevel)) return false; chend = clamp(chend, chbegin + 1, m_spec.nchannels); - // std::cerr << "openexr rns " << ybegin << ' ' << yend << ", channels " - // << chbegin << "-" << (chend-1) << "\n"; + DBGEXR("openexr rns {} {}-{}, channels {}-{}", ybegin, yend, chbegin, + chend - 1); // Compute where OpenEXR needs to think the full buffers starts. // OpenImageIO requires that 'data' points to where the client wants @@ -1258,11 +1259,8 @@ OpenEXRInput::read_native_tiles(int subimage, int miplevel, int xbegin, "OpenEXRInput::read_native_tiles is not supported for luminance-chroma images"); return false; } -#if 0 - std::cerr << "openexr rnt " << xbegin << ' ' << xend << ' ' << ybegin - << ' ' << yend << ", chans " << chbegin - << "-" << (chend-1) << "\n"; -#endif + DBGEXR("openexr rnt {} {}-{}, chans {}-{}", xbegin, xend, ybegin, yend, + chend - 1); if (!m_tiled_input_part || !m_spec.valid_tile_range(xbegin, xend, ybegin, yend, zbegin, zend)) { errorfmt("called OpenEXRInput::read_native_tiles without an open file"); diff --git a/src/openexr.imageio/exrinput_c.cpp b/src/openexr.imageio/exrinput_c.cpp index a073323262..e8fc49dbc9 100644 --- a/src/openexr.imageio/exrinput_c.cpp +++ b/src/openexr.imageio/exrinput_c.cpp @@ -57,9 +57,8 @@ oiio_exr_error_handler(exr_const_context_t ctxt, exr_result_t code, } // this should only happen from valid_file check, do we care? - //std::cerr << "EXR error with no valid context (" - // << exr_get_error_code_as_string(code) << "): " << msg - // << std::endl; + // print(std::cerr, "EXR error with no valid context ({}): {}\n", + // exr_get_error_code_as_string(code), msg); } static int64_t @@ -418,8 +417,9 @@ OpenEXRCoreInput::open(const std::string& name, ImageSpec& newspec, m_userdata.m_io = nullptr; return false; } -#if ENABLE_READ_DEBUG_PRINTS - exr_print_context_info(m_exr_context, 1); +#if ENABLE_EXR_DEBUG_PRINTS || !defined(NDEBUG) /* allow debugging */ + if (exrdebug) + exr_print_context_info(m_exr_context, 1); #endif rv = exr_get_count(m_exr_context, &m_nsubimages); if (rv != EXR_ERR_SUCCESS) { @@ -733,7 +733,8 @@ OpenEXRCoreInput::PartInfo::parse_header(OpenEXRCoreInput* in, case EXR_ATTR_TILEDESC: default: #if 0 - std::cerr << " unknown attribute type '" << attr->type_name << "' in name: '" << attr->name << "'" << std::endl; + print(std::cerr, " unknown attribute type '{}' in name '{}'\n", + attr->type_name, attr->name); #endif break; } @@ -1181,16 +1182,10 @@ OpenEXRCoreInput::read_native_scanlines(int subimage, int miplevel, int ybegin, if (rv != EXR_ERR_SUCCESS) return false; -#if ENABLE_READ_DEBUG_PRINTS - { - lock_guard lock(*this); - std::cerr << "exr rns " << m_userdata.m_io->filename() << ":" - << subimage << ":" << miplevel << " scans (" << ybegin << '-' - << yend << "|" << (yend - ybegin) << ")[" << chbegin << "-" - << (chend - 1) << "] -> pb " << pixelbytes << " sb " - << scanlinebytes << " spc " << scansperchunk << std::endl; - } -#endif + DBGEXR("exr rns {}:{}:{} scans ({}-{}|{}}[{}-{}] -> pb {} sb {} spc {}\n", + m_userdata.m_io->filename(), subimage, miplevel, ybegin, yend, + yend - ybegin, chbegin, chend - 1, pixelbytes, scanlinebytes, + scansperchunk); int endy = spec.y + spec.height; yend = std::min(endy, yend); int ychunkstart = spec.y @@ -1358,13 +1353,10 @@ OpenEXRCoreInput::read_native_tile(int subimage, int miplevel, int x, int y, scanlinebytes); } -#if ENABLE_READ_DEBUG_PRINTS - std::cerr << "openexr rnt single " << m_userdata.m_io->filename() << " si " - << subimage << " mip " << miplevel << " pos " << x << ' ' << y - << "\n -> tile " << tx << ", " << ty << ", pixbytes " - << pixelbytes << " scan " << scanlinebytes << " tilesz " << tilew - << "x" << tileh << std::endl; -#endif + DBGEXR( + "openexr rnt single {} si {} mip {} pos {} {} -> tile {} {} pixbytes {} scan {} tilesz {}x{}\n", + m_userdata.m_io->filename(), subimage, miplevel, x, y, tx, ty, + pixelbytes, scanlinebytes, tilew, tileh); uint8_t* cdata = static_cast(data); size_t chanoffset = 0; @@ -1379,12 +1371,9 @@ OpenEXRCoreInput::read_native_tile(int subimage, int miplevel, int x, int y, curchan.user_line_stride = scanlinebytes; //curchan.width * pixelbytes; chanoffset += chanbytes; -#if ENABLE_READ_DEBUG_PRINTS - std::cerr << " chan " << c << " tile " << tx << ", " << ty - << ": linestride " << curchan.user_line_stride - << " tilesize " << curchan.width << " x " - << curchan.height << std::endl; -#endif + DBGEXR(" chan {} tile {}, {}: linestride {} tilesize {} x {}\n", + c, tx, ty, curchan.user_line_stride, curchan.width, + curchan.height); break; } } @@ -1465,19 +1454,11 @@ OpenEXRCoreInput::read_native_tiles(int subimage, int miplevel, int xbegin, size_t scanlinebytes = size_t(nxtiles) * size_t(tilew) * pixelbytes; -#if ENABLE_READ_DEBUG_PRINTS - { - lock_guard lock(*this); - std::cerr << "exr rnt " << m_userdata.m_io->filename() << ":" - << subimage << ":" << miplevel << " (" << xbegin << ' ' - << xend << ' ' << ybegin << ' ' << yend << "|" - << (xend - xbegin) << "x" << (yend - ybegin) << ")[" - << chbegin << "-" << (chend - 1) << "] -> t " << firstxtile - << ", " << firstytile << " n " << nxtiles << ", " << nytiles - << " pb " << pixelbytes << " sb " << scanlinebytes << " tsz " - << tilew << "x" << tileh << std::endl; - } -#endif + DBGEXR( + "exr rnt {}:{}:{} ({}-{}|{}x{})[{}-{}] -> t {}, {} n {}, {} pb {} sb {} tsz {}x{}\n", + m_userdata.m_io->filename(), subimage, miplevel, xbegin, xend, + xend - xbegin, ybegin, yend, chbegin, chend - 1, firstxtile, firstytile, + nxtiles, nytiles, pixelbytes, scanlinebytes, tilew, tileh); std::atomic ok(true); parallel_for_2D(