Skip to content
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

Open
jimOnAir opened this issue Feb 28, 2023 · 5 comments

Comments

@jimOnAir
Copy link

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 of DicomMessage.write() which enabled by default. When fragmentMultiframe is true I have the following errors with further processing:

$ find . -name '*.dcm' -exec /opt/dcm4che/bin/dcm2jpg {} {}.jpg \;
./1.2.156.112536.1.2118.80201085056.14485206710.12.dcm-patched.dcm -> ./1.2.156.112536.1.2118.80201085056.14485206710.12.dcm-patched.dcm.jpg
Native OpenCV library has been found in /opt/dcm4che/lib/macosx-aarch64/libopencv_java.dylib
08:49:55.929 DEBUG - Decompressor: org.dcm4che3.imageio.plugins.rle.RLEImageReader
08:49:55.930 DEBUG - Start decompressing frame #1
08:49:55.933 INFO  - RLE Segment #1 too short, set missing 363917 bytes to 0
08:49:55.933 INFO  - RLE Segment #2 too short, set missing 480000 bytes to 0
08:49:55.933 INFO  - RLE Segment #3 too short, set missing 480000 bytes to 0
08:49:55.933 DEBUG - Finished decompressing frame #1
$ dcmj2pnm --nointerlace --write-png --accept-acr-nema --frame 1 1.2.156.112536.1.2118.80201085056.14485206710.12.dcm-patched.dcm 1.2.156.112536.1.2118.80201085056.14485206710.12.dcm-patched.png --debug
D: $dcmtk: dcmj2pnm v3.6.7 2022-04-22 $
D:
D: DcmDataDictionary: Loading file: /usr/share/dcmtk/dicom.dic
I: reading DICOM file: 1.2.156.112536.1.2118.80201085056.14485206710.12.dcm-patched.dcm
D: DcmMetaInfo::checkAndReadPreamble() TransferSyntax="Little Endian Explicit"
D: DcmDataset::read() TransferSyntax="RLE Lossless"
I: preparing pixel data
D: transfer syntax of DICOM dataset: RLE Lossless (1.2.840.10008.1.2.5)
D: using partial read access to compressed pixel data
D: RLE decoder processes frame 0
D: RLE decoder processes pixel item 1
Segmentation fault

What is the purpose of splitting PixelData into such small chunks by default?

@jimOnAir
Copy link
Author

The corresponding issue in dcmjs-dimse: #PantelisGeorgiadis/dcmjs-dimse/issues/40

@pieper
Copy link
Collaborator

pieper commented Feb 28, 2023

What is the purpose of splitting PixelData into such small chunks by default?

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.

https://github.com/dcmjs-org/dcmjs/blame/646b1f181a3add90588175c79376308e9d1eb84b/src/ValueRepresentation.js

b77c642

d92ffc4

Since you have been able to isolate this issue do you see a solution?

@richard-viney
Copy link
Contributor

I've also run into an issue in this area recently, and discovered that pydicom with certain libraries loaded doesn't reliably handle multiple fragments per frame in multi-frame DICOMs. Opened an issue for it here: pydicom/pydicom#1774.

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 writeOptions.

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.

@pieper
Copy link
Collaborator

pieper commented Mar 19, 2023

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.

@richard-viney
Copy link
Contributor

For what it's worth, it looks like pydicom defaults to one fragment per frame:

https://github.com/pydicom/pydicom/blob/2709ff6ed5458bef0da778c56b9918c8c62c4292/pydicom/encaps.py#L658

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants