Regression for the conversion of a file with redundant Patient Position entry

[bthyreau]

The attached file is a (part of) DICOM image which fails to convert correctly in current dcm2niix version, but used to work in older versions. The image comes from a Philips scanner, and some PACS somewhere probably added a private section with a redundant field which triggers the errors.

When converted with current dcm2niix, a set of single-slices nifti files is output, which is not good. When converted from an old version (eg. v1.0.20181125) or from mcverter, a single volumetric file slices is created, which is the expected result. (The attached files contains only 3 top slices extracted from a real T1 brain volume, with privacy fields removed)
shortvolume.tar.gz

The failure for this appeared since commit e6772de

Author: Chris Rorden <rorden@mailbox.sc.edu>
Date:   Wed Feb 20 16:45:56 2019 -0500

    New "-x i" option (https://www.nitrc.org/forum/forum.php?thread_id=9324&forum_id=4703)

 console/main_console.cpp    |  7 +++++--
 console/nii_dicom.cpp       | 19 +++++++++++++++++--
 console/nii_dicom.h         |  2 +-
 console/nii_dicom_batch.cpp |  5 +++--
 console/nii_dicom_batch.h   |  2 +-
 5 files changed, 27 insertions(+), 8 deletions(-)
bisect run success

In particular, the change happened when this got commented-out

console/nii_dicom.cpp:
    /*
    fixed 2/2019 by modifying to kDiffusionBFactor, kDiffusionDirectionRL, kDiffusionDirectionAP, kDiffusionDirectionFH
    if ((d.xyzDim[3] == 1) && (numDimensionIndexValues < 1) && (d.manufacturer == kMANUFACTURER_PHILIPS) && (B0Philips >= 0.0)) {
    	//Special case: old Philips Classic DWI storing vectors in 0019,10bb, 0019,10bc
    	//printf(">>>>%g  %g %g %g\n",B0Philips, vRLPhilips, vAPPhilips, vFHPhilips);
    	d.CSA.dtiV[0] = B0Philips;
    	d.CSA.dtiV[1] = vRLPhilips;
    	d.CSA.dtiV[2] = vAPPhilips;
    	d.CSA.dtiV[3] = vFHPhilips;
    	d.CSA.numDti = 1;
	}
	*/

The file is not a Diffusion Image, even though it include fields related to diffusion.

Below is my speculation on what is happening. It happens that "Image Position (Patient)" (0020, 0032) is present twice in the DICOM header, one time being in a subsection, probably added by a PACS or post-processing software.
duplicate:
(0020, 0032) Image Position (Patient) DS: [-89.114439893581, -131.68535648015, 127.929179193095]
And indeed dcm2niix rightfully outputs this warning:
Please check slice thicknesses: Philips R3.2.2 bug can disrupt estimation (2 positions reported for 1 slices)

However, a consequence is that CSA.numDTI is wrongly increased, triggering the misconversion.

It seems that the (0020, 0032) field duplication allows more call to "_update_Tvd" to have the necessary conditions to reach the following block:

        if(!isReady) return;
    // If still here, update dd and *pdti4D.
    ptvd->pdd->CSA.numDti++;

In normal DICOM images, that condition is not triggered often, possibly a direct cause of havingptvd->_isAtFirstPatientPositionset to 0 in most call, while it is often at 1 in the buggy case.

I note that the following change would solve my problem, although i don't know if it offers a valid generic solution:

diff --git a/console/nii_dicom.cpp b/console/nii_dicom.cpp
index 9194a63..5c0238f 100644
--- a/console/nii_dicom.cpp
+++ b/console/nii_dicom.cpp
@@ -5561,6 +5561,8 @@ uint32_t kSequenceDelimitationItemTag = 0xFFFE +(0xE0DD << 16 );
                                }
                                patientPositionNum++;
                                isAtFirstPatientPosition = true;
+                               if (patientPositionNum > 1)
+                                     isAtFirstPatientPosition = false;
                                //char dx[kDICOMStr];
                 //dcmStr(lLength, &buffer[lPos], dx);

[neurolabusc]

Please try out the development branch and see if this resolves your issue. Some Philips systems generate two image position patient (0020,0032) for each slice, one for public use and another in a sequence for private use (despite using a public tag). Handling this has become more challenging as now Philips can create enhanced DICOMs, with many 0020,0032 tags per file, each in a separate sequence. I note that your images omit the manufacturer (0008,0070), which is unfortunate as each manufacturer has interpreted DICOM differently, so recognizing the manufacturer allows us to refine our heuristics. Further, I note that your system reports System 5.4.1, which is much more recent than my previous experiences with this quirk of Philips DICOM. While I hope my kludge is robust, I would suggest you work with your Philips Research Collaboration Manager to understand these features of your images. This atypical usage may confuse other tools.

[bthyreau]

@neurolabusc The fix works well. Thanks you very much for your very fast reply !

Following your clear explanation, I must admit that for privacy reason, i had to manually alter the the DICOM header and probably over-deleted dicom fields, focusing only about triggering the bug. Sorry for wasting your time figuring that out. In particular, the original Dicom did included the (0008, 0070) Manufacturer LO: 'Philips Medical Systems', so the code branches as expected without guessing.

For reference, I attach a (redacted) dump of the full header, according to pydicon
header_edited.txt

Thanks

[neurolabusc]

@bthyreau can I suggest you close this issue, these sequences are handled in the upcoming dcm2niix release.