feat(dng): decode JPEG XL DNGs through a host-registered decoder - #4
Draft
puzza007 wants to merge 3 commits into
Draft
feat(dng): decode JPEG XL DNGs through a host-registered decoder#4puzza007 wants to merge 3 commits into
puzza007 wants to merge 3 commits into
Conversation
LibRaw has no JPEG XL decoder of its own - upstream's jxl_dng_load_raw is a placeholder that throws, with a comment pointing at the Adobe DNG SDK as the only route. That leaves every DNG 1.7 file (Lightroom lossy DNG, Adobe Enhance output, DNG Converter -jxl) unpackable. Rather than pull in the DNG SDK or libjxl, delegate: libraw_set_jxl_decoder() registers a decoder the host supplies, and jxl_dng_load_raw walks the segment table and hands each one over. With nothing registered it throws LIBRAW_EXCEPTION_UNSUPPORTED_FORMAT exactly as the placeholder did, so a build that never calls the setter is unchanged. The walk mirrors lossy_dng_load_raw, with two differences. The samples are linear 16-bit, so there is no opcode-8 curve to apply. And the decoder takes whole buffers rather than a stream, so segment lengths come from TileByteCounts/StripByteCounts instead of being discovered while decoding.
Read the segment offset/length table once up front instead of two extra buffered reads per tile interleaved with the payload - get4() moves the shared stream cursor, so doing it per segment is both slower and the thing that would stop the loop ever being parallelised. A stripped image is the same grid one segment wide, so deriving the segment height from RowsPerStrip lets one formula cover both layouts: the `tiled` ternaries collapse from seven to two, and the per-segment branching goes away. That also fixes multi-strip placement, which previously wrote every strip at row 0. Report JPEG XL support from whether a decoder is registered rather than from USE_DNGSDK - the capability is a runtime question now, and get_decoder_info() was contradicting the decoder.
Both segment tables are contiguous, so seek once per table and let get4() walk them, rather than re-seeking to where the cursor already is before every 4-byte read - which is what the comment above the loops said they avoided. Fold the colors bounds into one guard at the top so the geometry check below is only about geometry, and stop carrying both `dc` and `colors` through the placement loops when the check above proves them equal. Move libraw_set_jxl_decoder and libraw_have_jxl_decoder into libraw_c_api.cpp with the other 58 libraw_* entry points, reaching the callback static through internal accessors that stay beside the decoder. That stops the C++ core calling up into the C API wrapper, and keeps an upstream rebase conflicting in the C API file rather than in a decoder. Document the registration's thread-safety contract in the header: the callback pointer is a plain static, so it has to be installed before the first concurrent open.
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.
Related Linear issues (reference only — not picked up for status): EPD-1426, EPD-1285, EPD-890, EPD-152.
Why
LibRaw has no JPEG XL decoder of its own.
jxl_dng_load_raw_placeholderthrows unconditionally, with a comment pointing at the Adobe DNG SDK as the only route. That leaves every DNG 1.7 file unpackable: Lightroom lossy DNG, Adobe Enhance output, DNG Converter-jxl, and the pre-edit intermediate proxies Maxwell generates for itself.Integrating the DNG SDK was attempted and abandoned (EPD-151). Linking C++ libjxl means another library in the vendor bundle.
What
Delegate instead.
libraw_set_jxl_decoder()registers a decoder the host supplies, andjxl_dng_load_rawwalks the segment table and hands each segment over. With nothing registered it throwsLIBRAW_EXCEPTION_UNSUPPORTED_FORMATexactly as the placeholder did, so a build that never calls the setter behaves as it does today — verified by running the same harness both ways.The walk mirrors
lossy_dng_load_rawnext door, with two differences:TileByteCounts/StripByteCountsrather than being discovered while decodingget_decoder_info()now reports JPEG XL support from whether a decoder is registered rather than fromUSE_DNGSDK, which would otherwise contradict the decoder.Verified
Decodes three JPEG XL DNGs that previously failed with
-2: Adobe's lossy and lossless output, and a Samsung DNG 1.7 whose JPEG XL image sits in IFD0 rather than a SubIFD. The rendered result is byte-identical to an independent Rust transcode route that was itself validated bit-exact against libjxl's owndjxl.Builds clean through
maxwell-vendor'simage-libs/build.shand passes its smoke test.Not in scope
Tiles decode sequentially, as they do in
lossy_dng_load_raw. Measurements suggest ~3.8x is available from per-segment parallelism on a 24 MP file; deferred until there is evidence of volume, and better done host-side where thread priority is controlled.Stack
🤖 Generated with Claude Code