Fixed issue with DcmCodec::updateImageType().

When compressing a DICOM image, e.g. with JPEG Baseline, the JPEG
encoder calls DcmCodec::updateImageType(), which used to add the
Image Type (0008,0008) Attribute with a possibly invalid value.
For example, compressing a DICOM Secondary Capture image that does
not contain the Image Type (0008,0008) Attribute resulted in an
invalid value of "DERIVED" for the Image Type (0008,0008) Attribute.
This value is invalid, because the VM is "2-n" according to DICOM
PS3.6, i.e. at least two values should be present.

This and other related issues have been fixed by completely rewriting
the updateImageType() method. Also, the API documentation now clearly
states which cases are handled in what way. And, the method reports
details on what is done or what failed to the debug/error logger.

This closes DCMTK Bug #1056.

dcmdata/include/dcmtk/dcmdata/dccodec.h

@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 1997-2022, OFFIS e.V.
+ *  Copyright (C) 1997-2023, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -249,7 +249,7 @@ class DCMTK_DCMDATA_EXPORT DcmCodec
    */
   static OFCondition convertToSecondaryCapture(DcmItem *dataset);
 
-  /** create new SOP instance UID and Source Image Sequence
+  /** create new SOP Instance UID and Source Image Sequence
    *  referencing the old SOP instance (if present)
    *  @param dataset dataset to be modified
    *  @param purposeOfReferenceCodingScheme coding scheme designator for purpose of reference code sequence
@@ -263,7 +263,15 @@ class DCMTK_DCMDATA_EXPORT DcmCodec
     const char *purposeOfReferenceCodeValue = NULL,
     const char *purposeOfReferenceCodeMeaning = NULL);
 
-  /** set first value of Image Type to DERIVED.
+  /** update value of the Image Type element (if needed).
+   *  Three cases are handled by this method:
+   *    1. The Image Type element is not present or has an empty value.
+   *    2. The Image Type element is present and has a single value only.
+   *    3. The Image Type element is present and has two or more values.
+   *
+   *  For case 1, nothing is done. An empty value means "unknown" for Type 2.<br>
+   *  For case 2, the value "DERIVED\SECONDARY" is used (VM is 2-n).<br>
+   *  For case 3, the first value is replaced by the string "DERIVED".
    *  @param dataset dataset to be modified
    *  @return EC_Normal if successful, an error code otherwise
    */

dcmdata/libsrc/dccodec.cc

@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 1997-2020, OFFIS e.V.
+ *  Copyright (C) 1997-2023, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -30,6 +30,7 @@
 #include "dcmtk/dcmdata/dcpixseq.h"  /* for DcmPixelSequence */
 #include "dcmtk/dcmdata/dcpxitem.h"  /* for DcmPixelItem */
 #include "dcmtk/dcmdata/dcswap.h"    /* for swapIfNecessary */
+#include "dcmtk/dcmdata/dcvrcs.h"    /* for DcmCodeString */
 #include "dcmtk/dcmdata/dcvrui.h"    /* for DcmUniqueIdentifier */
 
 // static member variables
@@ -200,27 +201,37 @@ OFCondition DcmCodec::updateImageType(DcmItem *dataset)
 {
   if (dataset == NULL) return EC_IllegalCall;
 
-  DcmStack stack;
-  OFString imageType("DERIVED");
-  OFString a;
-
-  /* find existing Image Type element */
-  OFCondition status = dataset->search(DCM_ImageType, stack, ESM_fromHere, OFFalse);
-  if (status.good())
+  DcmElement *elem = NULL;
+  /* check for the data element (with non-empty value) */
+  if (dataset->findAndGetElement(DCM_ImageType, elem).good() && !elem->isEmpty())
   {
-    DcmElement *elem = OFstatic_cast(DcmElement *, stack.top());
-    unsigned long pos = 1;
-
-    // append old image type information beginning with second entry
-    while ((elem->getOFString(a, pos++)).good())
+    /* case 1: there is a single value only */
+    if (elem->getNumberOfValues() == 1)
     {
-      imageType += "\\";
-      imageType += a;
+      DCMDATA_DEBUG("DcmCodec::updateImageType() setting data element value 'DERIVED\\SECONDARY'");
+      /* overwrite with a valid value (VM=2-n) */
+      return elem->putString("DERIVED\\SECONDARY");
+    } else {
+      OFString elemValue;
+      /* case 2: value 1 is different from "DERIVED" */
+      if (elem->getOFString(elemValue, 0 /*pos*/).good() && (elemValue != "DERIVED"))
+      {
+        if (elem->ident() == EVR_CS)
+        {
+          DCMDATA_DEBUG("DcmCodec::updateImageType() setting data element value 1 to 'DERIVED'");
+          /* overwrite value 1 */
+          return OFstatic_cast(DcmCodeString *, elem)->putOFStringAtPos("DERIVED", 0);
+        } else {
+          /* cannot overwrite value with wrong VR (should never happen) */
+          DCMDATA_ERROR("DcmCodec: Internal ERROR: Cannot update element ImageType " << DCM_ImageType << " with wrong VR");
+          return EC_InvalidVR;
+        }
+      }
     }
   }
 
-  // insert new Image Type, replace old value
-  return dataset->putAndInsertString(DCM_ImageType, imageType.c_str(), OFTrue);
+  /* nothing to do */
+  return EC_Normal;
 }