Repository navigation
feat(libjxl): JPEG recompression and progressive encoding - #97
wayfarer3130 wants to merge 2 commits into
Conversation
Adds embind methods for the JPEG XL JPEG Recompression transfer syntax (1.2.840.10008.1.2.4.111) and for progressive output: - JpegXLEncoder.getJpegBuffer() / encodeJpeg(): recompress a JPEG bitstream losslessly (JxlEncoderAddJPEGFrame with JPEG metadata stored). - JpegXLDecoder.decodeToJpeg() / getJpegBuffer(): rebuild the original JPEG bytes from the jbrd box (JxlDecoderSetJPEGBuffer). - JpegXLEncoder.setProgressive(): progressive DC + QPROGRESSIVE_AC for lossy, RESPONSIVE for lossless, as cjxl -p. dicom-codec: transcode() goes directly between .50 and .111 without a pixel decode, the .111 codec exposes recompressJpeg()/reconstructJpeg(), and JPEG XL encodes accept a `progressive` option. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (8)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe JPEG XL encoder now supports progressive output and JPEG bitstream input. The decoder can reconstruct JPEG bytes from streams that contain reconstruction data. The DICOM codec adds direct transcoding between JPEG Baseline and JPEG XL JPEG Recompression. ChangesJPEG XL recompression
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant transcode
participant recompressJpeg
participant JpegXLEncoder
participant reconstructJpeg
participant JpegXLDecoder
transcode->>recompressJpeg: JPEG Baseline bytes and encode options
recompressJpeg->>JpegXLEncoder: JPEG bytes
JpegXLEncoder-->>recompressJpeg: JPEG XL bytes
transcode->>reconstructJpeg: JPEG XL bytes
reconstructJpeg->>JpegXLDecoder: JPEG XL bytes
JpegXLDecoder-->>reconstructJpeg: reconstructed JPEG bytes
Merge Risk: ⚪ Minimal · up to No identified issue blocks merging after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 6 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merging this PR will regress 2 benchmarks
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
The encoder now links libjxl's JPEG transcode path (JxlEncoderAddJPEGFrame and the JPEG metadata store), and the decoder links JPEG reconstruction. - jpegxlwasm_encode.wasm: 2550720 -> 2598167 raw (+1.86%), 927454 -> 946918 gzip (+2.10%) - jpegxlwasm_decode.wasm: 1090788 -> 1092412 raw (+0.15%), 377101 -> 377733 gzip (+0.17%) Sizes are from the dist-libjxl artifact of the PR CI build (emsdk 3.1.74). Only the libjxl entry changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
igoroctaviano
left a comment
igoroctaviano
left a comment
There was a problem hiding this comment.
Nice work on this PR. Left a few comments inline.
| } | ||
|
|
||
| case JXL_DEC_FULL_IMAGE: { | ||
| if (!reconstructing) { |
There was a problem hiding this comment.
This branch appears unreachable. If the stream lacks a jbrd box, the decoder fires JXL_DEC_NEED_IMAGE_OUT_BUFFER first (handled below at line 151), so we never reach JXL_DEC_FULL_IMAGE with reconstructing == false. Harmless but could be removed for clarity, or kept as defensive code with a comment explaining the intent.
| JxlEncoderCloseInput(enc); | ||
|
|
||
| // Recompression saves about 20%, so the JPEG size is a close first guess. | ||
| processOutput(enc, std::max<size_t>(64u * 1024u, jpeg_.size())); |
There was a problem hiding this comment.
After encodeJpeg(), calling getFrameInfo() returns stale/empty data since frameInfo_ isn't populated here. Might be worth either documenting this limitation or clearing frameInfo_ explicitly so callers don't get confused by leftover state from a previous encode() call.
| Decoder: undefined, | ||
| Encoder: undefined, | ||
| encoderName: "JpegXLEncoder", | ||
| decoderName: "", |
There was a problem hiding this comment.
Minor: empty strings for unused names work fine, but null would be more explicit about the intent that these wrappers are single-direction (encode-only or decode-only).
2 participants
Summary
This PR adds two features to
@cornerstonejs/codec-libjxl, and it uses them in@cornerstonejs/dicom-codec:1.2.840.10008.1.2.4.111). The encoder recompresses a JPEG bitstream without loss. The decoder gives back the original JPEG bytes.A whole-slide imaging (WSI) tool needs these features. The tool converts JPEG tiles to JPEG XL without loss, and it writes progressive JPEG XL pyramids.
libjxl 0.11.1 already compiles the transcode code, because
JPEGXL_ENABLE_TRANSCODE_JPEGisON. Before this PR, the embind bindings did not expose that code.Changes
@cornerstonejs/codec-libjxlJpegXLEncoder.getJpegBuffer(size)andJpegXLEncoder.encodeJpeg()callJxlEncoderStoreJPEGMetadataandJxlEncoderAddJPEGFrame. Effort and decoding speed apply. Lossless, distance, and progressive do not apply.JpegXLDecoder.decodeToJpeg()andJpegXLDecoder.getJpegBuffer()useJxlDecoderSetJPEGBuffer. The decoder does not decode the pixels.decodeToJpeg()throws when the stream has no JPEG reconstruction data (nojbrdbox).JpegXLEncoder.setProgressive(bool)uses the same settings ascjxl -p:PROGRESSIVE_DCandQPROGRESSIVE_AC;RESPONSIVE.@cornerstonejs/dicom-codectranscode()goes directly from.50to.111, and from.111to.50. The transcode does not decode the pixels, and the result is byte for byte the same JPEG..111codec exportsrecompressJpeg()andreconstructJpeg().progressiveoption.encode()to.111from pixels still throws. The message keeps the text "is not supported by the pixel encoder", and the message now tells the caller to usetranscode()orrecompressJpeg().Tests
packages/libjxl/test/module.test.jshas four new tests:decodeToJpeg()refuses a stream without reconstruction data;packages/dicom-codec/test/jpegxl-fixtures.test.jshas one new test:.50→.111→.50through the publictranscode().tools/docker/build.sh(emsdk 3.1.74):Size
The JPEG transcode code of libjxl is now linked into the modules. The
dist-sizecheck failed on the encoder, so this PR updates thelibjxlentry oftools/dist-size/baseline.json. The new values come from thedist-libjxlartifact of the CI build of this PR.jpegxlwasm_encode.wasmjpegxlwasm_decode.wasmThe decoder change is in the tolerance of the check, but the PR records it so that the baseline matches the build. The baseline of the other packages does not change.
Notes
jxl_dec.dcmjs transcodecommand and adcmjs wsiresizecommand. Those commands use these methods.🤖 Generated with Claude Code
Summary by CodeRabbit