I think I may have found a bug in the dicom reader. It won't
SetImageProperty() for any tags in groups 0x2 and 0x28 because the
code to set properties is in the default: block of the group handler.
This is a problem for me because I need fields RescaleSlope and
RescaleIntercept, both in group 0x28, and both ignored by dcm.c.

More detail: the switch() starting at line 2942 in dcm.c in
ImageMagick 6.3.7 looks something like this:

  switch(group){
  case 0x2:
    // set the transfer syntax
    break;
  case 0x28:
    // set things like width, height, etc.
    break;
  default:
    // attach the tag as an image property
  }

So tags such as (0x0028,0x1052) "Rescale Intercept" are never set as
properties. Anything in 0x2 will also be ignored, I don't know if
that's a problem.

Here's a patch that does the simplest possible change -- just moving
the default: handler outside the switch. Now you get properties set
for everything (including width, height, etc.). If you don't want to
set properties for things that magick represents by other header
fields I guess you'd need to put the attach-as-property code into a
function and call it for the default action for all nested cases.

---------------------------------
*** dcm.old     2007-11-10 02:34:33.000000000 +0000
--- dcm.c       2007-11-23 14:53:47.000000000 +0000
***************
*** 3124,3134 ****
        }
        default:
        {
          char
            *attribute;

-         if (data == (unsigned char *) NULL)
-           break;
          for (i=0; dicom_info[i].description != (char *) NULL; i++)
            if ((group == (long) dicom_info[i].group) &&
                (element == (long) dicom_info[i].element))
--- 3124,3137 ----
        }
        default:
        {
+         break;
+       }
+     }
+     if (data != (unsigned char *) NULL)
+       {
          char
            *attribute;

          for (i=0; dicom_info[i].description != (char *) NULL; i++)
            if ((group == (long) dicom_info[i].group) &&
                (element == (long) dicom_info[i].element))
***************
*** 3138,3150 ****
          for (i=0; i < (long) MagickMax(length,4); i++)
            if (isprint((int) data[i]) == MagickFalse)
              break;
!         if ((i != (long) length) && (length <= 4))
!           break;
!         (void) SubstituteString(&attribute," ","");
!         (void) SetImageProperty(image,attribute,(char *) data);
!         break;
        }
-     }
      if (image_info->verbose != MagickFalse)
        {
          if (data == (unsigned char *) NULL)
--- 3141,3152 ----
          for (i=0; i < (long) MagickMax(length,4); i++)
            if (isprint((int) data[i]) == MagickFalse)
              break;
!         if ((i == (long) length) || (length > 4))
!         {
!             (void) SubstituteString(&attribute," ","");
!             (void) SetImageProperty(image,attribute,(char *) data);
!         }
        }
      if (image_info->verbose != MagickFalse)
        {
          if (data == (unsigned char *) NULL)
---------------------------------

Does this memleak? I'd have thought you'd need a DestroyString(), but
perhaps I'm missing something.

Anyway, with this change I can get RescaleSlope out of the image.

John
_______________________________________________
Magick-developers mailing list
Magick-developers@imagemagick.org
http://studio.imagemagick.org/mailman/listinfo/magick-developers

Reply via email to