-
Notifications
You must be signed in to change notification settings - Fork 116
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
US cine loops with Transfer Syntax RLE Lossless are corrupted after DicomMessage.write when fragmentMultiframe option is enabled #340
Comments
The corresponding issue in dcmjs-dimse: #PantelisGeorgiadis/dcmjs-dimse/issues/40 |
Thanks for the careful debugging. it looks like there's a long history of commits related to this code, but the behavior seems to trace back to the original implementation, so there's no specific discussion of the original implementation, but it's probably for efficiency and memory management. Since you have been able to isolate this issue do you see a solution? |
I've also run into an issue in this area recently, and discovered that My understanding (which is not canonical) is that these fragments are supported in order to help split up very large pieces of compressed image/frame data, but exactly how this is meaningfully leveraged by implementations in practice I don't have direct experience of. The relevant part of the spec is https://dicom.nema.org/dicom/2013/output/chtml/part05/sect_A.4.html. In my experience, the general level of support for multi-fragment storage of DICOM image data, particularly multiple fragments per frame in e.g. a multi-frame ultrasound, seems patchy. For this reason, not using it seems reasonable unless one has a strong reason to want it (and frankly I've yet to come across one, but am keen to understand what the use cases are). Why dcmjs turns fragments on by default, and uses a fragment size of 20 KiB which does seem small, I simply don't know. I did some PRs a while back that fixed dcmjs' support for multi-fragment files so it was in alignment with the DICOM spec, but no doubt there are libraries/implementations around that don't follow the spec or implement all parts of it, so it may be best to just turn off multi-fragment altogether via I think I'd be in favour of making this the default in dcmjs, but in saying that I don't feel 100% confident that I understand the potential ramifications of doing that. Maybe in some cases with very large datasets it starts to matter, but also those people can always turn it on if they need it. |
I don't have a lot of experience with this either, but I'm sure you are correct @richard-viney that various data or implementations will vary in their support and correctness. I suspect this feature could come up in whole slide imaging use cases, e.g. in Slim. At this point I'm wary of changing default behavior but if we have some example data and tests we could compare notes with other dicom developers and see what we think the most reasonable behavior should be. |
For what it's worth, it looks like pydicom defaults to one fragment per frame: |
Hello.
I faced corrupted US cine loops with Transfer Syntax RLE Lossless when using dcmjs-dimse for receiving and sending DICOM images with C-STORE.
After some research, I've found that it is related to the
fragmentMultiframe
option ofDicomMessage.write()
which enabled by default. WhenfragmentMultiframe
is true I have the following errors with further processing:What is the purpose of splitting PixelData into such small chunks by default?
The text was updated successfully, but these errors were encountered: