BUG: Properly handle JPEGBaseline1 DICOM files

[malaterre]

A bug was introduced in :

c074bc5 BUG: Always convert YBR_FULL to RGB

The ImageChangePhotometricInterpretation filter is too simple and does not work
on compressed stream. Add an explicit decompression step to make sure
decompression from YBR_FULL_422 to YBR_FULL (JPEGBaseline1) happen in the
background.

Closes #1308.

PR Checklist

• 🚫 Makes breaking changes
• 🚫 Makes design changes
• 🚫 Adds the License notice to new files.
• 🚫 Adds Python wrapping.
• 🚫 Adds tests and baseline comparison (quantitative).
• 🚫 Adds test data.
• 🚫 Adds Examples to ITKExamples
• 🚫 Adds Documentation.

Refer to the ITK Software Guide for
further development details if necessary.

[malaterre]

@issakomi This is a bugfix for JPEGBaseline1, thanks for the report.

[issakomi]

FYI, PR

[blowekamp]

@malaterre That you VERY much for correcting this code to work for this case.

I am giving this a -1 because testing is required. We must add a couple test images and test to ensure ITK is handles these image types as expected and reports them as RGB_PIXEL type and the image is in RGB color space.

Are there a couple very small DCM images available for these cases? I'll be happy to add a the tests to this PR.

[malaterre]

@blowekamp @issakomi @dzenanz @thewtex Do not merge this PR until the discussion is done at:

• https://itk.org/Wiki/ITK/Release_5/DICOM/Color#Discussion

I could not make use of discourse since it would not let me edit my own post (!)

[malaterre]

@blowekamp Thanks much for your kind help. I do not know how to inject a DICOM file into ITKData these days anymore.

I would suggest this one:

• https://sourceforge.net/p/gdcm/gdcmdata/ci/master/tree/US-GE-4AICL142.dcm

Because this is part of GDCM regular test suite, maybe @issakomi could suggest another JPEGBaseline1 DICOM file, if you prefer.

[dzenanz]

@blowekamp You said you wanted to add a test. Any progress there? We should merge either this or #1309 soon.

#1309 seems to be a super-set of this, functionality-wise. Also, @issakomi made quite some effort to improve his style and reuse existing code when possible. So I prefer #1309.

@zivy @thewtex @hjmjohnson @N-Dekker @jhlegarreta does anyone else want to pitch in?

[blowekamp]

@blowekamp You said you wanted to add a test. Any progress there? We should merge either this or #1309 soon.

I was waiting form some type of decision or conclusion on the requirements or expected behavior from here: https://discourse.itk.org/t/dicom-color-support/2320

[dzenanz]

I guess YCbCr encoded DICOMs are rare, so not many people have opinion about it. I didn't encounter them in practice. So my mild preference is to convert them to RGB by default, and have an option to read them as-is into a vector3 image.

[jhlegarreta]

I have followed the conversation somehow loosely. I have not personally had to deal with color DICOM images, or to a very little extent, at least.

As long as the conversion is justified, well-documented, and meaningful tests and examples, or counter-examples if this last makes sense, I am fine with what you have decided is best.

The reasoning in the GitHub issues and PR comments and the wiki section look sound enough to me to go for this.

I am in favor of providing a compromise solution, even if not perfect, rather than not supporting YBR_FULL. That lays the ground for future improvements.

[issakomi]

That implicit TS filter seems to have side effects, i mean not only the overheat. I've tested it a little - this file PrivateGEImplicitVRBigEndianTransferSyntax16Bits.dcm (not a color file) looks different before and after. I didn't look closer why. Sorry, but #1309 doesn't have any side effects and applies only to YBR_FULL and YBR_FULL_422 files, it doesn't touch other files like this PR.

PS
Many viewers (Inobitec, PixelMed, Weasis) can not open that file at all (private transfer syntax), some viewers can (Aesulap, Aliza, MicroDicom,TomoVision). So it is (or was) an advantage of GDCM IO.

[blowekamp]

This is great you are testing these file locally and it is outstanding we are getting robust support for these format! 👏 But we really need to add some ITK tests for these formats. This PR appeared to just add support for the one more format which I had a little familiarity. The PR #1309 adds a lot more support and features which is outstanding, but should be tested in ITK for the cases you are doing locally.

I suggest writing a new test which reads a single DICOM file and writes it out as a mha ( or another arbitrary format), then does a comparison to a baseline image. The input test data and the baseline can be added to the subdirectories of the testing directory by following the instructions here. Small images are good.

[dzenanz]

@blowekamp I guess either you or me will have to add a unit test for this. How are you doing with time?

[blowekamp]

I can write the new test with the options for #1309 on Monday. But I'll need the help of the community to assemble a proper set of small test images for the cases.

[issakomi]

About sample images s. this post

[malaterre]

That implicit TS filter seems to have side effects, i mean not only the overheat. I've tested it a little - this file PrivateGEImplicitVRBigEndianTransferSyntax16Bits.dcm (not a color file) looks different before and after. I didn't look closer why. Sorry, but #1309 doesn't have any side effects and applies only to YBR_FULL and YBR_FULL_422 files, it doesn't touch other files like this PR.

PS
Many viewers (Inobitec, PixelMed, Weasis) can not open that file at all (private transfer syntax), some viewers can (Aesulap, Aliza, MicroDicom,TomoVision). So it is (or was) an advantage of GDCM IO.

I cannot reproduce the behavior you describe using ImageReadWrite:

$ bin/ImageReadWrite ../gdcm/Testing/Data/PrivateGEImplicitVRBigEndianTransferSyntax16Bits.dcm o.dcm

Can you elaborate a bit on which filter you used ?

[malaterre]

@blowekamp As a side note, this is possible to generate a color DICOM from an existing JPEG or JPEG 2000 file (eg. ExternalData/Testing/Data/Input/closerec1.jpg or ExternalData/Modules/IO/JPEG2000/test/Input/Bretagne1.j2k). I did not continue this path since I could not find the PNG Baseline for those.

[issakomi]

I cannot reproduce the behavior you describe using ImageReadWrite:

$ bin/ImageReadWrite ../gdcm/Testing/Data/PrivateGEImplicitVRBigEndianTransferSyntax16Bits.dcm o.dcm

Can you elaborate a bit on which filter you used ?

I've compiled this branch (PR) and just opened the image with ITK reader.

[malaterre]

I cannot reproduce the behavior you describe using ImageReadWrite:
$ bin/ImageReadWrite ../gdcm/Testing/Data/PrivateGEImplicitVRBigEndianTransferSyntax16Bits.dcm o.dcm
Can you elaborate a bit on which filter you used ?

I've compiled this branch (PR) and just opened the image with ITK reader.

Thanks ! You were actually right, sorry about that. This is fixed in upstream GDCM: d99cda0cd76608f32e0df4c2541cf97f237341b7

[malaterre]

I can write the new test with the options for #1309 on Monday. But I'll need the help of the community to assemble a proper set of small test images for the cases.

@blowekamp

Feel free to ping. There are some possible smart choices to re-use either existing files (TIFF/JPEG/JPEG 2000) and or derive one from the other (eg. RAW/YBR_FULL_422 can be converted to JPEG 8bits), this should save a bit of disk space (if this make sense)

[blowekamp]

I can write the new test with the options for #1309 on Monday. But I'll need the help of the community to assemble a proper set of small test images for the cases.

@blowekamp

Feel free to ping. There are some possible smart choices to re-use either existing files (TIFF/JPEG/JPEG 2000) and or derive one from the other (eg. RAW/YBR_FULL_422 can be converted to JPEG 8bits), this should save a bit of disk space (if this make sense)

The process of uploading data is easy and well documented here: https://github.com/InsightSoftwareConsortium/ITK/blob/master/Documentation/Data.md

I plan on writing a general test this afternoon that can be reused with different data sets. I am not familiar with the all the data requirements and options. I hope those who have data, and know how to generate small data sets, or reuses appropriate existing datasets can add the required tests cases to verify ITK having robust DICOM support with GDCM.

Thanks ! You were actually right, sorry about that. This is fixed in upstream GDCM: d99cda0cd76608f32e0df4c2541cf97f237341b7

Does ITK's GDCM need to be updated?

[malaterre]

@blowekamp After trying for 15min to understand the instructions. I am giving up on trying to generate a data file for unit test, the instructions are just not clear for me.

Generate the data:

$ dcmcjpeg --sample-422 --quality 100 --encode-baseline RGBDicomTest.dcm JPEGBaseline1DicomTest.dcm

The test should be just a matter of:

diff --git a/Modules/IO/GDCM/test/CMakeLists.txt b/Modules/IO/GDCM/test/CMakeLists.txt
index 14eaca1ecd..d6913bd2f8 100644
--- a/Modules/IO/GDCM/test/CMakeLists.txt
+++ b/Modules/IO/GDCM/test/CMakeLists.txt
@@ -173,3 +173,12 @@ itk_add_test(NAME itkGDCMImageReadWriteTest_RGB
       DATA{${ITK_DATA_ROOT}/Input/RGBDicomTest.dcm}
       ${ITK_TEST_OUTPUT_DIR}/itkGDCMImageReadWriteTest_RGB.mha
       rgb)
+
+itk_add_test(NAME itkGDCMImageReadWriteTest_JPEGBaseline1
+  COMMAND ITKIOGDCMTestDriver
+    --compare ${ITK_TEST_OUTPUT_DIR}/itkGDCMImageReadWriteTest_JPEGBaseline1.mha
+      DATA{Baseline/itkGDCMImageReadWriteTest_RGB.mha}
+    itkGDCMImageReadWriteTest
+      DATA{${ITK_DATA_ROOT}/Input/JPEGBaseline1DicomTest.dcm}
+      ${ITK_TEST_OUTPUT_DIR}/itkGDCMImageReadWriteTest_JPEGBaseline1.mha
+      rgb)

I used --quality 100 so as to reuse the same baseline but I was unable to even run the test to verify.

If I cannot get someone to upload a JPEGBaseline1/DICOM file for me, feel free to close this PR. Thanks

[seanm]

CC @seanm @vfonov

[dzenanz]

As #1309 was merged, closing this PR which has conflicting changes.