GDCM: several improvements

- fixed width and height order
- removed unused methods
- simplified signature matching
- rewrote pixel format matching in more compact form
- added dimensions number check (only 2 is allowed)
- added target buffer size check
- added debug messages in all failing points

modules/imgcodecs/src/grfmt_gdcm.cpp

@@ -45,8 +45,19 @@
 
 #ifdef HAVE_GDCM
 
+//#define DBG(...) printf(__VA_ARGS__)
+#define DBG(...)
+
 #include <gdcmImageReader.h>
 
+static const size_t preamble_skip = 128;
+static const size_t magic_len = 4;
+
+inline cv::String getMagic()
+{
+    return cv::String("\x44\x49\x43\x4D", 4);
+}
+
 namespace cv
 {
 
@@ -54,45 +65,22 @@ namespace cv
 
 DICOMDecoder::DICOMDecoder()
 {
-    /// DICOM preable is 128 bytes (can have any vale, defaults to x00) + 4 bytes magic number (DICM)
+    // DICOM preamble is 128 bytes (can have any value, defaults to 0) + 4 bytes magic number (DICM)
-    m_signature = "";
+    m_signature = String(preamble_skip, (char)'\x0') + getMagic();
-    for(int iSize=0; iSize<128; iSize++)
-    {
-        m_signature = m_signature + "\xFF";
-    }
-
-    m_signature = m_signature + "\x44\x49\x43\x4D";
-
     m_buf_supported = false;
 }
 
-
-DICOMDecoder::~DICOMDecoder()
-{
-}
-
 bool DICOMDecoder::checkSignature( const String& signature ) const
 {
-    size_t len = signatureLength();
+    if (signature.size() >= preamble_skip + magic_len)
-    bool bOK = signature.size() >= len;
-    for(int iIndex = 128; iIndex < len; iIndex++)
     {
-        if(signature[iIndex] == m_signature[iIndex])
+        if (signature.substr(preamble_skip, magic_len) == getMagic())
         {
-            continue;
+            return true;
-        }
-        else
-        {
-            bOK = false;
-            break;
         }
     }
-
+    DBG("GDCM | Signature does not match\n");
-    return(bOK);
+    return false;
-}
-
-void  DICOMDecoder::close()
-{
 }
 
 ImageDecoder DICOMDecoder::newDecoder() const
@@ -106,90 +94,66 @@ bool  DICOMDecoder::readHeader()
     csImageReader.SetFileName(m_filename.c_str());
     if(!csImageReader.Read())
     {
+        DBG("GDCM | Failed to open DICOM file\n");
         return(false);
     }
 
-    bool bOK = true;
-
     const gdcm::Image &csImage = csImageReader.GetImage();
-    if(        ( csImage.GetPhotometricInterpretation() == gdcm::PhotometricInterpretation::MONOCHROME1 )
+    bool bOK = true;
-        ||    ( csImage.GetPhotometricInterpretation() == gdcm::PhotometricInterpretation::MONOCHROME2 )
+    switch (csImage.GetPhotometricInterpretation().GetType())
-        )
     {
-        gdcm::PixelFormat ePixelFormat = csImage.GetPixelFormat();
+        case gdcm::PhotometricInterpretation::MONOCHROME1:
-        if( ePixelFormat == gdcm::PixelFormat::INT8)
+        case gdcm::PhotometricInterpretation::MONOCHROME2:
         {
-            m_type = CV_8SC1;
+            switch (csImage.GetPixelFormat().GetScalarType())
+            {
+                case gdcm::PixelFormat::INT8: m_type = CV_8SC1; break;
+                case gdcm::PixelFormat::UINT8: m_type = CV_8UC1; break;
+                case gdcm::PixelFormat::INT16: m_type = CV_16SC1; break;
+                case gdcm::PixelFormat::UINT16: m_type = CV_16UC1; break;
+                case gdcm::PixelFormat::INT32: m_type = CV_32SC1; break;
+                case gdcm::PixelFormat::FLOAT32: m_type = CV_32FC1; break;
+                case gdcm::PixelFormat::FLOAT64: m_type = CV_64FC1; break;
+                default: bOK = false; DBG("GDCM | Monochrome scalar type not supported\n"); break;
             }
-        else if( ePixelFormat == gdcm::PixelFormat::UINT8)
+            break;
-        {
-            m_type = CV_8UC1;
         }
-        else if( ePixelFormat == gdcm::PixelFormat::INT16)
+
+        case gdcm::PhotometricInterpretation::RGB:
         {
-            m_type = CV_16SC1;
+            switch (csImage.GetPixelFormat().GetScalarType())
-        }
-        else if( ePixelFormat == gdcm::PixelFormat::UINT16)
             {
-            m_type = CV_16UC1;
+                case gdcm::PixelFormat::UINT8: m_type = CV_8UC3; break;
+                default: bOK = false; DBG("GDCM | RGB scalar type not supported\n"); break;
             }
-        else if( ePixelFormat == gdcm::PixelFormat::INT32)
+            break;
-        {
-            m_type = CV_32SC1;
         }
-        else if( ePixelFormat == gdcm::PixelFormat::FLOAT32)
+
-        {
+        default:
-            m_type = CV_32FC1;
-        }
-        else if( ePixelFormat == gdcm::PixelFormat::FLOAT64)
-        {
-            m_type = CV_64FC1;
-        }
-        else if( ePixelFormat == gdcm::PixelFormat::INT12)
         {
             bOK = false;
+            DBG("GDCM | PI not supported: %s\n", csImage.GetPhotometricInterpretation().GetString());
+            break;
         }
-        else if( ePixelFormat == gdcm::PixelFormat::UINT12)
-        {
-            bOK = false;
-        }
-        else if( ePixelFormat == gdcm::PixelFormat::UINT32)
-        {
-            bOK = false;
-        }
-        else if( ePixelFormat == gdcm::PixelFormat::SINGLEBIT)
-        {
-            bOK = false;
-        }
-        else
-        {
-            bOK = false;
-        }
-    }
-    else if( csImage.GetPhotometricInterpretation() == gdcm::PhotometricInterpretation::RGB )
-    {
-        gdcm::PixelFormat ePixelFormat = csImage.GetPixelFormat();
-        if( ePixelFormat == gdcm::PixelFormat::UINT8)
-        {
-            m_type = CV_8UC3;
-        }
-        else
-        {
-            bOK = false;
-        }
-    }
-    else
-    {
-        bOK = false;
     }
 
     if(bOK)
+    {
+        unsigned int ndim = csImage.GetNumberOfDimensions();
+        if (ndim != 2)
+        {
+            DBG("GDCM | Invalid dimensions number: %d\n", ndim);
+            bOK = false;
+        }
+    }
+    if (bOK)
     {
         const unsigned int *piDimension = csImage.GetDimensions();
-        m_width = piDimension[0];
+        m_height = piDimension[0];
-        m_height = piDimension[1];
+        m_width = piDimension[1];
         if( ( m_width <=0 )  || ( m_height <=0 ) )
         {
+            DBG("GDCM | Invalid dimensions: %d x %d\n", piDimension[0], piDimension[1]);
             bOK = false;
         }
     }
@@ -206,15 +170,28 @@ bool  DICOMDecoder::readData( Mat& csImage )
     csImageReader.SetFileName(m_filename.c_str());
     if(!csImageReader.Read())
     {
-        return(false);
+        DBG("GDCM | Failed to Read\n");
+        return false;
     }
 
-    bool bOK = true;
+    const gdcm::Image &img = csImageReader.GetImage();
-    const gdcm::Image &csGDCMImage = csImageReader.GetImage();
-    bOK = csGDCMImage.GetBuffer((char*)csImage.ptr());
 
-    return(bOK);
+    unsigned long len = img.GetBufferLength();
-}
+    if (len > csImage.elemSize() * csImage.total())
+    {
+        DBG("GDCM | Buffer is bigger than Mat: %ld > %ld * %ld\n", len, csImage.elemSize(), csImage.total());
+        return false;
+    }
+
+    if (!img.GetBuffer((char*)csImage.ptr()))
+    {
+        DBG("GDCM | Failed to GetBuffer\n");
+        return false;
+    }
+    DBG("GDCM | Read OK\n");
+    return true;
 }
 
-#endif
+}
+
+#endif // HAVE_GDCM

modules/imgcodecs/src/grfmt_gdcm.hpp

@@ -56,22 +56,15 @@ namespace cv
 class DICOMDecoder : public BaseImageDecoder
 {
 public:
-
     DICOMDecoder();
-    ~DICOMDecoder();
-
     bool  readData( Mat& img );
     bool  readHeader();
-    void  close();
-
     ImageDecoder newDecoder() const;
     virtual bool checkSignature( const String& signature ) const;
-
-protected:
 };
 
 }
 
-#endif
+#endif // HAVE_GDCM
 
 #endif/*_GDCM_DICOM_H_*/