Mercurial > hg > orthanc

changeset 3946:1f33ed7f82e6 transcoding

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
automatic test of transcoding
author Sebastien Jodogne <s.jodogne@gmail.com>
date Tue, 19 May 2020 13:44:56 +0200
parents 0b3256c3ee14
children cf6eb4fc6841
files Core/DicomParsing/DcmtkTranscoder.cpp Core/DicomParsing/DcmtkTranscoder.h Core/DicomParsing/IDicomTranscoder.cpp Core/DicomParsing/IDicomTranscoder.h Core/DicomParsing/MemoryBufferTranscoder.cpp Plugins/Engine/OrthancPlugins.cpp
diffstat 6 files changed, 220 insertions(+), 70 deletions(-) [+]
line wrap: on
line diff
--- a/Core/DicomParsing/DcmtkTranscoder.cpp	Tue May 19 11:24:00 2020 +0200
+++ b/Core/DicomParsing/DcmtkTranscoder.cpp	Tue May 19 13:44:56 2020 +0200
@@ -62,59 +62,6 @@
   }
 
   
-  static std::string GetSopInstanceUid(DcmDataset& dataset)
-  {
-    const char* v = NULL;
-
-    if (dataset.findAndGetString(DCM_SOPInstanceUID, v).good() &&
-        v != NULL)
-    {
-      return std::string(v);
-    }
-    else
-    {
-      throw OrthancException(ErrorCode_BadFileFormat, "File without SOP instance UID");
-    }
-  }
-
-  
-  static void CheckSopInstanceUid(DcmFileFormat& dicom,
-                                  const std::string& sopInstanceUid,
-                                  bool mustEqual)
-  {
-    if (dicom.getDataset() == NULL)
-    {
-      throw OrthancException(ErrorCode_InternalError);
-    }
-
-    bool ok;
-      
-    if (dicom.getDataset()->tagExists(DCM_PixelData))
-    {
-      if (mustEqual)
-      {
-        ok = (GetSopInstanceUid(*dicom.getDataset()) == sopInstanceUid);
-      }
-      else
-      {
-        ok = (GetSopInstanceUid(*dicom.getDataset()) != sopInstanceUid);
-      }
-    }
-    else
-    {
-      // No pixel data: Transcoding must not change the SOP instance UID
-      ok = (GetSopInstanceUid(*dicom.getDataset()) == sopInstanceUid);
-    }
-    
-    if (!ok)
-    {
-      throw OrthancException(ErrorCode_InternalError,
-                             mustEqual ? "The SOP instance UID has changed unexpectedly during transcoding" :
-                             "The SOP instance UID has not changed as expected during transcoding");
-    }
-  }
-    
-
   void DcmtkTranscoder::SetLossyQuality(unsigned int quality)
   {
     if (quality <= 0 ||
@@ -134,6 +81,7 @@
 
     
   bool DcmtkTranscoder::InplaceTranscode(bool& hasSopInstanceUidChanged /* out */,
+                                         DicomTransferSyntax& selectedSyntax /* out */,
                                          DcmFileFormat& dicom,
                                          const std::set<DicomTransferSyntax>& allowedSyntaxes,
                                          bool allowNewSopInstanceUid) 
@@ -155,7 +103,7 @@
     uint16_t bitsStored;
     bool hasBitsStored = GetBitsStored(bitsStored, *dicom.getDataset());
     
-    std::string sourceSopInstanceUid = GetSopInstanceUid(*dicom.getDataset());
+    std::string sourceSopInstanceUid = IDicomTranscoder::GetSopInstanceUid(dicom);
     
     if (allowedSyntaxes.find(syntax) != allowedSyntaxes.end())
     {
@@ -166,28 +114,28 @@
     if (allowedSyntaxes.find(DicomTransferSyntax_LittleEndianImplicit) != allowedSyntaxes.end() &&
         FromDcmtkBridge::Transcode(dicom, DicomTransferSyntax_LittleEndianImplicit, NULL))
     {
-      CheckSopInstanceUid(dicom, sourceSopInstanceUid, true);
+      selectedSyntax = DicomTransferSyntax_LittleEndianImplicit;
       return true;
     }
 
     if (allowedSyntaxes.find(DicomTransferSyntax_LittleEndianExplicit) != allowedSyntaxes.end() &&
         FromDcmtkBridge::Transcode(dicom, DicomTransferSyntax_LittleEndianExplicit, NULL))
     {
-      CheckSopInstanceUid(dicom, sourceSopInstanceUid, true);
+      selectedSyntax = DicomTransferSyntax_LittleEndianExplicit;
       return true;
     }
       
     if (allowedSyntaxes.find(DicomTransferSyntax_BigEndianExplicit) != allowedSyntaxes.end() &&
         FromDcmtkBridge::Transcode(dicom, DicomTransferSyntax_BigEndianExplicit, NULL))
     {
-      CheckSopInstanceUid(dicom, sourceSopInstanceUid, true);
+      selectedSyntax = DicomTransferSyntax_BigEndianExplicit;
       return true;
     }
 
     if (allowedSyntaxes.find(DicomTransferSyntax_DeflatedLittleEndianExplicit) != allowedSyntaxes.end() &&
         FromDcmtkBridge::Transcode(dicom, DicomTransferSyntax_DeflatedLittleEndianExplicit, NULL))
     {
-      CheckSopInstanceUid(dicom, sourceSopInstanceUid, true);
+      selectedSyntax = DicomTransferSyntax_DeflatedLittleEndianExplicit;
       return true;
     }
 
@@ -201,7 +149,7 @@
         
       if (FromDcmtkBridge::Transcode(dicom, DicomTransferSyntax_JPEGProcess1, &parameters))
       {
-        CheckSopInstanceUid(dicom, sourceSopInstanceUid, false);
+        selectedSyntax = DicomTransferSyntax_JPEGProcess1;
         hasSopInstanceUidChanged = true;
         return true;
       }
@@ -217,7 +165,7 @@
       DJ_RPLossy parameters(lossyQuality_);
       if (FromDcmtkBridge::Transcode(dicom, DicomTransferSyntax_JPEGProcess2_4, &parameters))
       {
-        CheckSopInstanceUid(dicom, sourceSopInstanceUid, false);
+        selectedSyntax = DicomTransferSyntax_JPEGProcess2_4;
         hasSopInstanceUidChanged = true;
         return true;
       }
@@ -232,7 +180,7 @@
                                0 /* opt_point_transform */);
       if (FromDcmtkBridge::Transcode(dicom, DicomTransferSyntax_JPEGProcess14, &parameters))
       {
-        CheckSopInstanceUid(dicom, sourceSopInstanceUid, true);
+        selectedSyntax = DicomTransferSyntax_JPEGProcess14;
         return true;
       }
     }
@@ -246,7 +194,7 @@
                                0 /* opt_point_transform */);
       if (FromDcmtkBridge::Transcode(dicom, DicomTransferSyntax_JPEGProcess14SV1, &parameters))
       {
-        CheckSopInstanceUid(dicom, sourceSopInstanceUid, true);
+        selectedSyntax = DicomTransferSyntax_JPEGProcess14SV1;
         return true;
       }
     }
@@ -265,7 +213,7 @@
        **/              
       if (FromDcmtkBridge::Transcode(dicom, DicomTransferSyntax_JPEGLSLossless, &parameters))
       {
-        CheckSopInstanceUid(dicom, sourceSopInstanceUid, true);
+        selectedSyntax = DicomTransferSyntax_JPEGLSLossless;
         return true;
       }
     }
@@ -285,7 +233,7 @@
        **/              
       if (FromDcmtkBridge::Transcode(dicom, DicomTransferSyntax_JPEGLSLossy, &parameters))
       {
-        CheckSopInstanceUid(dicom, sourceSopInstanceUid, false);
+        selectedSyntax = DicomTransferSyntax_JPEGLSLossy;
         hasSopInstanceUidChanged = true;
         return true;
       }
@@ -343,6 +291,11 @@
       return false;
     }
 
+#if !defined(NDEBUG)
+    const std::string sourceSopInstanceUid = GetSopInstanceUid(source.GetParsed());
+#endif
+
+    DicomTransferSyntax targetSyntax;
     if (allowedSyntaxes.find(sourceSyntax) != allowedSyntaxes.end())
     {
       // No transcoding is needed
@@ -350,16 +303,24 @@
       target.AcquireBuffer(source);
       return true;
     }
-    else if (InplaceTranscode(hasSopInstanceUidChanged, source.GetParsed(),
+    else if (InplaceTranscode(hasSopInstanceUidChanged, targetSyntax, source.GetParsed(),
                               allowedSyntaxes, allowNewSopInstanceUid))
-    {
+    {   
       // Sanity check
-      DicomTransferSyntax targetSyntax;
-      if (FromDcmtkBridge::LookupOrthancTransferSyntax(targetSyntax, source.GetParsed()) &&
-          allowedSyntaxes.find(targetSyntax) != allowedSyntaxes.end())
+      DicomTransferSyntax targetSyntax2;
+      if (FromDcmtkBridge::LookupOrthancTransferSyntax(targetSyntax2, source.GetParsed()) &&
+          targetSyntax == targetSyntax2 &&
+          allowedSyntaxes.find(targetSyntax2) != allowedSyntaxes.end())
       {
         target.AcquireParsed(source);
         source.Clear();
+        
+#if !defined(NDEBUG)
+        // Only run the sanity check in debug mode
+        CheckTranscoding(target, hasSopInstanceUidChanged, sourceSyntax, sourceSopInstanceUid,
+                         allowedSyntaxes, allowNewSopInstanceUid);
+#endif
+        
         return true;
       }
       else
--- a/Core/DicomParsing/DcmtkTranscoder.h	Tue May 19 11:24:00 2020 +0200
+++ b/Core/DicomParsing/DcmtkTranscoder.h	Tue May 19 13:44:56 2020 +0200
@@ -51,6 +51,7 @@
     unsigned int  lossyQuality_;
     
     bool InplaceTranscode(bool& hasSopInstanceUidChanged /* out */,
+                          DicomTransferSyntax& selectedSyntax /* out */,
                           DcmFileFormat& dicom,
                           const std::set<DicomTransferSyntax>& allowedSyntaxes,
                           bool allowNewSopInstanceUid);
--- a/Core/DicomParsing/IDicomTranscoder.cpp	Tue May 19 11:24:00 2020 +0200
+++ b/Core/DicomParsing/IDicomTranscoder.cpp	Tue May 19 13:44:56 2020 +0200
@@ -39,9 +39,156 @@
 #include "ParsedDicomFile.h"
 
 #include <dcmtk/dcmdata/dcfilefo.h>
+#include <dcmtk/dcmdata/dcdeftag.h>
 
 namespace Orthanc
 {
+  IDicomTranscoder::TranscodingType IDicomTranscoder::GetTranscodingType(DicomTransferSyntax target,
+                                                                         DicomTransferSyntax source)
+  {
+    if (target == source)
+    {
+      return TranscodingType_Lossless;
+    }
+    else if (target == DicomTransferSyntax_LittleEndianImplicit ||
+             target == DicomTransferSyntax_LittleEndianExplicit ||
+             target == DicomTransferSyntax_BigEndianExplicit ||
+             target == DicomTransferSyntax_DeflatedLittleEndianExplicit ||
+             target == DicomTransferSyntax_JPEGProcess14 ||
+             target == DicomTransferSyntax_JPEGProcess14SV1 ||
+             target == DicomTransferSyntax_JPEGLSLossless ||
+             target == DicomTransferSyntax_JPEG2000LosslessOnly ||
+             target == DicomTransferSyntax_JPEG2000MulticomponentLosslessOnly)
+    {
+      return TranscodingType_Lossless;
+    }
+    else if (target == DicomTransferSyntax_JPEGProcess1 ||
+             target == DicomTransferSyntax_JPEGProcess2_4 ||
+             target == DicomTransferSyntax_JPEGLSLossy ||
+             target == DicomTransferSyntax_JPEG2000 ||
+             target == DicomTransferSyntax_JPEG2000Multicomponent)
+    {
+      return TranscodingType_Lossy;
+    }
+    else
+    {
+      return TranscodingType_Unknown;
+    }
+  }
+
+
+  std::string IDicomTranscoder::GetSopInstanceUid(DcmFileFormat& dicom)
+  {
+    if (dicom.getDataset() == NULL)
+    {
+      throw OrthancException(ErrorCode_InternalError);
+    }
+    
+    DcmDataset& dataset = *dicom.getDataset();
+    
+    const char* v = NULL;
+
+    if (dataset.findAndGetString(DCM_SOPInstanceUID, v).good() &&
+        v != NULL)
+    {
+      return std::string(v);
+    }
+    else
+    {
+      throw OrthancException(ErrorCode_BadFileFormat, "File without SOP instance UID");
+    }
+  }
+
+
+  void IDicomTranscoder::CheckTranscoding(IDicomTranscoder::DicomImage& transcoded,
+                                          bool hasSopInstanceUidChanged,
+                                          DicomTransferSyntax sourceSyntax,
+                                          const std::string& sourceSopInstanceUid,
+                                          const std::set<DicomTransferSyntax>& allowedSyntaxes,
+                                          bool allowNewSopInstanceUid)
+  {
+    DcmFileFormat& parsed = transcoded.GetParsed();
+    
+    if (parsed.getDataset() == NULL)
+    {
+      throw OrthancException(ErrorCode_InternalError);
+    }
+
+    std::string targetSopInstanceUid = GetSopInstanceUid(parsed);
+
+    if (hasSopInstanceUidChanged && (targetSopInstanceUid == sourceSopInstanceUid))
+    {
+      throw OrthancException(ErrorCode_InternalError);
+    }
+
+    if (!hasSopInstanceUidChanged && (targetSopInstanceUid != sourceSopInstanceUid))
+    {
+      throw OrthancException(ErrorCode_InternalError);
+    }
+
+    if (parsed.getDataset()->tagExists(DCM_PixelData))
+    {
+      if (!allowNewSopInstanceUid && (targetSopInstanceUid != sourceSopInstanceUid))
+      {
+        throw OrthancException(ErrorCode_InternalError);
+      }
+    }
+    else
+    {
+      if (hasSopInstanceUidChanged ||
+          targetSopInstanceUid != sourceSopInstanceUid)
+      {
+        throw OrthancException(ErrorCode_InternalError,
+                               "No pixel data: Transcoding must not change the SOP instance UID");
+      }
+    }
+
+    DicomTransferSyntax targetSyntax;
+    if (!FromDcmtkBridge::LookupOrthancTransferSyntax(targetSyntax, parsed))
+    {
+      return;  // Unknown transfer syntax, cannot do further test
+    }
+
+    if (allowedSyntaxes.find(sourceSyntax) != allowedSyntaxes.end())
+    {
+      // No transcoding should have happened
+      if (targetSopInstanceUid != sourceSopInstanceUid ||
+          hasSopInstanceUidChanged)
+      {
+        throw OrthancException(ErrorCode_InternalError);
+      }
+    }
+        
+    if (allowedSyntaxes.find(targetSyntax) == allowedSyntaxes.end())
+    {
+      throw OrthancException(ErrorCode_InternalError, "An incorrect output transfer syntax was chosen");
+    }
+    
+    if (parsed.getDataset()->tagExists(DCM_PixelData))
+    {
+      switch (GetTranscodingType(targetSyntax, sourceSyntax))
+      {
+        case TranscodingType_Lossy:
+          if (targetSopInstanceUid == sourceSopInstanceUid)
+          {
+            throw OrthancException(ErrorCode_InternalError);
+          }
+          break;
+
+        case TranscodingType_Lossless:
+          if (targetSopInstanceUid != sourceSopInstanceUid)
+          {
+            throw OrthancException(ErrorCode_InternalError);
+          }
+          break;
+
+        default:
+          break;
+      }
+    }
+  }
+    
+
   void IDicomTranscoder::DicomImage::Parse()
   {
     if (parsed_.get() != NULL)
--- a/Core/DicomParsing/IDicomTranscoder.h	Tue May 19 11:24:00 2020 +0200
+++ b/Core/DicomParsing/IDicomTranscoder.h	Tue May 19 13:44:56 2020 +0200
@@ -98,12 +98,32 @@
       size_t GetBufferSize();
     };
 
+
+  protected:
+    enum TranscodingType
+    {
+      TranscodingType_Lossy,
+      TranscodingType_Lossless,
+      TranscodingType_Unknown
+    };
+
+    static TranscodingType GetTranscodingType(DicomTransferSyntax target,
+                                              DicomTransferSyntax source);
+
+    static std::string GetSopInstanceUid(DcmFileFormat& dicom);
+
+    static void CheckTranscoding(DicomImage& transcoded,
+                                 bool hasSopInstanceUidChanged,
+                                 DicomTransferSyntax sourceSyntax,
+                                 const std::string& sourceSopInstanceUid,
+                                 const std::set<DicomTransferSyntax>& allowedSyntaxes,
+                                 bool allowNewSopInstanceUid);
     
+  public:    
     virtual ~IDicomTranscoder()
     {
     }
 
-
     virtual bool Transcode(DicomImage& target,
                            bool& hasSopInstanceUidChanged /* out */,
                            DicomImage& source /* in, "GetParsed()" possibly modified */,
--- a/Core/DicomParsing/MemoryBufferTranscoder.cpp	Tue May 19 11:24:00 2020 +0200
+++ b/Core/DicomParsing/MemoryBufferTranscoder.cpp	Tue May 19 13:44:56 2020 +0200
@@ -74,12 +74,32 @@
   {
     target.Clear();
     
+#if !defined(NDEBUG)
+    // Don't run this code in release mode, as it implies parsing the DICOM file
+    DicomTransferSyntax sourceSyntax;
+    if (!FromDcmtkBridge::LookupOrthancTransferSyntax(sourceSyntax, source.GetParsed()))
+    {
+      LOG(ERROR) << "Unsupport transfer syntax for transcoding";
+      return false;
+    }
+    
+    const std::string sourceSopInstanceUid = GetSopInstanceUid(source.GetParsed());
+#endif
+
     std::string buffer;
     if (TranscodeBuffer(buffer, hasSopInstanceUidChanged, source.GetBufferData(),
                         source.GetBufferSize(), allowedSyntaxes, allowNewSopInstanceUid))
     {
       CheckTargetSyntax(buffer, allowedSyntaxes);  // For debug only
+
       target.AcquireBuffer(buffer);
+      
+#if !defined(NDEBUG)
+      // Only run the sanity check in debug mode
+      CheckTranscoding(target, hasSopInstanceUidChanged, sourceSyntax, sourceSopInstanceUid,
+                       allowedSyntaxes, allowNewSopInstanceUid);
+#endif
+
       return true;
     }
     else
--- a/Plugins/Engine/OrthancPlugins.cpp	Tue May 19 11:24:00 2020 +0200
+++ b/Plugins/Engine/OrthancPlugins.cpp	Tue May 19 13:44:56 2020 +0200
@@ -5247,6 +5247,7 @@
           OrthancPluginErrorCode_Success)
       {
         a.ToString(target);
+        hasSopInstanceUidChanged = b;
         return true;
       }
     }