Invalid loading of segmentation with cropped in-plane data

[razorx89]

Bug Report

Describe the Bug

I am the author of pydicom-seg, a library for reading and writing DICOM-SEG in Python, and currently try to track down errors in using the generated DICOM-SEGs in OHIF/Viewer (https://github.com/razorx89/pydicom-seg/issues/20).

Currently, OHIF viewer does not correctly load DICOM-SEG data, which was cropped in-plane. The CT image has an in-plane resolution of 512x512, while the cropped segmentation has a resolution of 272x224. As you can see below, the 512x512 is shown correctly while the 272x224 is distorted. Cropping is intended to reduce the generated DICOM size, since for small segmented objects most of the frame data is only zeros.

It seems like an error in accessing the allocated segmentation buffer and thus assigning to the wrong voxels. In order to correctly assign the voxels, the PlanePositionSequence[0].ImagePositionPatient attribute in the PerFrameFunctionalGroupSequence as well as the Rows and Columns need to be used for computing the correct buffer indices.

Loading the DICOM-SEG using my package or via the conversion tool segimage2itkimage from QIICR/dcmqi, followed by resampling to the original CT image grid using SimpleITK, displays the correct segmentation in ITKSnap.

What steps can we follow to reproduce the bug?

Data credits go to: QIICR/dcmqi I prepared the ct-3slice scan, from dcmqi's repository, with the corresponding liver.dcm segmentation as example.zip. The liver-nocrop.dcm is exported using my package without in-plane cropping, the liver-cropped.dcm is exported with cropped segmentation data.

[pieper]

Thanks for the detailed report 👍

Can you try one quick experiment? Click the 2D MPR button and see if it displays correctly on the other planes. If it does, then this is something to do with how OHIF cornerstone handles the different grids. If the MPR is also wrong then it's something with the SEG parsing in dcmjs.

[razorx89]

Sure!

It is hard to see since the CT image only contains three slices, but it is also broken in MPR.

[pieper]

Interesting - a bit off-topic for this issue, but I tried to replicate this using OHIF against google's dicomweb and I get the message below about it being an invalid instance. I guess google doesn't support a value of 1 for BitsAllocated. But this is not correct according to the standard. FYI @fedorov I guess I'll report this as a bug to the google folks.

https://healthcare.googleapis.com/v1beta1/projects/idc-sandbox-000/locations/us-central1/datasets/sdp-dev/dicomStores/cropped-seg/dicomWeb/studies/1.2.392.200103.20080913.113635.0.2009.6.22.21.43.10.22941.1/series/1.2.826.0.1.3680043.8.498.71167181430473953726005125072372749258/instances/1.2.826.0.1.3680043.8.498.88983434253411987964922722856183426978

generic::invalid_argument: error while transcoding from 1.2.840.10008.1.2.2 to application/dicom; transfer-syntax=1.2.840.10008.1.2.1: invalid Pixeldata macro attributes: Rows/Columns/SamplesPerPixel must be set and BitsAllocated must be a multiple of 8, cause (internal only): generic::invalid_argument: invalid Pixeldata macro attributes: Rows/Columns/SamplesPerPixel must be set and BitsAllocated must be a multiple of 8
com.google.apps.framework.request.StatusException: <eye3 title='INVALID_ARGUMENT'/> generic::INVALID_ARGUMENT: invalid Pixeldata macro attributes: Rows/Columns/SamplesPerPixel must be set and BitsAllocated must be a multiple of 8

[razorx89]

Hm, could you please check if this liver.dcm is working on Google's implementation? Just to sort out any issues in pydicom-seg.

When using dcmdump there is a warning about an odd PixelData length, if Rows*Columns*NumberOfFrames != N*8 (not in this liver example though). This comes from numpy.packbits, which pads bits to a multiple of 8. But I think that this should be correct, according to the implementation in DCMTK.

[fedorov]

There is also this odd-sized sample dataset in dcmqi that we tested against BrainLab and I believe confirmed the encoding is correct (that is the one that we used to debug encoding bug in DCMTK): https://github.com/QIICR/dcmqi/tree/master/data/segmentations/24x38x3.

[pieper]

Interesting - liver.dcm from qiicr works on google+OHIF, but liver-full.dcm (from pydicom_seg) does not.

[fedorov]

liver.dcm from qiicr is not cropped, so it does not have that issue. Did you try the odd-sized sample I mentioned above?

[pieper]

liver.dcm from qiicr is not cropped, so it does not have that issue.

Both liver.dcm and liver-full.dcm are 512x512 with BitsAllocated of 1, but only the pydicom-seg one generates the error from google.

[fedorov]

oh, sorry - I forgot there are two separate (or not so separate) issues now being investigated.

[pieper]

Yes, 24x38x3 works

[pieper]

Getting back to the original issue, it would be good to set up a test in dcmjs to track down which versions of the files unpack correctly and which do not.

[razorx89]

liver.dcm from qiicr is not cropped, so it does not have that issue.

Both liver.dcm and liver-full.dcm are 512x512 with BitsAllocated of 1, but only the pydicom-seg one generates the error from google.

I made a diff of both files and spotted two differences in the FileMeta header (and smaller differences in the actual DICOM, which should not cause any DicomWeb issues). pydicom overrode my specified TransferSyntaxUID from ExplicitVRLittleEndian to ExplicitVRBigEndian and the FileMetaInformationGroupLength is 12 Bytes longer using pydicom's serialization (I don't know why, but more bytes shouldn't be a problem). Maybe ExplicitVRBigEndian and bit packing is not supported? Below are new DICOM-SEGs with only differences in UIDs and Strings.

example-fix.zip

[JamesAPetts]

Sure, the change required is at the dcmjs level. We currently only support segmentations with the same size as the source data, as implementation for SEG support has thusfar been a "most common up" approach, but this shouldn't be too hard to change if you would like to submit a pull request.

[pieper]

@razorx89 yes, confirmed that example-fix.zip work on google's dicomweb and also has the same behavior as in your original screenshot, so it looks like your analysis is correct. Do you think the original example.zip files were actually valid also and we should report the issue to google?

Regarding supporting cropped seg in dcmjs/ohif I agree with @JamesAPetts that we'd accept a PR but don't have a clear use case ourselves at the moment. Is this something in the critical path, or just experimentation?

[razorx89]

@razorx89 yes, confirmed that example-fix.zip work on google's dicomweb and also has the same behavior as in your original screenshot, so it looks like your analysis is correct. Do you think the original example.zip files were actually valid also and we should report the issue to google?

Great to hear! I think there shouldn't be a limitation according to the standard. DCMTK also allows writing with ExplicitVRBigEndian (source). The only problem i can think of with example.zip is that the bits were packed via np.packbits in little instead of big endian order. But that should only cause an invalid decoding of the segmentation, not an issue when transferring the data.

Regarding supporting cropped seg in dcmjs/ohif I agree with @JamesAPetts that we'd accept a PR but don't have a clear use case ourselves at the moment. Is this something in the critical path, or just experimentation?

Currently, it is nothing critical, I can disable it and add a warning in the documentation until we have a PR for this. I haven't looked much into dcmjs yet, but I will try to implement this functionality. In my opinion this feature would be an easy way to save storage space in a PACS with automatically generated AI reports. For pure research projects the cohort sizes might not cause issues in storage space, for clinical deployment it matters if the segmentation is a few MB or few KB (for thousands of studies).

[fedorov]

@razorx89 you might want to consider if the savings in size are worth the various risks related to the complicated usage of the resulting objects.

Even such simple things as skipping empty slices apparently cause a lot of headaches for some presumably technically savvy imaging researchers, and took some time before it was supportedin BrainLab and OHIF.

[pieper]

I agree with @fedorov that there none of the implementation are sure to be feature-complete with respect to the standard and may not have the resources to be fully fleshed out.

Regarding file sizes, the thing that came up in both Slicer and OHIF was that labelmap-stye indexed segmentations save a lot of memory in the common case of non-overlapping segments but are not in the standard.

[razorx89]

Thank you for your opinion on this. I agree, the support will be a problem not only in OHIF although it is standard conformant. Maybe I should deactivate this option by default and mark it as an experimental serialization feature.

At first it was only an idea to save some storage space when I was implementing the first version of the writer class. Now I got SurfaceSegmentationStorage DICOM-SEGs from one of our research partners and I am implementing a conversion script from SurfaceSegmentationStorage to SegmentationStorage via voxelization of the polygonal mesh (will be available in pydicom-seg soon). The voxelized mesh is not of the same size as the image, but I think I will just resample it to the referenced image series to make things simple outside of my package.

[fedorov]

Now I got SurfaceSegmentationStorage DICOM-SEGs from one of our research partners

This is great - is read/write of DICOM surface segmentations going to be included in pydicom-seg?

[pieper]

Just to close the loop, Google got back to me and said it's a known issue they hope to fix.

On the topic of surface seg, there's a prototype slicer reader, but it would be cool to think about doing a more complete implementation.

[razorx89]

Yes, I wrote a first simple conversion script and am able to generate SegmentationStorage DICOMs from a SurfaceSegmentationStorage (and display it in OHIF Viewer :+1:). Next step will be to get more examples to test it for robustness and refactor the code for inclusion in pydicom-seg. For writing I have also something in mind. But especially for round-trip conversions it is not straight forward, since IMO SurfaceSegmentationStorage DICOMs not only represent semantic segmentation, but also instance segmentation and thus we would require an additional indexing dimension when writing SegmentationStorage DICOMs. And then we are already back on the topic of compatibility with other frameworks/viewers. Without the additional indexing dimension, the individual surfaces per segment could be fused, but then the round-trip wouldn't work.

Maybe we should continue the discussion on SurfaceSegmentationStorage in https://github.com/razorx89/pydicom-seg/issues/18.

[pieper]

Sounds good. Would be nice to have better support for surfaces but it's not in my critical path right now and not too common yet in the wild.

[dclunie]

@razorx89, I would strongly advise never using ExplicitVRBigEndian.

We retired it from the standard ages ago (2006) to discourage its use (Sup 98 and CP-1549)

It should never be the default output from a toolkit.

It is good to test that it works though, since reading ExplicitVRBigEndian is supported by most toolkits.

David

PS. I had no trouble displaying either of your cropped segmentations (big or little endian VR), and validation of the file showed just a few errors that will not affect use of the PixelData;

% dciodvfy example-fix/liver-cropped.dcm Warning - Value dubious for this VR - (0x0010,0x0010) PN Patient's Name PN [0] = - Retired Person Name form Warning - Value dubious for this VR - (0x0070,0x0084) PN Content Creator's Name PN [0] = - Retired Person Name form Segmentation Error - Missing attribute Type 1 Required Element= Module= Warning - Coding Scheme Designator is deprecated - attribute = Warning - Coding Scheme Designator is deprecated - attribute = Error - Empty attribute (no value) Type 1C Conditional Element= Module=

[sedghi]

Try viewer-dev.ohif.org instead of viewer.ohif.org Our viewer.ohif.org is deployed from release branch while viewer-dev.ohif.org is our master branch Read more about branch explanations here https://docs.ohif.org/development/getting-started#developing

Please try and re-open if the issue persists