Add support for single-frame DICOM images and images with Bits Allocated > 8 #102
Conversation
|
Hi again @tokyovigilante, This is fantastic! Thank you for doing this work. And how nice that such a (relatively) simple change is all that's needed! We'll need a changelog entry, and we should add some tests. Do you have a sample CC0 image you could suggest? Or I can try and find one from nema. Would you like to add the tests? Or if you prefer I could try after this is merged. I have another question too -- just for my curiosity, what's your use case, and how did you find this library? There are many other C libraries for reading DICOMs, of course, why pick this one? |
No problem, thanks for reviewing!
Sure, happy to work on those and supply or find an image sample too.
I’m working on an image viewer / 3D viewer for some new/novel visualisation techniques, and needed something fairly portable and C-based as my app is not written in C++, and has a much better C FFI than C++. Most of the other libs are C++, python, JS, or just add too many features that I don’t need over basic DICOM header parsing and file access. |
|
Hi again, did you find any more time to look at this? Or we can merge and then do any further work (tests etc.) in a follow-up PR, if that's easier. |
tokyovigilante
force-pushed
the
main
branch
2 times, most recently
from
f876048 to
a4db07e
Compare
Hey, sorry for the delay. I've gone back to the simpler calculation for BitsStored, and added tests, an anonymised CT test file, and a changelog entry. Hopefully this is ready to merge! [EDIT]: Whoops of course I mean BitsAllocated, have updated the PR. |
tokyovigilante
force-pushed
the
main
branch
2 times, most recently
from
dd04fa5 to
f1c5687
Compare
…ated. 8.1.1 Pixel Data Encoding of Related Data Elements Bits Allocated (0028,0100) shall either be 1, or a multiple of 8. https://dicom.nema.org/medical/dicom/current/output/html/part05.html#PS3.5
| @@ -1038,7 +1038,7 @@ char *dcm_parse_frame(DcmError **error, | |||
| return NULL; | |||
| } | |||
| } else { | |||
| *length = desc->rows * desc->columns * desc->samples_per_pixel; | |||
| *length = desc->rows * desc->columns * desc->samples_per_pixel * desc->bits_allocated / 8; | |||
There was a problem hiding this comment.
Could we change this computation? This could overflow int32 rather easily for larger values of bits_allocated (eg. 32).
How about:
*length = desc->rows * desc->columns *
desc->samples_per_pixel * (desc->bits_allocated / 8);We could also use int64, but bracketing the /8 is easier.
|
Sorry for the long delay @tokyovigilante, I was on holiday and then distracted by other projects. I thought of one final (tiny!) thing. |
Most radiology images are stored with a single image per file (even for stacked/volumetric data sets like CT), and require > 8 bits of pixel storage (e.g. CT is 12-bit, DR/CR is 16-bit).
Assume a single image frame for files which do not have a NumberOfFrames tag, and calculate the correct Pixel Data length for files with Bits Allocated > 8.