-
Notifications
You must be signed in to change notification settings - Fork 92
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
Conversation
There was a problem hiding this 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.
Co-authored-by: Eduardo Pinho <[email protected]>
There was a problem hiding this 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! 🦀
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.