Skip to content

Support bits_allocated == 1 #654

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jun 4, 2025
Merged

Support bits_allocated == 1 #654

merged 8 commits into from
Jun 4, 2025

Conversation

Cryt1c
Copy link
Contributor

@Cryt1c Cryt1c commented May 31, 2025

Thanks a lot for this awesome project! I am using it for my learning project to create a DICOM Viewer: https://cryt1c.github.io/DICOMViewer/

Currently I want to extend the DICOMViewer with a feature where users can load DICOM-SEG files. After checking this repo I did not find this functionality or mentions on the issue queue, so I have tinkered around a bit and made it work. I would love to contribute this back upstream.

I am basically handling the case where bits_allocated is 1. In the decode_pixel_data_frame where each byte gets mapped to 8 separate bits which are handled as one byte each. Values are mapped to 0..255 so that the result should be a black and white segmentation mask.

We should probably handle the DICOM-SEG more explicitly or even create a separate crate for it? Please let me know what your preferred way would be.

I am happy to adjust the PR on your feedback.

@Enet4 Enet4 added A-lib Area: library C-pixeldata Crate: dicom-pixeldata labels May 31, 2025
Copy link
Owner

@Enet4 Enet4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this! It appears to bring support for images where bits allocated = 1 (#348). Please see if you can address the two concerns listed below.

  • It breaks other cases such as when there is more than one sample per pixel (tests fail too: run cargo test -p dicom-pixeldata --features=image,ndarray). I have left some advice inline.
  • We should extend this test coverage to include a file of a similar nature. liver.dcm from the pydicom dataset has Bits Allocated = 1, so it might do the trick.

We should probably handle the DICOM-SEG more explicitly or even create a separate crate for it? Please let me know what your preferred way would be.

That would probably fit as its own crate. Though the project at the moment does not offer many IOD abstractions on top the other generic crates, the pattern would eventually exist as additional crates in the DICOM-rs umbrella.

@Cryt1c
Copy link
Contributor Author

Cryt1c commented Jun 1, 2025

Thanks for your feedback!

I have fixed the frame_size calculation and implemented test_1bit_image_decoding. This is the resulting image:
liver-0

Co-authored-by: Eduardo Pinho <[email protected]>
@Cryt1c Cryt1c requested review from Enet4 June 2, 2025 18:13
@Cryt1c Cryt1c changed the title Implement DICOM-SEG Support bits_allocated == 1 Jun 2, 2025
Copy link
Owner

@Enet4 Enet4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! It is not fully clear whether the expansion of 1-bit pixel data values to the 0-255 range will come across as surprising to some users, but it shouldn't stop them from working around the pixeldata abstraction if this is a concern to them.

Thank you and welcome to the list of DICOM-rs contributors. I also want to thank you for giving a shout out to DICOM-rs last year at EuroRust 2024! 🦀

@Enet4 Enet4 merged commit f840a24 into Enet4:master Jun 4, 2025
5 checks passed
@Cryt1c Cryt1c deleted the segmentation branch June 5, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lib Area: library C-pixeldata Crate: dicom-pixeldata
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants