feat(12bit-support): add support to 12-bits containers for Little Endian, Big Endian, JPEG, gzip and RLE file formats#2661
Open
TaylorHo wants to merge 9 commits into
Open
feat(12bit-support): add support to 12-bits containers for Little Endian, Big Endian, JPEG, gzip and RLE file formats#2661TaylorHo wants to merge 9 commits into
TaylorHo wants to merge 9 commits into
Conversation
Author
|
The PR to GDCM was opened in its repo. |
📝 WalkthroughWalkthroughThis PR extends DICOM image decoding to support 12-bit pixel data. A Changes12-bit DICOM Image Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/dicomImageLoader/src/shared/decoders/decodeBigEndian.ts (1)
13-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid byte-swapping unpacked 12-bit samples in big-endian decode.
The 12-bit uncompressed path already unpacks into 16-bit sample values before this decoder runs. Including
bitsAllocated === 12in theswap16branch causes an extra byte inversion and corrupts pixel values.💡 Proposed fix
- if (imageFrame.bitsAllocated === 16 || imageFrame.bitsAllocated === 12) { + if (imageFrame.bitsAllocated === 16) { let arrayBuffer = pixelData.buffer; let offset = pixelData.byteOffset; const length = pixelData.length; @@ // Do the byte swap for (let i = 0; i < imageFrame.pixelData.length; i++) { imageFrame.pixelData[i] = swap16(imageFrame.pixelData[i]); } + } else if (imageFrame.bitsAllocated === 12) { + let arrayBuffer = pixelData.buffer; + let offset = pixelData.byteOffset; + const length = pixelData.length; + + if (offset % 2) { + arrayBuffer = arrayBuffer.slice(offset); + offset = 0; + } + + imageFrame.pixelData = + imageFrame.pixelRepresentation === 0 + ? new Uint16Array(arrayBuffer, offset, length / 2) + : new Int16Array(arrayBuffer, offset, length / 2); } else if (imageFrame.bitsAllocated === 8) { imageFrame.pixelData = pixelData; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dicomImageLoader/src/shared/decoders/decodeBigEndian.ts` around lines 13 - 34, The decoder currently performs a 16-bit byte-swap for both 16-bit and 12-bit paths which corrupts already-unpacked 12-bit samples; update the condition in decodeBigEndian (file: decodeBigEndian.ts) so that the swap loop running swap16(...) only executes when imageFrame.bitsAllocated === 16 (leave the 12-bit path untouched), i.e. ensure imageFrame.pixelData for bitsAllocated === 12 is not passed through swap16; keep all other logic (alignment/Uint16Array vs Int16Array selection) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/dicomImageLoader/src/shared/decoders/decodeBigEndian.ts`:
- Around line 13-34: The decoder currently performs a 16-bit byte-swap for both
16-bit and 12-bit paths which corrupts already-unpacked 12-bit samples; update
the condition in decodeBigEndian (file: decodeBigEndian.ts) so that the swap
loop running swap16(...) only executes when imageFrame.bitsAllocated === 16
(leave the 12-bit path untouched), i.e. ensure imageFrame.pixelData for
bitsAllocated === 12 is not passed through swap16; keep all other logic
(alignment/Uint16Array vs Int16Array selection) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b79a06fd-53fe-4cd2-ac04-a6abc8c030e4
📒 Files selected for processing (7)
packages/core/src/utilities/generateVolumePropsFromImageIds.tspackages/dicomImageLoader/src/imageLoader/wadouri/getUncompressedImageFrame.tspackages/dicomImageLoader/src/shared/decoders/decodeBigEndian.tspackages/dicomImageLoader/src/shared/decoders/decodeJPEGBaseline12Bit-js.tspackages/dicomImageLoader/src/shared/decoders/decodeJPEGLossless.tspackages/dicomImageLoader/src/shared/decoders/decodeLittleEndian.tspackages/dicomImageLoader/src/shared/decoders/decodeRLE.ts
Author
|
Can someone please take a look here? @JamesAPetts, @wayfarer3130, @swederik, @sedghi Also, regarding this AI Caution warning, in this specific part of the code, there's no need for different logic to read the file when |
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.
Context
In the DICOM format, we can define Bits Allocated (0028,0100) and Bits Stored (0028,0101). In many CT DICOM images, there are 12 bits stored (Bits Stored) into 16-bit containers (Bits Allocated).
This PR adds support also for loading and rendering into Cornerstone3D DICOM images of 12 bits stored (Bits Stored) into 12-bit containers (Bits Allocated). This approach reduces ~25% of the file sizes, due to the removal of the extra empty bits. We know this can be useful only for some cases (for example, not all CT images can fit in 12 bits).
We also developed a tool for converting DICOM files that use 16-bit but have 12-bit stored values into this "12-bit allocated and 12-bit stored" format. We called it dicom-12bit.
We also applied this conversion and compression support to GDCM (a PR will be opened soon for the origin repo) in this GDCM fork.
All this is part of research conducted over the past months. This research will be sent for paper evaluation soon.
We have a working example of this 12-bit allocated and 12-bit stored into this demo repo: TaylorHo/dicom-12-bit-example-viewer. Here, the same changes that this PR does are applied as git patches to add this support. 12-bit images are provided as examples in this same repo.
We describe more useful results of this change in the paper; we are just opening this PR before submitting to keep the history.
Changes & Results
The changes try to be minimal, adding support to the uncompressed 12-bit formats: Little Endian and Big Endian. This also supports the JPEG family of compression algorithms, including JPEG-2000, JPEG-LS, and JPEG lossy, and files compressed with gzip and RLE.
If you, for example, convert a DICOM that is 12-bit stored into 16-bit containers, and use our tool to convert to 12-bit stored into 12-bit containers, you can also use our modified version of GDCM to compress this new format.
This way, using the demo application provided, you can see that all the cited formats are rendering correctly.
To facilitate, the demo application has a folder with all the possible compressed formats supported by Cornerstone3D, but with 12 bits allocated.
Testing
We developed a demo application that applies our changes using a git patch to the Cornerstone3D library and that can be used to visualize the 12-bit images.
You can use a tool like
gdcmdumpto see that all the example files in the repo are 12-bit stored into 12 bits.Checklist
PR
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
Summary by CodeRabbit