DICOM read is broken for many files with 6.9.x and 7.x

[jcupitt]

Hi, it looks like the refactoring of dcm pixel reads last year has broken dicom read for many files. Here's a sample image:

http://www.rollthepotato.net/~john/MRIm5.dcm

It's a 16-bit, one channel MRI Image. You are welcome to use the image in a test suite.

If I examine it with ImageMagick-6.9.8-4 (though IM 7 has the same problem) I see:

Code: Select all

$ identify -verbose MRIm5.dcm 
Image: MRIm5.dcm
  Format: DCM (Digital Imaging and Communications in Medicine image)
  Class: PseudoClass
  Geometry: 200x200+0+0
  Units: Undefined
  Type: Grayscale
  Base type: Grayscale
  Endianess: LSB
  Colorspace: Gray
  Depth: 16-bit
  Channel depth:
    gray: 15-bit
  Channel statistics:
    Pixels: 40000
    Gray:
      min: 32769 (0.500023)
      max: 32769 (0.500023)

ie. it's a uniform grey.

I looked inside dcm.c and the problem seems to be in applying rescale_slope and rescale_intercept. At the line:

Code: Select all

ReadDCMPixels(), line 2880
          index=(pixel_value*info->rescale_slope)+info->rescale_intercept;

pixel_value is correctly read (pixel 0, 0 is 4100), but info->rescale_slope is 0, so you always get index == 0, hence the uniform grey.

This line sets info->rescale_slope:

Code: Select all

ReadDCMImage(), line 3516
              info.rescale_slope=(ssize_t) StringToLong((char *) data);

And for this image, data has the value:

Code: Select all

3516	              info.rescale_slope=(ssize_t) StringToLong((char *) data);
(gdb) p data
$11 = (unsigned char *) 0x555555882a60 "0.154761567 "
(gdb) 

Most DICOM images will have a double for rescale_slope, so ssize_t here must be incorrect.

There seem to me to be two solutions:

1. output a double image and interpret rescale_slope and rescale_intercept
2. output a ushort image and leave applying the scale and intercept to the calling program

I think I would prefer 2., which is the previous behaviour of IM.

[jcupitt]

I could have a go at preparing a patch.

I propose simply not interpreting the scale and intercept. Does that sound acceptable?

[dlemstra]

Wouldn't it make more sense to change the type of rescale_slope and rescale_intercept to a double instead? We do have a StringToDouble method that you could use. And we always welcome patches or pull requests: https://github.com/ImageMagick/ImageMagick/pulls

[jcupitt]

Unfortunately, many DICOM images use fractional units, for example here the unit is t0, relaxation time. If I have a ushort image with rescale_slope == 0.1 and I express this as an int image, I'm immediately losing 10x of my precision. I think if you bake rescale_slope into the image, you must output a double value.

I think IM7 might make this possible since (is this right?) it uses double for all pixel values, but with IM6 I think you must not interpret rescale_slope.

[jcupitt]

It looks like this commit is the cause, not the refactoring I fingered earlier:

https://github.com/ImageMagick/ImageMag ... 4ec0b734dd

"The DICOM reader now handles the rescale intercept and slope." from Nov 2015.

6.9.1-10 works, 6.9.2-10 fails.

[jcupitt]

OK, PR opened:

https://github.com/ImageMagick/ImageMagick/pull/477