# HG changeset patch # User Sebastien Jodogne # Date 1589888696 -7200 # Node ID 1f33ed7f82e6dad51abb4425ea4685ead208bf0d # Parent 0b3256c3ee14fe72f52608a57f3b0692e1976dfe automatic test of transcoding diff -r 0b3256c3ee14 -r 1f33ed7f82e6 Core/DicomParsing/DcmtkTranscoder.cpp --- 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& 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, ¶meters)) { - 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, ¶meters)) { - 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, ¶meters)) { - CheckSopInstanceUid(dicom, sourceSopInstanceUid, true); + selectedSyntax = DicomTransferSyntax_JPEGProcess14; return true; } } @@ -246,7 +194,7 @@ 0 /* opt_point_transform */); if (FromDcmtkBridge::Transcode(dicom, DicomTransferSyntax_JPEGProcess14SV1, ¶meters)) { - CheckSopInstanceUid(dicom, sourceSopInstanceUid, true); + selectedSyntax = DicomTransferSyntax_JPEGProcess14SV1; return true; } } @@ -265,7 +213,7 @@ **/ if (FromDcmtkBridge::Transcode(dicom, DicomTransferSyntax_JPEGLSLossless, ¶meters)) { - CheckSopInstanceUid(dicom, sourceSopInstanceUid, true); + selectedSyntax = DicomTransferSyntax_JPEGLSLossless; return true; } } @@ -285,7 +233,7 @@ **/ if (FromDcmtkBridge::Transcode(dicom, DicomTransferSyntax_JPEGLSLossy, ¶meters)) { - 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 diff -r 0b3256c3ee14 -r 1f33ed7f82e6 Core/DicomParsing/DcmtkTranscoder.h --- 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& allowedSyntaxes, bool allowNewSopInstanceUid); diff -r 0b3256c3ee14 -r 1f33ed7f82e6 Core/DicomParsing/IDicomTranscoder.cpp --- 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 +#include 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& 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) diff -r 0b3256c3ee14 -r 1f33ed7f82e6 Core/DicomParsing/IDicomTranscoder.h --- 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& allowedSyntaxes, + bool allowNewSopInstanceUid); + public: virtual ~IDicomTranscoder() { } - virtual bool Transcode(DicomImage& target, bool& hasSopInstanceUidChanged /* out */, DicomImage& source /* in, "GetParsed()" possibly modified */, diff -r 0b3256c3ee14 -r 1f33ed7f82e6 Core/DicomParsing/MemoryBufferTranscoder.cpp --- 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 diff -r 0b3256c3ee14 -r 1f33ed7f82e6 Plugins/Engine/OrthancPlugins.cpp --- 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; } }