Fixed frame size calculation for special cases.

DcmElement::getUncompressedFrameSize() now takes into account special cases
such as compressed images where Bits Allocated has a value that is not a
multiple of 8, but is overwritten during decompression anyway. In the case
of compressed images, the active decoder is now looked up and its behavior is
taken into account in the calculation.

This closes DCMTK issue #1140.

dcmdata/include/dcmtk/dcmdata/dccodec.h

@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 1997-2023, OFFIS e.V.
+ *  Copyright (C) 1997-2024, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -208,6 +208,19 @@ class DCMTK_DCMDATA_EXPORT DcmCodec
     const E_TransferSyntax oldRepType,
     const E_TransferSyntax newRepType) const = 0;
 
+  /** determines the effective value of BitsAllocated that a dataset will have
+   *  after decompression of an image with the given values for bitsAllocated
+   *  and bitsStored. This may differ from the bitsAllocated parameter for example
+   *  if that value is not a multiple of 8. Returns zero if an image with the
+   *  given parameters cannot be decoded with this codec.
+   *  @param bitsAllocated current value of Bits Allocated
+   *  @param bitsStored current value of Bits Stored
+   *  @return value of BitsAllocated after decompression, 0 if no decompression possible
+   */
+  virtual Uint16 decodedBitsAllocated(
+    Uint16 bitsAllocated,
+    Uint16 bitsStored) const = 0;
+
   /** determine color model of the decompressed image
    *  @param fromParam representation parameter of current compressed
    *    representation, may be NULL
@@ -510,6 +523,21 @@ class DCMTK_DCMDATA_EXPORT DcmCodecList
     DcmItem *dataset,
     OFString &decompressedColorModel);
 
+  /** determines the effective value of BitsAllocated that a dataset will have
+   *  after decompression of an image with the given values for bitsAllocated
+   *  and bitsStored. This may differ from the bitsAllocated parameter for example
+   *  if that value is not a multiple of 8. Returns zero if an image with the
+   *  given parameters cannot be decoded.
+   *  @param fromType transfer syntax to decode from
+   *  @param bitsAllocated current value of Bits Allocated
+   *  @param bitsStored current value of Bits Stored
+   *  @return value of BitsAllocated after decompression, 0 if no decompression possible
+   */
+  static Uint16 decodedBitsAllocated(
+    const DcmXfer & fromType,
+    Uint16 bitsAllocated,
+    Uint16 bitsStored);
+
 private:
 
   /** constructor

dcmdata/include/dcmtk/dcmdata/dcelem.h

@@ -896,6 +896,19 @@ class DCMTK_DCMDATA_EXPORT DcmElement
     static OFCondition checkVM(const unsigned long vmNum,
                                const OFString &vmStr);
 
+    /** determines the effective value of BitsAllocated that a dataset will have
+     *  after decompression of an image with the given values for bitsAllocated
+     *  and bitsStored. This may differ from the bitsAllocated parameter for example
+     *  if that value is not a multiple of 8. Returns zero if an image with the
+     *  given parameters cannot be decoded.
+     *  @param bitsAllocated current value of Bits Allocated
+     *  @param bitsStored current value of Bits Stored
+     *  @return effective value of BitsAllocated, 0 if no decompression possible
+     */
+    virtual Uint16 decodedBitsAllocated(
+      Uint16 bitsAllocated,
+      Uint16 bitsStored) const;
+
   protected:
 
     /** This function returns this element's value. The returned value corresponds to the

dcmdata/include/dcmtk/dcmdata/dcpixel.h

@@ -180,6 +180,7 @@ class DCMTK_DCMDATA_EXPORT DcmPixelData : public DcmPolymorphOBOW
     /// in write function: pointer to current pixel sequence
     DcmPixelSequence * pixelSeqForWrite;
 
+
     /** check if this element should be written unencapsulated, even though an
      *  encapsulated transfer syntax is used. In other words, this checks if
      *  this pixel data element is on the main level on the dataset or not.
@@ -694,6 +695,20 @@ class DCMTK_DCMDATA_EXPORT DcmPixelData : public DcmPolymorphOBOW
     virtual OFCondition getDecompressedColorModel(
         DcmItem *dataset,
         OFString &decompressedColorModel);
+
+   /** determines the effective value of BitsAllocated that a dataset will have
+     *  after decompression of an image with the given values for bitsAllocated
+     *  and bitsStored. This may differ from the bitsAllocated parameter for example
+     *  if that value is not a multiple of 8. Returns zero if an image with the
+     *  given parameters cannot be decoded.
+     *  @param bitsAllocated current value of Bits Allocated
+     *  @param bitsStored current value of Bits Stored
+     *  @return effective value of BitsAllocated, 0 if no decompression possible
+     */
+    virtual Uint16 decodedBitsAllocated(
+      Uint16 bitsAllocated,
+      Uint16 bitsStored) const;
+
 };
 
 #endif

dcmdata/include/dcmtk/dcmdata/dcrleccd.h

@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 2002-2011, OFFIS e.V.
+ *  Copyright (C) 2002-2024, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -163,6 +163,19 @@ class DCMTK_DCMDATA_EXPORT DcmRLECodecDecoder: public DcmCodec
     const E_TransferSyntax oldRepType,
     const E_TransferSyntax newRepType) const;
 
+  /** determines the effective value of BitsAllocated that a dataset will have
+   *  after decompression of an image with the given values for bitsAllocated
+   *  and bitsStored. This may differ from the bitsAllocated parameter for example
+   *  if that value is not a multiple of 8. Returns zero if an image with the
+   *  given parameters cannot be decoded with this codec.
+   *  @param bitsAllocated current value of Bits Allocated
+   *  @param bitsStored current value of Bits Stored
+   *  @return value of BitsAllocated after decompression, 0 if no decompression possible
+   */
+  virtual Uint16 decodedBitsAllocated(
+    Uint16 bitsAllocated,
+    Uint16 bitsStored) const;
+
   /** determine color model of the decompressed image
    *  @param fromParam representation parameter of current compressed
    *    representation, may be NULL

dcmdata/include/dcmtk/dcmdata/dcrlecce.h

@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 2002-2020, OFFIS e.V.
+ *  Copyright (C) 2002-2024, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -165,6 +165,19 @@ class DCMTK_DCMDATA_EXPORT DcmRLECodecEncoder: public DcmCodec
     const E_TransferSyntax oldRepType,
     const E_TransferSyntax newRepType) const;
 
+  /** determines the effective value of BitsAllocated that a dataset will have
+   *  after decompression of an image with the given values for bitsAllocated
+   *  and bitsStored. This may differ from the bitsAllocated parameter for example
+   *  if that value is not a multiple of 8. Returns zero if an image with the
+   *  given parameters cannot be decoded with this codec.
+   *  @param bitsAllocated current value of Bits Allocated
+   *  @param bitsStored current value of Bits Stored
+   *  @return value of BitsAllocated after decompression, 0 if no decompression possible
+   */
+  virtual Uint16 decodedBitsAllocated(
+    Uint16 bitsAllocated,
+    Uint16 bitsStored) const;
+
   /** determine color model of the decompressed image
    *  @param fromParam representation parameter of current compressed
    *    representation, may be NULL

dcmdata/libsrc/dccodec.cc

@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 1997-2023, OFFIS e.V.
+ *  Copyright (C) 1997-2024, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -518,6 +518,40 @@ OFCondition DcmCodecList::decodeFrame(
 }
 
 
+Uint16 DcmCodecList::decodedBitsAllocated(
+  const DcmXfer & fromType,
+  Uint16 bitsAllocated,
+  Uint16 bitsStored)
+{
+#ifdef WITH_THREADS
+  if (! codecLock.initialized()) return 0; // should never happen
+#endif
+  Uint16 result = 0;
+
+  // acquire write lock on codec list.  Will block if some write lock is currently active.
+#ifdef WITH_THREADS
+  OFReadWriteLocker locker(codecLock);
+  if (0 == locker.rdlock())
+  {
+#endif
+    E_TransferSyntax fromXfer = fromType.getXfer();
+    OFListIterator(DcmCodecList *) first = registeredCodecs.begin();
+    OFListIterator(DcmCodecList *) last = registeredCodecs.end();
+    while (first != last)
+    {
+      if ((*first)->codec->canChangeCoding(fromXfer, EXS_LittleEndianExplicit))
+      {
+        result = (*first)->codec->decodedBitsAllocated(bitsAllocated, bitsStored);
+        first = last;
+      } else ++first;
+    }
+#ifdef WITH_THREADS
+  } else result = 0;
+#endif
+  return result;
+}
+
+
 OFCondition DcmCodecList::encode(
   const E_TransferSyntax fromRepType,
   const DcmRepresentationParameter * fromParam,

dcmdata/libsrc/dcelem.cc

@@ -2000,6 +2000,17 @@ OFCondition DcmElement::createValueFromTempFile(DcmInputStreamFactory *factory,
 }
 
 
+Uint16 DcmElement::decodedBitsAllocated(
+      Uint16 bitsAllocated,
+      Uint16 bitsStored) const
+{
+  // Note: This method only handles the uncompressed case.
+  // A different implementation is provided in class DcmPixelData.
+  if (bitsStored > bitsAllocated) return 0;
+  return bitsAllocated;
+}
+
+
 // the following macro makes the source code more readable and easier to maintain
 
 #define GET_AND_CHECK_UINT16_VALUE(tag, variable)                                                                   \
@@ -2029,6 +2040,7 @@ OFCondition DcmElement::getUncompressedFrameSize(DcmItem *dataset,
         Uint16 cols = 0;
         Uint16 samplesPerPixel = 0;
         Uint16 bitsAllocated = 0;
+        Uint16 bitsStored = 0;
         Sint32 numberOfFrames = 1;
         OFString photometricInterpretation;
 
@@ -2091,7 +2103,15 @@ OFCondition DcmElement::getUncompressedFrameSize(DcmItem *dataset,
             /* see PS3.3 Table C.7-11c: "Bits Allocated (0028,0100) shall be either 1, or a multiple of 8." */
             else if ((bitsAllocated == 0) || ((bitsAllocated > 1) && (bitsAllocated % 8 != 0)))
                 DCMDATA_WARN("DcmElement: Dubious value (" << bitsAllocated << ") for element BitsAllocated " << DCM_BitsAllocated);
+
+            GET_AND_CHECK_UINT16_VALUE(DCM_BitsStored, bitsStored)
+            else if (bitsStored > bitsAllocated)
+            {
+                DCMDATA_WARN("DcmElement: Dubious value (" << bitsStored << ") for element BitsStored " << DCM_BitsAllocated << " larger than value of BitsAllocated " << DCM_BitsAllocated);
+                result = EC_InvalidValue;
+            }
         }
+
         /* if all checks were passed... */
         if (result.good())
         {
@@ -2129,26 +2149,42 @@ OFCondition DcmElement::getUncompressedFrameSize(DcmItem *dataset,
                    DCMDATA_WARN("DcmElement: failed to compute size of PixelData element");
             }
 
-            /* compute frame size (TODO: check for 32-bit integer overflow?) */
-            if ((bitsAllocated % 8) == 0)
+            // Determine the effective value for Bits Allocated, taking into account compression
+            Uint16 effectiveBitsAllocated = decodedBitsAllocated(bitsAllocated, bitsStored);
+            if (effectiveBitsAllocated == 0)
             {
-                const Uint16 bytesAllocated = bitsAllocated / 8;
-                frameSize = bytesAllocated * rows * cols * samplesPerPixel;
+                DCMDATA_WARN("DcmElement: Encapsulated image with BitsAllocated=" << bitsAllocated << " and BitsStored=" << bitsStored << " cannot be decoded");
+                result = EC_InvalidValue;
             }
             else
             {
-                /* need to split calculation in order to avoid integer overflow for large pixel data */
-                const Uint32 v1 = rows * cols * samplesPerPixel;
-                const Uint32 v2 = (bitsAllocated / 8) * v1;
-                const Uint32 v3 = ((bitsAllocated % 8) * v1 + 7) / 8;
-            //  # old code: frameSize = (bitsAllocated * rows * cols * samplesPerPixel + 7) / 8;
-                frameSize = v2 + v3;
+                /* compute frame size */
+                if ((effectiveBitsAllocated % 8) == 0)
+                {
+                    const Uint16 bytesAllocated = effectiveBitsAllocated / 8;
+                    const Uint32 v1 = rows * cols * samplesPerPixel;
+                    frameSize = bytesAllocated * v1;
+                    if (frameSize / bytesAllocated != v1)
+                    {
+                        DCMDATA_WARN("DcmElement: frame size too large, 32-bit integer overflow");
+                        result = EC_InvalidValue;
+                    }
+                }
+                else
+                {
+                    // Split the calculation in order to avoid integer overflow for large pixel data.
+                    // # old code: frameSize = (effectiveBitsAllocated * rows * cols * samplesPerPixel + 7) / 8;
+                    const Uint32 v1 = rows * cols * samplesPerPixel;
+                    const Uint32 v2 = (effectiveBitsAllocated / 8) * v1;
+                    const Uint32 v3 = ((effectiveBitsAllocated % 8) * v1 + 7) / 8;
+                    frameSize = v2 + v3;
+                }
             }
-        } else {
-            /* in case of error, return a frame size of 0 */
-            frameSize = 0;
         }
     }
+
+    /* in case of error, return a frame size of 0 */
+    if (result.bad()) frameSize = 0;
     return result;
 }
 

dcmdata/libsrc/dcpixel.cc

@@ -1356,3 +1356,21 @@ OFCondition DcmPixelData::writeJson(STD_NAMESPACE ostream &out,
     // pixel data is encapsulated, return error
     return EC_CannotWriteJsonInlineBinary;
 }
+
+
+Uint16 DcmPixelData::decodedBitsAllocated(
+      Uint16 bitsAllocated,
+      Uint16 bitsStored) const
+{
+    if (bitsStored > bitsAllocated) return 0;
+    if (existUnencapsulated || (original == repListEnd))
+    {
+        // we have uncompressed pixel data or pixel data is empty
+        return DcmElement::decodedBitsAllocated(bitsAllocated, bitsStored);
+    }
+    else
+    {
+       return DcmCodecList::decodedBitsAllocated((*original)->repType, bitsAllocated, bitsStored);
+    }
+}
+

dcmdata/libsrc/dcrleccd.cc

@@ -60,6 +60,20 @@ OFBool DcmRLECodecDecoder::canChangeCoding(
 }
 
 
+Uint16 DcmRLECodecDecoder::decodedBitsAllocated(
+    Uint16 bitsAllocated,
+    Uint16 /* bitsStored */) const
+{
+    // The RLE decoder only supports images where BitsAllocated is a multiple of 8.
+    if ((bitsAllocated < 8)||(bitsAllocated % 8 != 0)) return 0;
+
+    // RLE cannot support more than 120 bits/sample (15 bands) in DICOM
+    if (bitsAllocated > 120) return 0;
+
+    return bitsAllocated;
+}
+
+
 OFCondition DcmRLECodecDecoder::decode(
     const DcmRepresentationParameter * /* fromRepParam */,
     DcmPixelSequence * pixSeq,

dcmdata/libsrc/dcrlecce.cc

@@ -62,6 +62,14 @@ OFBool DcmRLECodecEncoder::canChangeCoding(
 }
 
 
+Uint16 DcmRLECodecEncoder::decodedBitsAllocated(
+    Uint16 /* bitsAllocated */,
+    Uint16 /* bitsStored */) const
+{
+  return 0;
+}
+
+
 OFCondition DcmRLECodecEncoder::decode(
     const DcmRepresentationParameter * /* fromRepParam */,
     DcmPixelSequence * /* pixSeq */,

dcmdata/tests/CMakeLists.txt

@@ -4,6 +4,7 @@ DCMTK_ADD_TEST_EXECUTABLE(dcmdata_tests
   tchval.cc
   tdict.cc
   telemlen.cc
+  tfrmsiz.cc
   tests.cc
   tfilter.cc
   tgenuid.cc

dcmdata/tests/Makefile.in

@@ -26,7 +26,7 @@ LIBDCMXML = -ldcmxml
 objs = tests.o tpread.o ti2dbmp.o tchval.o tpath.o tvrdatim.o telemlen.o tparser.o \
 	tdict.o tvrds.o tvrfd.o tvrui.o tvrol.o tvrov.o tvrsv.o tvruv.o tstrval.o \
 	tspchrs.o tvrpn.o tparent.o tfilter.o tvrcomp.o tmatch.o tnewdcme.o \
-	tgenuid.o tsequen.o titem.o ttag.o txfer.o tbytestr.o
+	tgenuid.o tsequen.o titem.o ttag.o txfer.o tbytestr.o tfrmsiz.o
 
 progs = tests
 

dcmdata/tests/tests.cc

@@ -126,4 +126,6 @@ OFTEST_REGISTER(dcmdata_xferLookup_2);
 OFTEST_REGISTER(dcmdata_xferLookup_3);
 OFTEST_REGISTER(dcmdata_xferLookup_4);
 OFTEST_REGISTER(dcmdata_putOFStringAtPos);
+OFTEST_REGISTER(dcmdata_uncompressedFrameSize);
+
 OFTEST_MAIN("dcmdata")