ENH: Handle IPP/IOP/PixelSpacing for SC

[issakomi]

This will add support for many data sets, specially Visible Human Project recently released collection.

S. David Clunie's post post

S. InsightSoftwareConsortium/ITK#4109

P.S. The PR intentionally doesn't change writing of SC files.

[issakomi]

One file from the collection is here .

Whole collection can be downloaded with s5cmd utility (~13 GB):
s5cmd --no-sign-request --endpoint-url https://s3.amazonaws.com cp 's3://idc-open-data/4aaf9181-fb6a-4a4c-bf49-d1eb9ed4a385/*' .

[malaterre]

where does that come from ? Image Plane Module is not part of Secondary Capture Image IOD Modules ...

[issakomi]

I know, but if IPP/IOP are in SC there is not an error, but the warning that SC is extended, please look at David Clunie's post. Honestly, I am a little bit surprised too.

r@deb2:~$ dciodvfy 02725f56-c604-4007-8535-16be09b6615b.dcm 
...
SCImage
... 
Warning - Attribute is not present in standard DICOM IOD - (0x0020,0x0032) DS Image Position (Patient) 
Warning - Attribute is not present in standard DICOM IOD - (0x0020,0x0037) DS Image Orientation (Patient) 
... 
Warning - Dicom dataset contains attributes not present in standard DICOM IOD - this is a Standard Extended SOP Class

[malaterre]

full ref here, I cannot find it. Thanks

[issakomi]

https://dicom.nema.org/medical/dicom/current/output/chtml/part03/sect_C.8.6.2.html#table_C.8-25

Include Table 10-10 “Basic Pixel Spacing Calibration Macro Attributes”

---

[issakomi]

Please let me know if the PR is not accepted or just close it. Maybe I will look for another workaround.

[pieper]

Thanks for working on this @issakomi and thank you for the review @malaterre. I agree there are other options if changing GDCM's behavior is not preferred.

[malaterre]

@issakomi I am going to close this PR. Honestly you are trying to hide a data problem using a rather crude hack. I cannot possibly accept a patch which by default will read IOP/IPP for legacy SC.

Instead I woud prefer to a see two PRs:

1. One to implement CP-586 (solely). I find this CP rather confusing honestly; but anyway this is approved CP so why not
2. One to implement support for extended SOP Class. The idea would be an API where programmer could specify a possible extension. Possible extension would be support for IOP/IPP for legacy SC instance, or even Modality LUT for RTDose (why not!)...maybe plenty more ?

As stated before I am not a big fan of (2) above, DICOM is about interoperability format, so I fail to understand use case for (2).

In any case thanks for the communication and patch.

Closing

[dclunie]

@malaterre it would be preferable to reopen this PR and implement it, since the use of the 3D information has been standard for the multi-frame SC objects since CP 600, and I propose to retrofit the same behavior to the traditional single frame IOD as well as clarify the priority of PixelSpacing over NominalScannedPixelSpacing (which has been the case since CP 586).

See the attached proposed CP:
cp_dac625_01_AddImagePlaneModuleToSC.pdf

Our (IDC's) particular use case in the short term is the photographs of the 3D registered cryosections in the visible human collections, but this is a not uncommon pattern for derived 3D from other applications.

[malaterre]

Our (IDC's) particular use case in the short term is the photographs of the 3D registered cryosections in the visible human collections, but this is a not uncommon pattern for derived 3D from other applications.

So just use one the new multi-frame SC objects since they work out of the box without the need to change the DICOM standard...

[dclunie]

No, combining them into multi-frame makes them too large, and it wouldn't be appropriate to use a bunch of single frame instances of the multi-frame SOP Class.

We are going to fix the standard, since lots of folks do this, so you can improve gdcm or not as you see fit, but if not we will have to fork it for ITK and Slicer since we need this functionality.

[malaterre]

No, combining them into multi-frame makes them too large, and it wouldn't be appropriate to use a bunch of single frame instances of the multi-frame SOP Class.

Says who ? WSI have the same issue with multiple MF instances. There is absolutely nothing wrong with writing multi-frame SOP Class with a NumberOfFrames=1.

Here is a 10 line python script to convert your extended SOP class (*) into proper mf instance, that will be valid with all of today implementations (DICOM 2023c).

We are going to fix the standard, since lots of folks do this, so you can improve gdcm or not as you see fit, but if not we will have to fork it for ITK and Slicer since we need this functionality.

This will not be backward compatible ! You'll need to update all of other people code to handle your specific change (osirix, weasis ...). The fact that "lots of folks" do this does not mean you should do it. You are resurecting a dead beast (legacy SC), for no good reason.

You do not "need" this functionality. AFAIK you are simply trying to work around a bug in your rawtodc tool (you never implemented 1.2.840.10008.5.1.4.1.1.7.4 in it did you ?).

(*)

$ cat fix.py
import pydicom
from pydicom.sequence import Sequence
from pydicom.dataset import Dataset

ds = pydicom.dcmread('000198cd-cd88-4760-97d1-21bbea047fff.dcm')
ds.SOPClassUID = '1.2.840.10008.5.1.4.1.1.7.4'
ds.file_meta.MediaStorageSOPClassUID = ds.SOPClassUID
ds.NumberOfFrames = 1
ds.SharedFunctionalGroupsSequence = Sequence([Dataset()])
ds.SharedFunctionalGroupsSequence[0].PlaneOrientationSequence = Sequence([Dataset()])
ds.SharedFunctionalGroupsSequence[0].PlaneOrientationSequence[0].ImageOrientationPatient = ds.ImageOrientationPatient
ds.SharedFunctionalGroupsSequence[0].PixelMeasuresSequence = Sequence([Dataset()])
ds.SharedFunctionalGroupsSequence[0].PixelMeasuresSequence[0].PixelSpacing = ds.PixelSpacing
ds.SharedFunctionalGroupsSequence[0].PixelMeasuresSequence[0].SpacingBetweenSlices = ds.SpacingBetweenSlices

ds.PerFrameFunctionalGroupsSequence = Sequence([Dataset()])
ds.PerFrameFunctionalGroupsSequence[0].PlanePositionSequence = Sequence([Dataset()])
ds.PerFrameFunctionalGroupsSequence[0].PlanePositionSequence[0].ImagePositionPatient = ds.ImagePositionPatient

ds.save_as("test.dcm", write_like_original = True)

Before:

$ gdcminfo 000198cd-cd88-4760-97d1-21bbea047fff.dcm
[...]
Origin: (0,0,0)
Spacing: (1,1,1)
DirectionCosines: (1,0,0,0,1,0)
[...]

After:

$ gdcminfo test.dcm
MediaStorage is 1.2.840.10008.5.1.4.1.1.7.4 [Multi-frame True Color Secondary Capture Image Storage]
[...]
Origin: (0,0,-2192)
Spacing: (0.33,0.33,1)
DirectionCosines: (1,0,0,0,-1,0)
[...]

[malaterre]

We are going to fix the standard, since lots of folks do this, so you can improve gdcm or not as you see fit, but if not we will have to fork it for ITK and Slicer since we need this functionality.

Philips has been using Modality LUT for legacy MR Instance for years, you did not "fix" the standard just because "lots of folks do this" !

[malaterre]

For later reference, I cannot click on "re-open" button

[issakomi]

For later reference, I cannot click on "re-open" button

Here is the branch, there are two commits now, one for spacing and another for IPP/IOP.

master...issakomi:GDCM:sc1

... support for extended SOP Class. The idea would be an API where programmer could specify a possible extension.

Let me know if you wish a static variable like eg bool ImageHelper::ForceRescaleInterceptSlope for IPP/IOP. Not sure, but perhaps you meant something like this.

There is no PR yet, but it is easy to open.

[thewtex]

@malaterre @issakomi @pieper @dclunie thank you for the tremendous effort taken towards this issue, code-wise, standard-wise, and community-wise.

We all want:

• Open standards
• Interoperability
• Support for crucial metadata like spacing
• Backwards compatibility
• Adoption across the community for expressed DICOM needs
• Functionality upstreamed in open source libraries like GDCM

These goals are extremely impactful. They are also difficult to achieve. And in reality we often have to compromise because "all the stars do not align".

Mathieu, your points are valid. Still, the community wants additional improved DICOM spatial metadata support for derived data. And GDCM is an important part of the community.

Let me know if you wish a static variable like eg bool ImageHelper::ForceRescaleInterceptSlope for IPP/IOP. Not sure, but perhaps you meant something like this.

There is no PR yet, but it is easy to open.

@malaterre do you agree that a flag like this is not ideal? But it is a good path forward? If @issakomi puts more time into this and opens the PR, will you merge it?

[thewtex]

As suggested by @issakomi based on feedback from @malaterre , I added a commit that makes the IPP/IOP opt-in and re-opened here: #167

[dclunie]

FYI, this behavior (the optional presence of the ImagePlane Module and accompanying note about relationship of spacing attributes) is now standardized.