From 49499605dce1772297fb3209b477e6f579f1ba2b Mon Sep 17 00:00:00 2001 From: Doug Walker Date: Sun, 10 Nov 2024 20:04:23 -0500 Subject: [PATCH] Fix bit-depth attr for inv luts for CTF writing Signed-off-by: Doug Walker --- .../fileformats/ctf/CTFTransform.cpp | 66 +++++++-- tests/cpu/fileformats/FileFormatCTF_tests.cpp | 135 ++++++++++++++++-- 2 files changed, 180 insertions(+), 21 deletions(-) diff --git a/src/OpenColorIO/fileformats/ctf/CTFTransform.cpp b/src/OpenColorIO/fileformats/ctf/CTFTransform.cpp index 4e0f18b3a..927f2c2c2 100644 --- a/src/OpenColorIO/fileformats/ctf/CTFTransform.cpp +++ b/src/OpenColorIO/fileformats/ctf/CTFTransform.cpp @@ -736,7 +736,7 @@ const char * BitDepthToCLFString(BitDepth bitDepth) else if (bitDepth == BIT_DEPTH_F16) return "16f"; else if (bitDepth == BIT_DEPTH_F32) return "32f"; - throw Exception("Bitdepth has been validated before calling this."); + throw Exception("Bitdepth has not been validated before calling this."); } BitDepth GetValidatedFileBitDepth(BitDepth bd, OpData::Type type) @@ -2125,7 +2125,9 @@ void Lut1DWriter::writeContent() const // To avoid needing to duplicate the const objects, // we scale the values on-the-fly while writing. - const float scale = (float)GetBitDepthMaxValue(m_outBitDepth); + const BitDepth arrayBitDepth = m_lut->getDirection() == TRANSFORM_DIR_INVERSE ? + m_inBitDepth : m_outBitDepth; + const float scale = (float) GetBitDepthMaxValue(arrayBitDepth); if (m_lut->isOutputRawHalfs()) { @@ -2155,7 +2157,7 @@ void Lut1DWriter::writeContent() const values.begin(), values.end(), array.getNumColorComponents(), - m_outBitDepth, + arrayBitDepth, array.getNumColorComponents() == 1 ? 3 : 1, scale); } @@ -2249,12 +2251,15 @@ void Lut3DWriter::writeContent() const // To avoid needing to duplicate the const objects, // we scale the values on-the-fly while writing. - const float scale = (float)GetBitDepthMaxValue(m_outBitDepth); + const BitDepth arrayBitDepth = m_lut->getDirection() == TRANSFORM_DIR_INVERSE ? + m_inBitDepth : m_outBitDepth; + const float scale = (float) GetBitDepthMaxValue(arrayBitDepth); + WriteValues(m_formatter, array.getValues().begin(), array.getValues().end(), 3, - m_outBitDepth, + arrayBitDepth, 1, scale); @@ -2647,6 +2652,28 @@ BitDepth GetInputFileBD(ConstOpDataRcPtr op) const auto bd = range->getFileInputBitDepth(); return GetValidatedFileBitDepth(bd, type); } + else if (type == OpData::Lut1DType) + { + // For an InverseLut1D, the file "out" depth, which determines the array scaling, + // must actually be set as the in-depth. + auto lut = OCIO_DYNAMIC_POINTER_CAST(op); + if (lut->getDirection() == TRANSFORM_DIR_INVERSE) + { + const auto bd = lut->getFileOutputBitDepth(); + return GetValidatedFileBitDepth(bd, type); + } + } + else if (type == OpData::Lut3DType) + { + // For an InverseLut3D, the file "out" depth, which determines the array scaling, + // must actually be set as the in-depth. + auto lut = OCIO_DYNAMIC_POINTER_CAST(op); + if (lut->getDirection() == TRANSFORM_DIR_INVERSE) + { + const auto bd = lut->getFileOutputBitDepth(); + return GetValidatedFileBitDepth(bd, type); + } + } return BIT_DEPTH_F32; } @@ -2654,6 +2681,9 @@ BitDepth GetInputFileBD(ConstOpDataRcPtr op) void TransformWriter::writeOps(const CTFVersion & version) const { + // If the ops have a specific file input/output bit-depth that was set (e.g., if the + // ops were originally read in from a CTF/CLF file), then we try to preserve those + // values on write. Otherwise, default to 32f. BitDepth inBD = BIT_DEPTH_F32; BitDepth outBD = BIT_DEPTH_F32; @@ -2662,15 +2692,27 @@ void TransformWriter::writeOps(const CTFVersion & version) const size_t numSavedOps = 0; if (numOps) { + // Initialize the input bit-depth from the first op. Thereafter, the input depth + // will be determined from the output depth of the previous op that was written. inBD = GetInputFileBD(ops[0]); + for (size_t i = 0; i < numOps; ++i) { auto & op = ops[i]; + // Each op is allowed to set its preferred output depth, so that is always + // respected. However, we try to honour preferred input depth too, where possible. + // If the next op has a preferred file input bit-depth, use that as the default + // out-depth for the current op. However, if the current op has a preferred + // file out depth, that will take precedence below. If an op's preferred file + // depth cannot be honoured, it still results in a perfectly valid CTF file, + // it's only that the scaling of array values may be adjusted relative to the + // original CTF file it was created from (if any). + // See FileFormatCTF_tests.cpp test bitdepth_ctf. if (i + 1 < numOps) { auto & nextOp = ops[i + 1]; - // Return file input bit-depth for Matrix & Range, F32 for others. + // Return file input bit-depth for ops with a preference, F32 for others. outBD = GetInputFileBD(nextOp); } @@ -2841,7 +2883,11 @@ void TransformWriter::writeOps(const CTFVersion & version) const // Avoid copying LUT, write will take bit-depth into account. Lut1DWriter opWriter(m_formatter, lut); - outBD = GetValidatedFileBitDepth(lut->getFileOutputBitDepth(), type); + if (lut->getDirection() == TRANSFORM_DIR_FORWARD) + { + // For an inverse Lut1D, the fileOutDepth is used for the inBitDepth above. + outBD = GetValidatedFileBitDepth(lut->getFileOutputBitDepth(), type); + } opWriter.setInputBitdepth(inBD); opWriter.setOutputBitdepth(outBD); @@ -2861,7 +2907,11 @@ void TransformWriter::writeOps(const CTFVersion & version) const // Avoid copying LUT, write will take bit-depth into account. Lut3DWriter opWriter(m_formatter, lut); - outBD = GetValidatedFileBitDepth(lut->getFileOutputBitDepth(), type); + if (lut->getDirection() == TRANSFORM_DIR_FORWARD) + { + // For an inverse Lut3D, the fileOutDepth is used for the inBitDepth above. + outBD = GetValidatedFileBitDepth(lut->getFileOutputBitDepth(), type); + } opWriter.setInputBitdepth(inBD); opWriter.setOutputBitdepth(outBD); diff --git a/tests/cpu/fileformats/FileFormatCTF_tests.cpp b/tests/cpu/fileformats/FileFormatCTF_tests.cpp index 0635c4a0c..2f00c8416 100644 --- a/tests/cpu/fileformats/FileFormatCTF_tests.cpp +++ b/tests/cpu/fileformats/FileFormatCTF_tests.cpp @@ -955,6 +955,41 @@ OCIO_ADD_TEST(FileFormatCTF, lut1d_inv) OCIO_CHECK_CLOSE(a2.getValues()[50], 1.0f, error); } +OCIO_ADD_TEST(FileFormatCTF, lut1d_inv_scaling) +{ + // Validate that the InverseLUT1D array values are scaled based on inBitDepth. + // (The previous example had inBitDepth=32f, so it does not validate that.) + + OCIO::LocalCachedFileRcPtr cachedFile; + const std::string ctfFile("lut1d_inverse_halfdom_slog_fclut.ctf"); + OCIO_CHECK_NO_THROW(cachedFile = LoadCLFFile(ctfFile)); + OCIO_REQUIRE_ASSERT((bool)cachedFile); + + const OCIO::ConstOpDataVec & opList = cachedFile->m_transform->getOps(); + OCIO_REQUIRE_EQUAL(opList.size(), 1); + + auto pLut = std::dynamic_pointer_cast(opList[0]); + OCIO_REQUIRE_ASSERT(pLut); + // For an InverseLUT1D, the file "out" depth is actually taken from inBitDepth. + OCIO_CHECK_EQUAL(pLut->getFileOutputBitDepth(), OCIO::BIT_DEPTH_UINT16); + OCIO_CHECK_EQUAL(pLut->getDirection(), OCIO::TRANSFORM_DIR_INVERSE); + + const OCIO::Array & a2 = pLut->getArray(); + OCIO_CHECK_EQUAL(a2.getNumColorComponents(), 1); + + OCIO_CHECK_EQUAL(a2.getLength(), 65536); + OCIO_CHECK_EQUAL(a2.getNumValues(), + a2.getLength()*a2.getMaxColorComponents()); + + const float error = 1e-6f; + OCIO_REQUIRE_EQUAL(a2.getValues().size(), a2.getNumValues()); + + // Input value 17830 scaled by 65535. + OCIO_CHECK_CLOSE(a2.getValues()[0], 0.27206836f, error); + // Input value 55070 scaled by 65535. + OCIO_CHECK_CLOSE(a2.getValues()[31743 * 3], 0.84031434f, error); +} + namespace { OCIO::LocalCachedFileRcPtr ParseString(const std::string & str) @@ -1056,7 +1091,9 @@ OCIO_ADD_TEST(FileFormatCTF, lut3d_inv) auto pLut = std::dynamic_pointer_cast(opList[0]); OCIO_REQUIRE_ASSERT(pLut); + // For an InverseLUT3D, the file "out" depth is set by the inBitDepth of the file. OCIO_CHECK_EQUAL(pLut->getFileOutputBitDepth(), OCIO::BIT_DEPTH_UINT12); + OCIO_CHECK_EQUAL(pLut->getInterpolation(), OCIO::INTERP_TETRAHEDRAL); OCIO_CHECK_EQUAL(pLut->getDirection(), OCIO::TRANSFORM_DIR_INVERSE); @@ -1067,6 +1104,7 @@ OCIO_ADD_TEST(FileFormatCTF, lut3d_inv) *array.getLength()*array.getMaxColorComponents()); OCIO_REQUIRE_EQUAL(array.getValues().size(), array.getNumValues()); + // Validate that the array was scaled by the inBitDepth of the file. OCIO_CHECK_EQUAL(array.getLength(), 17); OCIO_CHECK_CLOSE(array.getValues()[0], 25.0f / 4095.0f, 1e-8f); OCIO_CHECK_CLOSE(array.getValues()[1], 30.0f / 4095.0f, 1e-8f); @@ -7644,9 +7682,12 @@ OCIO_ADD_TEST(CTFTransform, lut1d_inverse_ctf) OCIO_CHECK_NO_THROW(WriteGroupCTF(group, outputTransform)); // Note the type of the node. + // + // For an InverseLUT1D, the scaling of array values is based on the inBitDepth. + // const std::string expected{ R"( - + 0 1 2 3 4 5 @@ -7812,9 +7853,12 @@ OCIO_ADD_TEST(CTFTransform, lut3d_inverse_ctf) OCIO_CHECK_NO_THROW(WriteGroupCTF(group, outputTransform)); // Note the type of the node. + // + // For an InverseLUT3D, the scaling of array values is based on the inBitDepth. + // const std::string expected{ R"( - + 0 1 2 3 4 5 @@ -7946,39 +7990,80 @@ OCIO_ADD_TEST(CTFTransform, bitdepth_ctf) auto range = OCIO::RangeTransform::Create(); range->setFileInputBitDepth(OCIO::BIT_DEPTH_F16); range->setFileOutputBitDepth(OCIO::BIT_DEPTH_UINT12); - range->setMinInValue(0.); - range->setMinOutValue(0.); + range->setMinInValue(0.1); + range->setMinOutValue(-0.1); + range->setMaxInValue(0.9); + range->setMaxOutValue(1.1); + + auto log = OCIO::LogTransform::Create(); + + auto invlut = OCIO::Lut1DTransform::Create(); + invlut->setDirection(OCIO::TRANSFORM_DIR_INVERSE); + invlut->setFileOutputBitDepth(OCIO::BIT_DEPTH_UINT16); + invlut->setLength(3); auto mat2 = OCIO::MatrixTransform::Create(); mat2->setFileInputBitDepth(OCIO::BIT_DEPTH_UINT8); mat2->setFileOutputBitDepth(OCIO::BIT_DEPTH_UINT10); - - auto log = OCIO::LogTransform::Create(); + mat2->setDirection(OCIO::TRANSFORM_DIR_INVERSE); OCIO::GroupTransformRcPtr group = OCIO::GroupTransform::Create(); group->getFormatMetadata().addAttribute(OCIO::METADATA_ID, "UID42"); - // First op keeps bit-depth + // Transforms are setup as follows: + // + // Matrix fileIn = 8i, fileOut = 10i + // Lut1D fileOut = 10i + // Exponent + // Range fileIn = 16f, fileOut = 12i + // Matrix fileIn = 8i, fileOut = 10i + // Log + // InvLut1D fileOut = 16i + // InvMatrix fileIn = 8i, fileOut = 10i + // InvLut1D fileOut = 16i + + // First op keeps its in & out bit-depth. + // group->appendTransform(mat); // Previous op out bit-depth used for in bit-depth. + // group->appendTransform(lut); // Previous op out bit-depth used for in bit-depth. // And next op (range) in bit-depth used for out bit-depth. + // group->appendTransform(exp); // In bit-depth preserved and has been used for out bit-depth of previous op. // Next op is a matrix, but current op is range, first op out bit-depth - // is preserved and used for next op in bit-depth. + // is preserved and overrides the next op's in in bit-depth. + // group->appendTransform(range); // Previous op out bit-depth used for in bit-depth. - group->appendTransform(mat2); + // + group->appendTransform(mat); // 2); - // Previous op out bit-depth used for in bit-depth. + // Previous op out bit-depth used for in bit-depth. Out depth is set by preference + // of the next op. + // group->appendTransform(log); + // Preferred in bit-depth is preserved. Out depth is set by the next op. + // + group->appendTransform(invlut); + + // Sets both its preferred in and out depth. Note that because the transform + // direction is inverse, the mapping of file depths is swapped. + // + group->appendTransform(mat2); + + // This time it doesn't get its preferred in depth, since the previous op has priority. + // The array values are scaled accordingly. + // + group->appendTransform(invlut); + std::ostringstream outputTransform; OCIO_CHECK_NO_THROW(WriteGroupCTF(group, outputTransform)); @@ -8002,8 +8087,10 @@ OCIO_ADD_TEST(CTFTransform, bitdepth_ctf) - 0 - 0 + 0.1 + 0.9 + -409.5 + 4504.5 @@ -8012,10 +8099,32 @@ OCIO_ADD_TEST(CTFTransform, bitdepth_ctf) 0 0 0.24981684981685 - + + + + 0 +32767.5 + 65535 + + + + + 0.249266862170088 0 0 + 0 0.249266862170088 0 + 0 0 0.249266862170088 + + + + + 0 +127.5 + 255 + + )" }; + OCIO_CHECK_EQUAL(expected.size(), outputTransform.str().size()); OCIO_CHECK_EQUAL(expected, outputTransform.str()); }