Add GroupTransform::ParseFromBuffer to parse a LUT from memory#2326
Open
hmms wants to merge 2 commits into
Open
Add GroupTransform::ParseFromBuffer to parse a LUT from memory#2326hmms wants to merge 2 commits into
hmms wants to merge 2 commits into
Conversation
FileTransform's LUT readers can currently only be reached via a file path on disk (or a path resolved through a ConfigIOProxy). There was no way to parse the contents of a LUT file already held in memory. Add a static GroupTransform::ParseFromBuffer(buffer, bufferSize) method that reuses the existing ConfigIOProxy / FileTransform / Config::getProcessor pipeline instead of duplicating any LUT-parsing code: a small internal ConfigIOProxy serves the buffer as LUT data, and the existing file-format readers do the actual parsing. The file caches in FileTransform.cpp and PathUtils.cpp are keyed by filename/hash strings, and a buffer has no filename, so the buffer's own content is hashed and used as both the synthetic source string and the ConfigIOProxy fast-hash value. This keeps two different buffers from colliding in the global caches while still allowing a cache hit when the same buffer is parsed more than once. Adds unit tests covering a successful parse from two different LUT formats (SPI1D, CLF) in the same process, re-parsing the same buffer, and error handling for null/empty/malformed input. Fixes AcademySoftwareFoundation#2076 Signed-off-by: Muralidhar M Shenoy <shenoy.muralidhar.m@gmail.com>
Follow-up to the initial ParseFromBuffer implementation, addressing review feedback on the already-open PR: - Add GroupTransform.ParseFromBuffer Python binding (PyGroupTransform.cpp), with a test exercising it from Python (GroupTransformTest.py). - Document that the buffer is copied internally (only needs to remain valid for the call) and that parsed results are cached process-wide by content hash, releasable via ClearAllCaches(). - Prefix the synthetic cache filename with "ParseFromBuffer:" so it can never collide with a real file whose resolved path happens to match the bare content hash string. Signed-off-by: Muralidhar M Shenoy <shenoy.muralidhar.m@gmail.com>
edb66f4 to
da59b5c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
GroupTransform::ParseFromBuffer(const char * buffer, size_t bufferSize), a static factory that parses a LUT directly from an in-memory buffer rather than requiring a file on disk.Closes #2076
Implementation
Reuses the existing LUT-reading pipeline rather than duplicating parser code. A small internal
BufferConfigIOProxy(anonymous namespace insrc/OpenColorIO/transforms/GroupTransform.cpp) serves the buffer as LUT data throughConfig::CreateRaw()->createEditableCopy()->setConfigIOProxy(...), then aFileTransformis resolved throughConfig::getProcessor(), andprocessor->createGroupTransform()returns the result.Because OCIO's internal file caches are keyed by filename/hash strings and a buffer has no filename, the buffer's own content is hashed (
CacheIDHash, XXH3-128) and used to form a synthetic filename ("ParseFromBuffer:" + hash, namespaced so it can never collide with a real file's resolved path) and theConfigIOProxy::getFastLutFileHashvalue, so two different buffers can never collide in the cache, and re-parsing identical content is a cache hit. Null/empty buffers and unparseable content both throw a clearException.Caching behavior worth flagging explicitly: like OCIO's existing file cache, entries created via
ParseFromBufferare never evicted, only released viaClearAllCaches(). This is fine for the typical case (a bounded set of files/buffers), but an application feeding many distinct, dynamically-generated buffers through this API (e.g. procedural LUTs per-frame) will grow the cache unboundedly unless it callsClearAllCaches()itself. Happy to discuss whether this API should default to not caching, or take an opt-in/opt-out flag, if that's a concern, didn't want to guess at the right policy and expand scope without your input.Python bindings
Added
GroupTransform.ParseFromBufferto the Python bindings (src/bindings/python/transforms/PyGroupTransform.cpp), with a corresponding test (tests/python/GroupTransformTest.py::test_parse_from_buffer). Verified with a realOCIO_BUILD_PYTHON=ONbuild (Python 3.14, pybind11 3.0.4): module imports,ParseFromBufferis callable and correctly round-trips both text LUT content and produces the expectedLut1DTransform, raises on empty/malformed input, fulltests/pythonsuite (400 tests) passes with zero regressions.Test plan
OCIO_ADD_TEST(GroupTransform, parse_from_buffer)intests/cpu/transforms/GroupTransform_tests.cppcovering:GroupTransformTest.py::test_parse_from_bufferand the fulltests/pythonsuite (400 tests): all passing against a real Python build, not just compile-checked.Notes for maintainers
ParseFromBuffer(const char*, size_t)as a static factory onGroupTransform) is a reasonable judgment call but not the literal signature sketched in the issue, happy to adjust if you'd prefer something else.