Fixed bug in handling of pixel data representations.

Compression and decompression codecs that may modify the DICOM dataset such that the pixel data before the coding process is not consistent anymore with the DICOM dataset after the coding (e.g. because planar configuration or photometric interpretation may have changed) now remove the previous in-memory pixel data representation. Unfortunately this requires an API change in DcmCodec::encode() and DcmCodec::decode() that must be implemented by all image compression codecs. Thanks to Peter Klotz <[email protected]> for the bug report. This closes DCMTK issue #845.
daudrain · May 27, 2020 · 0e8fae8 · 0e8fae8
1 parent c1a32df
commit 0e8fae8
Show file tree

Hide file tree

Showing 15 changed files with 241 additions and 63 deletions.
diff --git a/dcmdata/include/dcmtk/dcmdata/dccodec.h b/dcmdata/include/dcmtk/dcmdata/dccodec.h
@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 1997-2018, OFFIS e.V.
+ *  Copyright (C) 1997-2020, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -92,14 +92,19 @@ class DCMTK_DCMDATA_EXPORT DcmCodec
    *  @param cp codec parameters for this codec
    *  @param objStack stack pointing to the location of the pixel data
    *    element in the current dataset.
+   *  @param removeOldRep boolean flag that should be set to false before this method call
+   *    and will be set to true if the codec modifies the DICOM dataset such
+   *    that the pixel data of the original representation may not be usable
+   *    anymore.
    *  @return EC_Normal if successful, an error code otherwise.
    */
   virtual OFCondition decode(
     const DcmRepresentationParameter * fromRepParam,
     DcmPixelSequence * pixSeq,
     DcmPolymorphOBOW& uncompressedPixelData,
     const DcmCodecParameter * cp,
-    const DcmStack& objStack) const = 0;
+    const DcmStack & objStack,
+    OFBool& removeOldRep) const = 0;
 
   /** decompresses a single frame from the given pixel sequence and
    *  stores the result in the given buffer.
@@ -149,6 +154,10 @@ class DCMTK_DCMDATA_EXPORT DcmCodec
    *  @param cp codec parameters for this codec
    *  @param objStack stack pointing to the location of the pixel data
    *    element in the current dataset.
+   *  @param removeOldRep boolean flag that should be set to false before this method call
+   *    and will be set to true if the codec modifies the DICOM dataset such
+   *    that the pixel data of the original representation may not be usable
+   *    anymore.
    *  @return EC_Normal if successful, an error code otherwise.
    */
   virtual OFCondition encode(
@@ -157,7 +166,8 @@ class DCMTK_DCMDATA_EXPORT DcmCodec
     const DcmRepresentationParameter * toRepParam,
     DcmPixelSequence * & pixSeq,
     const DcmCodecParameter *cp,
-    DcmStack & objStack) const = 0;
+    DcmStack & objStack,
+    OFBool& removeOldRep) const = 0;
 
   /** transcodes (re-compresses) the given compressed DICOM image and stores
    *  the result in the given toPixSeq element.
@@ -171,6 +181,10 @@ class DCMTK_DCMDATA_EXPORT DcmCodec
    *  @param cp codec parameters for this codec
    *  @param objStack stack pointing to the location of the pixel data
    *    element in the current dataset.
+   *  @param removeOldRep boolean flag that should be set to false before this method call
+   *    and will be set to true if the codec modifies the DICOM dataset such
+   *    that the pixel data of the original representation may not be usable
+   *    anymore.
    *  @return EC_Normal if successful, an error code otherwise.
    */
   virtual OFCondition encode(
@@ -180,7 +194,8 @@ class DCMTK_DCMDATA_EXPORT DcmCodec
     const DcmRepresentationParameter * toRepParam,
     DcmPixelSequence * & toPixSeq,
     const DcmCodecParameter * cp,
-    DcmStack & objStack) const = 0;
+    DcmStack & objStack,
+    OFBool& removeOldRep) const = 0;
 
   /** checks if this codec is able to convert from the
    *  given current transfer syntax to the given new
@@ -345,14 +360,19 @@ class DCMTK_DCMDATA_EXPORT DcmCodecList
    *  @param uncompressedPixelData uncompressed pixel data stored in this element
    *  @param pixelStack stack pointing to the location of the pixel data
    *    element in the current dataset.
+   *  @param removeOldRep boolean flag that should be set to false before this method call
+   *    and will be set to true if the codec modifies the DICOM dataset such
+   *    that the pixel data of the original representation may not be usable
+   *    anymore.
    *  @return EC_Normal if successful, an error code otherwise.
    */
   static OFCondition decode(
     const DcmXfer & fromType,
     const DcmRepresentationParameter * fromParam,
     DcmPixelSequence * fromPixSeq,
     DcmPolymorphOBOW& uncompressedPixelData,
-    DcmStack & pixelStack);
+    DcmStack & pixelStack,
+    OFBool& removeOldRep);
 
   /** looks for a codec that is able to decode from the given transfer syntax
    *  and calls the decodeFrame() method of the codec.  A read lock on the list of
@@ -405,6 +425,10 @@ class DCMTK_DCMDATA_EXPORT DcmCodecList
    *    allocated on heap) returned in this parameter upon success.
    *  @param pixelStack stack pointing to the location of the pixel data
    *    element in the current dataset.
+   *  @param removeOldRep boolean flag that should be set to false before this method call
+   *    and will be set to true if the codec modifies the DICOM dataset such
+   *    that the pixel data of the original representation may not be usable
+   *    anymore.
    *  @return EC_Normal if successful, an error code otherwise.
    */
   static OFCondition encode(
@@ -414,7 +438,8 @@ class DCMTK_DCMDATA_EXPORT DcmCodecList
     const E_TransferSyntax toRepType,
     const DcmRepresentationParameter * toRepParam,
     DcmPixelSequence * & pixSeq,
-    DcmStack & pixelStack);
+    DcmStack & pixelStack,
+    OFBool& removeOldRep);
 
   /** looks for a codec that is able to transcode (re-compresses)
    *  from the given transfer syntax to the given transfer syntax
@@ -431,6 +456,10 @@ class DCMTK_DCMDATA_EXPORT DcmCodecList
    *    allocated on heap) returned in this parameter upon success.
    *  @param pixelStack stack pointing to the location of the pixel data
    *    element in the current dataset.
+   *  @param removeOldRep boolean flag that should be set to false before this method call
+   *    and will be set to true if the codec modifies the DICOM dataset such
+   *    that the pixel data of the original representation may not be usable
+   *    anymore.
    *  @return EC_Normal if successful, an error code otherwise.
    */
   static OFCondition encode(
@@ -440,7 +469,8 @@ class DCMTK_DCMDATA_EXPORT DcmCodecList
     const E_TransferSyntax toRepType,
     const DcmRepresentationParameter * toRepParam,
     DcmPixelSequence * & toPixSeq,
-    DcmStack & pixelStack);
+    DcmStack & pixelStack,
+    OFBool& removeOldRep);
 
   /** looks for a codec that claims to be able to convert
    *  between the given transfer syntaxes.

diff --git a/dcmdata/include/dcmtk/dcmdata/dcrleccd.h b/dcmdata/include/dcmtk/dcmdata/dcrleccd.h
@@ -47,14 +47,19 @@ class DCMTK_DCMDATA_EXPORT DcmRLECodecDecoder: public DcmCodec
    *  @param cp codec parameters for this codec
    *  @param objStack stack pointing to the location of the pixel data
    *    element in the current dataset.
+   *  @param removeOldRep boolean flag that should be set to false before this method call
+   *    and will be set to true if the codec modifies the DICOM dataset such
+   *    that the pixel data of the original representation may not be usable
+   *    anymore.
    *  @return EC_Normal if successful, an error code otherwise.
    */
   virtual OFCondition decode(
     const DcmRepresentationParameter * fromRepParam,
     DcmPixelSequence * pixSeq,
     DcmPolymorphOBOW& uncompressedPixelData,
     const DcmCodecParameter * cp,
-    const DcmStack& objStack) const;
+    const DcmStack & objStack,
+    OFBool& removeOldRep) const;
 
   /** decompresses a single frame from the given pixel sequence and
    *  stores the result in the given buffer.
@@ -104,6 +109,10 @@ class DCMTK_DCMDATA_EXPORT DcmRLECodecDecoder: public DcmCodec
    *  @param cp codec parameters for this codec
    *  @param objStack stack pointing to the location of the pixel data
    *    element in the current dataset.
+   *  @param removeOldRep boolean flag that should be set to false before this method call
+   *    and will be set to true if the codec modifies the DICOM dataset such
+   *    that the pixel data of the original representation may not be usable
+   *    anymore.
    *  @return EC_Normal if successful, an error code otherwise.
    */
   virtual OFCondition encode(
@@ -112,7 +121,8 @@ class DCMTK_DCMDATA_EXPORT DcmRLECodecDecoder: public DcmCodec
     const DcmRepresentationParameter * toRepParam,
     DcmPixelSequence * & pixSeq,
     const DcmCodecParameter *cp,
-    DcmStack & objStack) const;
+    DcmStack & objStack,
+    OFBool& removeOldRep) const;
 
   /** transcodes (re-compresses) the given compressed DICOM image and stores
    *  the result in the given toPixSeq element.
@@ -126,6 +136,10 @@ class DCMTK_DCMDATA_EXPORT DcmRLECodecDecoder: public DcmCodec
    *  @param cp codec parameters for this codec
    *  @param objStack stack pointing to the location of the pixel data
    *    element in the current dataset.
+   *  @param removeOldRep boolean flag that should be set to false before this method call
+   *    and will be set to true if the codec modifies the DICOM dataset such
+   *    that the pixel data of the original representation may not be usable
+   *    anymore.
    *  @return EC_Normal if successful, an error code otherwise.
    */
   virtual OFCondition encode(
@@ -135,7 +149,8 @@ class DCMTK_DCMDATA_EXPORT DcmRLECodecDecoder: public DcmCodec
     const DcmRepresentationParameter * toRepParam,
     DcmPixelSequence * & toPixSeq,
     const DcmCodecParameter * cp,
-    DcmStack & objStack) const;
+    DcmStack & objStack,
+    OFBool& removeOldRep) const;
 
   /** checks if this codec is able to convert from the
    *  given current transfer syntax to the given new

diff --git a/dcmdata/include/dcmtk/dcmdata/dcrlecce.h b/dcmdata/include/dcmtk/dcmdata/dcrlecce.h
@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 2002-2011, OFFIS e.V.
+ *  Copyright (C) 2002-2020, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -49,14 +49,19 @@ class DCMTK_DCMDATA_EXPORT DcmRLECodecEncoder: public DcmCodec
    *  @param cp codec parameters for this codec
    *  @param objStack stack pointing to the location of the pixel data
    *    element in the current dataset.
+   *  @param removeOldRep boolean flag that should be set to false before this method call
+   *    and will be set to true if the codec modifies the DICOM dataset such
+   *    that the pixel data of the original representation may not be usable
+   *    anymore.
    *  @return EC_Normal if successful, an error code otherwise.
    */
   virtual OFCondition decode(
     const DcmRepresentationParameter * fromRepParam,
     DcmPixelSequence * pixSeq,
     DcmPolymorphOBOW& uncompressedPixelData,
     const DcmCodecParameter * cp,
-    const DcmStack& objStack) const;
+    const DcmStack & objStack,
+    OFBool& removeOldRep) const;
 
   /** decompresses a single frame from the given pixel sequence and
    *  stores the result in the given buffer.
@@ -106,6 +111,10 @@ class DCMTK_DCMDATA_EXPORT DcmRLECodecEncoder: public DcmCodec
    *  @param cp codec parameters for this codec
    *  @param objStack stack pointing to the location of the pixel data
    *    element in the current dataset.
+   *  @param removeOldRep boolean flag that should be set to false before this method call
+   *    and will be set to true if the codec modifies the DICOM dataset such
+   *    that the pixel data of the original representation may not be usable
+   *    anymore.
    *  @return EC_Normal if successful, an error code otherwise.
    */
   virtual OFCondition encode(
@@ -114,7 +123,8 @@ class DCMTK_DCMDATA_EXPORT DcmRLECodecEncoder: public DcmCodec
     const DcmRepresentationParameter * toRepParam,
     DcmPixelSequence * & pixSeq,
     const DcmCodecParameter *cp,
-    DcmStack & objStack) const;
+    DcmStack & objStack,
+    OFBool& removeOldRep) const;
 
   /** transcodes (re-compresses) the given compressed DICOM image and stores
    *  the result in the given toPixSeq element.
@@ -128,6 +138,10 @@ class DCMTK_DCMDATA_EXPORT DcmRLECodecEncoder: public DcmCodec
    *  @param cp codec parameters for this codec
    *  @param objStack stack pointing to the location of the pixel data
    *    element in the current dataset.
+   *  @param removeOldRep boolean flag that should be set to false before this method call
+   *    and will be set to true if the codec modifies the DICOM dataset such
+   *    that the pixel data of the original representation may not be usable
+   *    anymore.
    *  @return EC_Normal if successful, an error code otherwise.
    */
   virtual OFCondition encode(
@@ -137,7 +151,8 @@ class DCMTK_DCMDATA_EXPORT DcmRLECodecEncoder: public DcmCodec
     const DcmRepresentationParameter * toRepParam,
     DcmPixelSequence * & toPixSeq,
     const DcmCodecParameter * cp,
-    DcmStack & objStack) const;
+    DcmStack & objStack,
+    OFBool& removeOldRep) const;
 
   /** checks if this codec is able to convert from the
    *  given current transfer syntax to the given new

diff --git a/dcmdata/libsrc/dccodec.cc b/dcmdata/libsrc/dccodec.cc
@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 1997-2016, OFFIS e.V.
+ *  Copyright (C) 1997-2020, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -434,7 +434,8 @@ OFCondition DcmCodecList::decode(
   const DcmRepresentationParameter * fromParam,
   DcmPixelSequence * fromPixSeq,
   DcmPolymorphOBOW& uncompressedPixelData,
-  DcmStack & pixelStack)
+  DcmStack & pixelStack,
+  OFBool& removeOldRep)
 {
 #ifdef WITH_THREADS
   if (! codecLock.initialized()) return EC_IllegalCall; // should never happen
@@ -454,7 +455,7 @@ OFCondition DcmCodecList::decode(
     {
       if ((*first)->codec->canChangeCoding(fromXfer, EXS_LittleEndianExplicit))
       {
-        result = (*first)->codec->decode(fromParam, fromPixSeq, uncompressedPixelData, (*first)->codecParameter, pixelStack);
+        result = (*first)->codec->decode(fromParam, fromPixSeq, uncompressedPixelData, (*first)->codecParameter, pixelStack, removeOldRep);
         first = last;
       } else ++first;
     }
@@ -513,7 +514,8 @@ OFCondition DcmCodecList::encode(
   const E_TransferSyntax toRepType,
   const DcmRepresentationParameter * toRepParam,
   DcmPixelSequence * & toPixSeq,
-  DcmStack & pixelStack)
+  DcmStack & pixelStack,
+  OFBool& removeOldRep)
 {
   toPixSeq = NULL;
 #ifdef WITH_THREADS
@@ -535,7 +537,7 @@ OFCondition DcmCodecList::encode(
       {
         if (!toRepParam) toRepParam = (*first)->defaultRepParam;
         result = (*first)->codec->encode(fromRepType, fromParam, fromPixSeq,
-                 toRepParam, toPixSeq, (*first)->codecParameter, pixelStack);
+                 toRepParam, toPixSeq, (*first)->codecParameter, pixelStack, removeOldRep);
         first = last;
       } else ++first;
     }
@@ -553,7 +555,8 @@ OFCondition DcmCodecList::encode(
   const E_TransferSyntax toRepType,
   const DcmRepresentationParameter * toRepParam,
   DcmPixelSequence * & toPixSeq,
-  DcmStack & pixelStack)
+  DcmStack & pixelStack,
+  OFBool& removeOldRep)
 {
   toPixSeq = NULL;
 #ifdef WITH_THREADS
@@ -575,7 +578,7 @@ OFCondition DcmCodecList::encode(
       {
         if (!toRepParam) toRepParam = (*first)->defaultRepParam;
         result = (*first)->codec->encode(pixelData, length, toRepParam, toPixSeq,
-                 (*first)->codecParameter, pixelStack);
+                 (*first)->codecParameter, pixelStack, removeOldRep);
         first = last;
       } else ++first;
     }

diff --git a/dcmdata/libsrc/dcpixel.cc b/dcmdata/libsrc/dcpixel.cc
@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 1997-2019, OFFIS e.V.
+ *  Copyright (C) 1997-2020, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -442,13 +442,20 @@ DcmPixelData::decode(
     DcmStack & pixelStack)
 {
     if (existUnencapsulated) return EC_Normal;
-    OFCondition l_error = DcmCodecList::decode(fromType, fromParam, fromPixSeq, *this, pixelStack);
+    OFBool removeOldPixelRepresentation = OFFalse;
+    OFCondition l_error = DcmCodecList::decode(fromType, fromParam, fromPixSeq, *this, pixelStack, removeOldPixelRepresentation);
     if (l_error.good())
     {
         existUnencapsulated = OFTrue;
         current = repListEnd;
         setVR(EVR_OW);
         recalcVR();
+
+        // the codec has indicated that the image pixel module has been modified
+        // in a way that may affect the validity of the old representation of pixel data.
+        // Thus, we cannot just switch back to the old representation.
+        // Thus, remove old representation(s).
+        if (removeOldPixelRepresentation) removeAllButCurrentRepresentations();
     }
     else
     {
@@ -473,10 +480,11 @@ DcmPixelData::encode(
     if (toType.isEncapsulated())
     {
        DcmPixelSequence * toPixSeq = NULL;
+       OFBool removeOldPixelRepresentation = OFFalse;
        if (fromType.isEncapsulated())
        {
          l_error = DcmCodecList::encode(fromType.getXfer(), fromParam, fromPixSeq,
-                   toType.getXfer(), toParam, toPixSeq, pixelStack);
+                   toType.getXfer(), toParam, toPixSeq, pixelStack, removeOldPixelRepresentation);
        }
        else
        {
@@ -486,7 +494,7 @@ DcmPixelData::encode(
          if (l_error == EC_Normal)
          {
            l_error = DcmCodecList::encode(fromType.getXfer(), pixelData, length,
-                     toType.getXfer(), toParam, toPixSeq, pixelStack);
+                     toType.getXfer(), toParam, toPixSeq, pixelStack, removeOldPixelRepresentation);
          }
        }
 
@@ -495,6 +503,11 @@ DcmPixelData::encode(
            current = insertRepresentationEntry(
              new DcmRepresentationEntry(toType.getXfer(), toParam, toPixSeq));
            recalcVR();
+           // the codec has indicated that the image pixel module has been modified
+           // in a way that may affect the validity of the old representation of pixel data.
+           // Thus, we cannot just switch back to the old representation, but have
+           // to actually decode in this case. Thus, remove old representation(s).
+           if (removeOldPixelRepresentation) removeAllButCurrentRepresentations();
        } else delete toPixSeq;
 
        // if it was possible to convert one encapsulated syntax into