feat(dng): decode JPEG XL DNGs through a host-registered decoder - #4
Draft
puzza007 wants to merge 14 commits into
Draft
feat(dng): decode JPEG XL DNGs through a host-registered decoder#4puzza007 wants to merge 14 commits into
puzza007 wants to merge 14 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.
The decoder wrote straight into image[] at output coordinates, which was wrong in three ways at once for the camera-derived DNG 1.7 files this hook exists to unlock. unpack() allocates rawdata.raw_image rather than image whenever filters is non-zero or colors is 1, so a CFA-mosaic JPEG XL DNG - the output of DNG Converter's -losslessJXL on a camera raw - hit the `if (!image)` guard and threw IO_CORRUPT for a valid file, while get_decoder_info() advertised it as supported. The `dc != colors` check rejected the same file a second time: a mosaic codestream carries one channel, not three. Writing at image[row * width + col] also ignored top_margin/left_margin, and a decoder declaring neither ADOBECOPYPIXEL nor LEGACY_WITH_MARGINS makes unpack() zero the margins afterwards - so any file with an ActiveArea was decoded shifted, showing masked pixels at the top left and dropping the last live rows and columns. Files written with phint 34892 essentially never carry margins, which is why the shape inherited from lossy_dng_load_raw held up there; DNG 1.7 files converted from camera raws do carry them. And the samples never passed through curve[], so a LinearizationTable was left unapplied. That is not hypothetical: DNG Converter 18.6 -losslessJXL on lossy-compressed NEFs emits compression-52546 files with a non-identity 772-entry table, and Apple ProRAW-style DNGs carry one too. adobe_copy_pixel() is the existing answer to all three - it dispatches on raw_image, works in raw-sensor coordinates, and applies the curve - so use it and declare the flags that go with it, as lossless_dng_load_raw() does. The sample-count guard moves to tiff_samples, which is what it actually consumes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2DjC7aaBNbYKFVPjc17on
`maximum = 0xffff` was copied over from lossy_dng_load_raw() without the thing that makes it true there. identify() special-cases that decoder and resets dng_whitelevel and linear_max to 0xffff alongside it, because 34892 data really is normalised to full range. For every other DNG decoder it takes maximum from the file's WhiteLevel and leaves black, cblack and linear_max in file scale, and lossless_dng_load_raw() correspondingly does not touch maximum. Stamping it here discarded the WhiteLevel while those other levels kept the file's scale, so nothing agreed. A 14-bit-in-16-bit-container JPEG XL DNG (WhiteLevel 16383) renders about four stops dark: scale_colors divides by 65535 - black, and adjust_maximum only steps in above 0.75 * maximum. Only files that genuinely are full-range came out right. The callback contract says nothing about sample scaling either, so the host cannot be expected to rescale - and if it did, black and linear_max would then be four times too small instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2DjC7aaBNbYKFVPjc17on
identify() assigns jxl_dng_load_raw for compression 52546 unconditionally, where compression 8 routes on tiff_sampleformat and bps. So a DNG 1.7 file with SampleFormat 3 - a half-float HDR frame, a legal shape - reached an integer decoder. None of identify()'s three float rejects fire, because they only test bps > 16; tiff.cpp's maximum = 1 for float data is then overwritten with the 0xffff default; and the callback is handed no sample-format signal, so the half-float codestream would be emitted as whatever the host makes of it and reported as a successful unpack with meaningless tone. The placeholder this hook replaced returned UNSUPPORTED_FORMAT for that shape. Keep doing so, using the same is_floating_point() the deflate path uses to spot it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2DjC7aaBNbYKFVPjc17on
Every input to the segment geometry is unvalidated file data, and both the segment buffer and the offset/length tables were sized from it before anything could be range-checked. RowsPerStrip was taken as-is once it was non-zero. Writers routinely say "one strip" by setting it past the frame height - 65535 is the usual choice - so a 4000-row image would allocate and zero-fill a 65535-row segment buffer, 2.3 GB for a three-sample frame. 0x7FFFFFFF, which survives because getint() lands it in an int where only 0xFFFFFFFF reads as -1, turns a valid file into a length_error that unpack() reports as out-of-memory. Clamping to raw_height matches what tile_stripe_data_t::init() already does, and the same clamp on the tile dimensions costs nothing. The segment count had no bound at all. TileWidth = TileLength = 1 on a 24 MP frame yields 24 million segments: two INT64 vectors of 384 MB and 48 million get4() calls walking a table that is only checked against the file size afterwards. At the frame size identify() permits it reaches tens of GB. Cap it at a million as tile_stripe_data_t::init() does. Also refuse a tiled IFD whose TileByteCounts tag is missing: `bytes` is zero there, and seeking to zero reads the TIFF header as though it were an array of segment lengths. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2DjC7aaBNbYKFVPjc17on
A TIFF tile is padded out to a multiple of 16 in each direction, and the decoded segment comes back at that padded size. For a multi-tile image TileWidth and TileLength already carry the padded dimensions, so the buffer was the right size and the `dw > seg_width` check passed. A single-tile IFD is the exception: tiff.cpp zeroes TileWidth and TileLength when TileOffsets has one entry, tiff.cpp then turns those into INT_MAX, and the geometry collapses onto the frame size - which need not be a multiple of 16. So a 1000x750 image stored as one 1008x752 tile got a buffer 24 thousand samples too small, and the decode was rejected as DECODE_JPEG whether the host reported the padded size or simply ran out of room. The lossy path never hit this because it clips rather than rejecting. Size the buffer for the padding, and check the reported geometry against the buffer instead of against the frame. adobe_copy_pixel() already drops whatever lands outside the raw frame, so the padding needs no separate handling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2DjC7aaBNbYKFVPjc17on
Replacing the placeholder dropped the `#ifndef USE_DNGSDK` guard that used to sit around the UNSUPPORTED_FORMAT flag, so a build with the DNG SDK and qDNGSupportJXL started reporting JPEG XL DNGs as unsupported unless a host decoder happened to be registered. valid_for_dngsdk() accepts compression 52546 "regardless of flags", and unpack() routes through try_dngsdk() before load_raw is ever called, so those files decode fine - a caller pre-screening on the flag, which is what the flag is for, would skip a file that works. Reporting-only, and Narrative's vendor build carries no DNG SDK, but it contradicts the header's promise that a build which never registers a decoder keeps its present behaviour. Restore the guard in the form the vc5 branch two cases up uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2DjC7aaBNbYKFVPjc17on
The hook was a file static in dng.cpp reached through two extern "C" trampolines whose prototypes were hand-retyped at both call sites - once in libraw_c_api.cpp, once in decoder_info.cpp - with no header between them. Unmangled linkage means a signature drifting apart from the definition compiles and links without a word. Declaring the pair once in libraw.h as static members removes the retyped prototypes and the extra linkage names, and puts the C++ API on a par with the C one, which could previously reach the hook while the class could not. Registration stays process-wide. Per-instance would be the more conventional shape - every other host hook is a libraw_callbacks_t slot with a void* - but the decoder it points at is a single global libjxl-style runner on the host side, and making the C API take a handle would be a breaking change for a property that is not per-file. LibRaw::version() and cameraList() are the precedent for a static declared this way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2DjC7aaBNbYKFVPjc17on
None of this had automated coverage; verification was three files on one machine, and there is no JPEG XL fixture in the repo to build a test on. There does not need to be. What LibRaw owns is the segment walk, the geometry it derives from the TIFF tags, where the samples end up and what it refuses - none of which needs a real codestream. So the test writes its own DNGs, with a segment payload that names the segment's geometry and its position in the frame, and registers a stub decoder that fills the buffer from that. Every sample is then a known function of its absolute raw coordinate, so a whole frame can be checked exactly, margins and padding included. Nine cases, one per shape that went wrong: a CFA mosaic decoded into raw_image, a tiled LinearRaw frame with an ActiveArea whose margins have to survive, a LinearizationTable that has to be applied, a single tile padded past the frame, RowsPerStrip claiming 65535 rows, a tile size that implies sixteen million segments, a half-float frame, a tiled IFD with no TileByteCounts, and the decoder-not-registered path including the flags get_decoder_info() reports. Eight of the nine fail against the commit this branch started from. The exception is the missing-byte-counts case, which pins behaviour rather than catching a regression. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2DjC7aaBNbYKFVPjc17on
Three things the contract left open and the decoder now depends on: the channel count has to match SamplesPerPixel rather than the colour count, a padded tile may legitimately report a geometry larger than the frame, and samples are passed through unscaled because LibRaw applies the LinearizationTable and takes the levels from the DNG tags itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2DjC7aaBNbYKFVPjc17on
…PEG XL DNG Converter does not store a JPEG XL CFA mosaic as a mosaic. It sets RowInterleaveFactor (DNG 1.2, tag 50975) and ColumnInterleaveFactor (DNG 1.7.1, tag 52547) to 2, and per the 1.7.1 spec the stored image is then a grid of subimages each holding every Fth row and column - "a Bayer mosaic image with a 2x2 CFA pattern can be stored as a set of four monochrome subimages, each of which are 1/2 the width and 1/2 the height of the full image ... The top-left color of the 2x2 pattern maps to the top-left subimage". It is that grid the IFD's tile grid covers, and a tile can straddle several subimages. The spec adds why: it "may be useful when using compression methods that do not natively support interleaved pixels", which JPEG XL does not. LibRaw parsed neither tag, so the decoder placed each tile's samples as though they were image pixels at the tile's position. Every tile then held a quarter-resolution view of a region twice its size, seams ran along the grid, and the demosaicer read green sites as red and blue: a lossless conversion of a Canon 400D CR2 rendered magenta with 77% of pixels more than 32 levels off, while its channel means still looked plausible. Parse both tags per IFD, and map each stored position back to its image position before adobe_copy_pixel: subimage k holds the rows (columns) congruent to k mod F, in order, in ceil(H/F) (ceil(W/F)) each; the padding an odd-sized frame's later subimages carry maps outside the frame and is dropped. With both factors 1 - the lossy LinearRaw files DNG Converter writes, and any CFA file from a writer that does not use the tags - the map is the identity. Nothing here is specific to CFA data or to JPEG XL; the tags are honoured for whatever this decoder is handed, though no other LibRaw decoder reads them. Verified against that CR2: with the tags honoured the decoded mosaic is sample-for-sample identical to LibRaw's decode of the CR2 itself, and the render is within 19 levels of the CR2 render on its worst pixel. Six synthetic cases: the striped CFA frame stored 2x2, a CFA frame without the tags that must be left alone, tiles straddling subimage boundaries in both directions, an odd-sized frame whose padding has to be dropped, rows only in three fields, and a three-sample LinearRaw frame interleaved 2x2. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2DjC7aaBNbYKFVPjc17on
A lossy DNG stores its samples through a gamma-like curve and undoes it with one MapPolynomial per plane in OpcodeList2. lossy_dng_load_raw() parses those into per-plane tables; a lossy JPEG XL DNG carries exactly the same opcodes - DNG Converter's -lossy -jxl output has three - but nothing in the JPEG XL path read them, so the encoded samples went through as if linear. The result renders near white: channel means of 254/231/247 for a frame whose scene averages 156. The parse moves into dng_map_polynomials(), shared by both decoders, which then evaluate the polynomial over their own sample ranges: lossy JPEG over 8-bit samples onto the full 16-bit range identify() sets for it, JPEG XL over 0..maximum in the file's own scale, keeping the result there so the white level identify() read from the file stays true. One behaviour change for lossy JPEG on the way: a plane that has no opcode now keeps its samples as they came, where before its table was never written and read as whatever was on the stack. Verified against DNG Converter's lossy output of a Canon 400D CR2: the render's channel means move from 254/231/247 to 157/155/158, against 156/153/156 for the CR2 rendered directly, and it sits 9.2 levels on average from the CR2 render resampled to its size. The synthetic case gives every plane a slope-0.5 polynomial and expects every sample halved. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2DjC7aaBNbYKFVPjc17on
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
1 participant
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.
Turns
jxl_dng_load_rawfrom a throwing placeholder into a real decoder driven by a host callback, so a JPEG XL DNG (compression 52546, DNG 1.7) unpacks natively. LibRaw carries no JPEG XL decoder of its own and this doesn't add one —libraw_set_jxl_decoder()takes a callback, and NarrativeApp/Narrative-Maxwell#4141 registers one backed by the pure-Rustjxlcrate. No Adobe DNG SDK, no C++ libjxl.With no decoder registered,
jxl_dng_load_raw()throwsUNSUPPORTED_FORMATexactly as the placeholder did, so a build that never calls the setter is unchanged.What the decoder does
Same segment walk as
lossless_dng_load_raw(): step through the tiles or strips, decode each, hand its samples toadobe_copy_pixel()at the segment's place in the grid. The callback is given whole buffers rather than a stream, so each segment's length comes fromTileByteCounts/StripByteCounts.Two things a JPEG XL DNG needs that no other LibRaw decoder does, both learned from DNG Converter's output rather than from the first three commits' assumptions:
RowInterleaveFactor/ColumnInterleaveFactor(DNG 1.2 / 1.7.1). DNG Converter sets both to 2 for a CFA mosaic, so — per the 1.7.1 spec — the stored image is "a set of four monochrome subimages, each of which are ½ the width and ½ the height … The top-left color of the 2x2 pattern maps to the top-left subimage", and the tile grid covers that arrangement. The decoder maps each stored position back to its image position for any factor and any sample count; identity when both are 1.OpcodeList2MapPolynomial for lossy frames, the same gamma-like encoding lossy JPEG DNG uses. The parse is shared withlossy_dng_load_raw()viadng_map_polynomials()and evaluated over 0..maximum here.The commits
588f19a585bbfc65025603262dbe2ff2adobe_copy_pixel— CFA frames allocateraw_imagenotimage(the original threwIO_CORRUPTon every one); ActiveArea margins;curve[]for aLinearizationTable383f7e0d0xffff— a 14-bit-in-16-bit file rendered ~4 stops dark362e4b24SampleFormat3) rather than decoding it as integers181cf4fbRowsPerStrip, cap the segment count at 1 M, refuse a tiled IFD with noTileByteCountsadc1c32443a1729fUSE_DNGSDKguard indecoder_infoa042877eLibRaw::setJxlDecoder/haveJxlDecoder— one declaration inlibraw.hinstead of hand-retypedextern "C"prototypes; C ABI unchanged603a253f07d074d9SamplesPerPixel, padded geometry allowed, samples unscaled7c41338674233a89Verification
Real files, decoded end to end through Maxwell against a bundle built from this branch. Both fixtures were converted by DNG Converter 18.6 from a Canon 400D CR2, so that CR2 is the reference:
7c413386/74233a89The CFA file has
WhiteLevel3650, ActiveArea margins of 18/42 and 672×656 tiles straddling the subimage boundaries — the non-full-range, margined, tiled shape the earlier "Known gaps" section said was untested. It is now the test.Synthetic DNGs:
samples/jxl_dng_test.cpp, wired intoMakefile.amandMakefile.dist, runbin/jxl_dng_test. No fixtures, no real codestream: each segment's payload names its geometry and stored position, a stub decoder fills the buffer from it, and whole frames are checked exactly. 15 cases: CFA striped and stored 2×2, a CFA frame without the tags that must be left alone, tiles straddling subimage boundaries both ways, an odd-sized frame whose padding is dropped, rows-only in three fields, a 3-sample LinearRaw frame interleaved 2×2, LinearRaw with ActiveArea margins,LinearizationTable, MapPolynomial, a padded single tile,RowsPerStrip65535, a 16-million-segment tile size, half-float, missingTileByteCounts, and the not-registered path with theget_decoder_infoflags. Eight of the review-round cases fail against02560326.Sony A7 V and A7R VI ARW controls unchanged throughout.
Still open
Merge order
artifact.envbump to r5🤖 Generated with Claude Code
https://claude.ai/code/session_01L2DjC7aaBNbYKFVPjc17on