Add compression context cache - #2578
Open
cary-ilm wants to merge 2 commits into
Open
Conversation
* Properly initialize decode pipeline struct in prep for adding new version Signed-off-by: Kimball Thurston <kdt3rd@gmail.com> * Add a new version of decode pipeline to add decompression context This will allow a couple of the compression types to avoid constantly creating a context object internally every chunk when the pipeline is re-used (which it is most times). Signed-off-by: Kimball Thurston <kdt3rd@gmail.com> * Enable zip to use new compression cache in decode pipe Adds some helper routines to avoid code duplication as well in enabling this Signed-off-by: Kimball Thurston <kdt3rd@gmail.com> * Enable htj2k to use new compression context cache in decode Signed-off-by: Kimball Thurston <kdt3rd@gmail.com> * remove dead code after prints were removed Signed-off-by: Kimball Thurston <kdt3rd@gmail.com> * Relax the struct size check to just assume v1 if not a new size Signed-off-by: Kimball Thurston <kdt3rd@gmail.com> * add new header file to bazel build Signed-off-by: Kimball Thurston <kdt3rd@gmail.com> --------- Signed-off-by: Kimball Thurston <kdt3rd@gmail.com> Co-authored-by: Cary Phillips <seabeepea@gmail.com>
The compression context cache added in AcademySoftwareFoundation#2440 reused a std::vector of CodestreamChannelInfo (cs_to_file_ch) across decode calls by storing it on the cached ht_context_cache. Each CodestreamChannelInfo has a "scratch" field that read_header() uses as a per-chunk duplicate file_index marker, expecting it to start at zero. Before AcademySoftwareFoundation#2440 this vector was always freshly constructed per call, so scratch was always initialized to 0. Once cached, read_header()'s own resize only initializes newly added elements, so scratch values set to 1 while processing one chunk persisted into the next chunk that reused the same vector at the same size. The next chunk's duplicate check then saw a stale scratch of 1, threw an exception that was caught and turned into EXR_ERR_CORRUPT_CHUNK, and decompression silently produced zero decoded bytes. This reproduced broadly in CI (Linux, Windows, macOS, FreeBSD) as failures in testCompression, testConversion, testCopyPixels, testExistingStreams, testRgba, testCRgba, testRgbaThreading, testSharedFrameBuffer, testTiledCompression, and testTiledRgba, all with the signature "Unable to decompress w 10/11 image data ... got 0". The fix keeps cs_to_file_ch as a local variable, constructed fresh on every call as it was before AcademySoftwareFoundation#2440, since it is cheap to build and its per-chunk scratch state must start clean. Only the genuinely expensive ojph::codestream is kept on the cached context, preserving the intended performance benefit of AcademySoftwareFoundation#2440 without the correctness regression. Verified by rebuilding OpenEXRTest and rerunning the ten previously failing tests listed above; all now pass. Assisted-by: GitHub Copilot CLI (model: Claude Sonnet 5) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
2 participants
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.
Substitute for #2440, merged prematurely.