Mercurial > hg > orthanc
changeset 4465:fe774d8e904b
New configuration option: "DicomScuPreferredTransferSyntax" to control transcoding in C-STORE SCU
| author | Sebastien Jodogne <s.jodogne@gmail.com> |
|---|---|
| date | Thu, 21 Jan 2021 17:08:32 +0100 |
| parents | e8c7be7a02a9 |
| children | 2243f1bb909b |
| files | NEWS OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.cpp OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.h OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp OrthancServer/Resources/Configuration.json OrthancServer/Sources/ServerContext.cpp OrthancServer/Sources/ServerContext.h OrthancServer/Sources/ServerJobs/DicomModalityStoreJob.cpp |
| diffstat | 8 files changed, 210 insertions(+), 111 deletions(-) [+] |
line wrap: on
line diff
--- a/NEWS Wed Jan 20 17:43:15 2021 +0100 +++ b/NEWS Thu Jan 21 17:08:32 2021 +0100 @@ -13,6 +13,7 @@ - "UseDicomTls" in "DicomModalities" to enable DICOM TLS in outgoing SCU on a per-modality basis - "MaximumPduLength" to tune the maximum PDU length (Protocol Data Unit) - "LocalAet" in "DicomModalities" to overwrite global "DicomAet" for SCU on a per-modality basis +* New configuration option: "DicomScuPreferredTransferSyntax" to control transcoding in C-STORE SCU * New command-line option: "--openapi" to write the OpenAPI documentation of the REST API to a file * New metadata automatically computed at the series level: "RemoteAET"
--- a/OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.cpp Wed Jan 20 17:43:15 2021 +0100 +++ b/OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.cpp Thu Jan 21 17:08:32 2021 +0100 @@ -31,6 +31,8 @@ #include <dcmtk/dcmdata/dcdeftag.h> +#include <list> + namespace Orthanc { @@ -49,76 +51,77 @@ bool DicomStoreUserConnection::ProposeStorageClass(const std::string& sopClassUid, - const std::set<DicomTransferSyntax>& syntaxes) + const std::set<DicomTransferSyntax>& sourceSyntaxes, + bool hasPreferred, + DicomTransferSyntax preferred) { - // Default transfer syntax for DICOM - const bool addLittleEndianImplicit = ( - proposeUncompressedSyntaxes_ && - syntaxes.find(DicomTransferSyntax_LittleEndianImplicit) == syntaxes.end()); - - const bool addLittleEndianExplicit = ( - proposeUncompressedSyntaxes_ && - syntaxes.find(DicomTransferSyntax_LittleEndianExplicit) == syntaxes.end()); - - const bool addBigEndianExplicit = ( - proposeUncompressedSyntaxes_ && - proposeRetiredBigEndian_ && - syntaxes.find(DicomTransferSyntax_BigEndianExplicit) == syntaxes.end()); - - size_t requiredCount = syntaxes.size(); - if (addLittleEndianImplicit) + typedef std::list< std::set<DicomTransferSyntax> > GroupsOfSyntaxes; + + GroupsOfSyntaxes groups; + + // Firstly, add one group for each individual transfer syntax + for (std::set<DicomTransferSyntax>::const_iterator + it = sourceSyntaxes.begin(); it != sourceSyntaxes.end(); ++it) + { + std::set<DicomTransferSyntax> group; + group.insert(*it); + groups.push_back(group); + } + + // Secondly, add one group with the preferred transfer syntax + if (hasPreferred && + sourceSyntaxes.find(preferred) == sourceSyntaxes.end()) + { + std::set<DicomTransferSyntax> group; + group.insert(preferred); + groups.push_back(group); + } + + // Thirdly, add all the uncompressed transfer syntaxes as one single group + if (proposeUncompressedSyntaxes_) { - requiredCount += 1; + static const size_t N = 3; + static const DicomTransferSyntax UNCOMPRESSED_SYNTAXES[N] = { + DicomTransferSyntax_LittleEndianImplicit, + DicomTransferSyntax_LittleEndianExplicit, + DicomTransferSyntax_BigEndianExplicit + }; + + std::set<DicomTransferSyntax> group; + + for (size_t i = 0; i < N; i++) + { + DicomTransferSyntax syntax = UNCOMPRESSED_SYNTAXES[i]; + if (sourceSyntaxes.find(syntax) == sourceSyntaxes.end() && + (!hasPreferred || preferred != syntax)) + { + group.insert(syntax); + } + } + + if (!group.empty()) + { + groups.push_back(group); + } } - - if (addLittleEndianExplicit || - addBigEndianExplicit) - { - requiredCount += 1; - } - - if (association_->GetRemainingPropositions() <= requiredCount) + + // Now, propose each of these groups of transfer syntaxes + if (association_->GetRemainingPropositions() <= groups.size()) { return false; // Not enough room } else { - for (std::set<DicomTransferSyntax>::const_iterator - it = syntaxes.begin(); it != syntaxes.end(); ++it) + for (GroupsOfSyntaxes::const_iterator it = groups.begin(); it != groups.end(); ++it) { association_->ProposePresentationContext(sopClassUid, *it); - proposedOriginalClasses_.insert(std::make_pair(sopClassUid, *it)); - } - if (addLittleEndianImplicit) - { - association_->ProposePresentationContext(sopClassUid, DicomTransferSyntax_LittleEndianImplicit); - proposedOriginalClasses_.insert(std::make_pair(sopClassUid, DicomTransferSyntax_LittleEndianImplicit)); - } - - if (addLittleEndianExplicit || - addBigEndianExplicit) - { - std::set<DicomTransferSyntax> uncompressed; - - if (addLittleEndianExplicit) + // Remember the syntaxes that were individually proposed, in + // order to avoid renegociation if they are seen again (**) + if (it->size() == 1) { - uncompressed.insert(DicomTransferSyntax_LittleEndianExplicit); - } - - if (addBigEndianExplicit) - { - uncompressed.insert(DicomTransferSyntax_BigEndianExplicit); - } - - association_->ProposePresentationContext(sopClassUid, uncompressed); - - assert(!uncompressed.empty()); - if (addLittleEndianExplicit ^ addBigEndianExplicit) - { - // Only one transfer syntax was proposed for this presentation context - assert(uncompressed.size() == 1); - proposedOriginalClasses_.insert(std::make_pair(sopClassUid, *uncompressed.begin())); + DicomTransferSyntax syntax = *it->begin(); + proposedOriginalClasses_.insert(std::make_pair(sopClassUid, syntax)); } } @@ -247,7 +250,9 @@ bool DicomStoreUserConnection::NegotiatePresentationContext( uint8_t& presentationContextId, const std::string& sopClassUid, - DicomTransferSyntax transferSyntax) + DicomTransferSyntax transferSyntax, + bool hasPreferred, + DicomTransferSyntax preferred) { /** * Step 1: Check whether this presentation context is already @@ -265,6 +270,8 @@ CLOG(INFO, DICOM) << "Re-negotiating DICOM association with " << parameters_.GetRemoteModality().GetApplicationEntityTitle(); + // Don't renegociate if we know that the remote modality was + // already proposed this individual transfer syntax (**) if (proposedOriginalClasses_.find(std::make_pair(sopClassUid, transferSyntax)) != proposedOriginalClasses_.end()) { @@ -294,7 +301,7 @@ throw OrthancException(ErrorCode_InternalError); } - if (!ProposeStorageClass(sopClassUid, mandatory->second)) + if (!ProposeStorageClass(sopClassUid, mandatory->second, hasPreferred, preferred)) { // Should never happen in real life: There are no more than // 128 transfer syntaxes in DICOM! @@ -314,7 +321,7 @@ { if (it->first != sopClassUid) { - ProposeStorageClass(it->first, it->second); + ProposeStorageClass(it->first, it->second, hasPreferred, preferred); } } @@ -340,7 +347,7 @@ if (c != sopClassUid && registeredClasses_.find(c) == registeredClasses_.end()) { - ProposeStorageClass(c, ts); + ProposeStorageClass(c, ts, hasPreferred, preferred); } } } @@ -367,7 +374,8 @@ LookupParameters(sopClassUid, sopInstanceUid, transferSyntax, dicom); uint8_t presID; - if (!NegotiatePresentationContext(presID, sopClassUid, transferSyntax)) + if (!NegotiatePresentationContext(presID, sopClassUid, transferSyntax, proposeUncompressedSyntaxes_, + DicomTransferSyntax_LittleEndianExplicit)) { throw OrthancException(ErrorCode_NetworkProtocol, "No valid presentation context was negotiated for " @@ -465,7 +473,9 @@ void DicomStoreUserConnection::LookupTranscoding(std::set<DicomTransferSyntax>& acceptedSyntaxes, const std::string& sopClassUid, - DicomTransferSyntax sourceSyntax) + DicomTransferSyntax sourceSyntax, + bool hasPreferred, + DicomTransferSyntax preferred) { acceptedSyntaxes.clear(); @@ -473,7 +483,7 @@ // syntax. We don't use the return code: Transcoding is possible // even if the "sourceSyntax" is not supported. uint8_t presID; - NegotiatePresentationContext(presID, sopClassUid, sourceSyntax); + NegotiatePresentationContext(presID, sopClassUid, sourceSyntax, hasPreferred, preferred); std::map<DicomTransferSyntax, uint8_t> contexts; if (association_->LookupAcceptedPresentationContext(contexts, sopClassUid)) @@ -492,6 +502,7 @@ IDicomTranscoder& transcoder, const void* buffer, size_t size, + DicomTransferSyntax preferredTransferSyntax, bool hasMoveOriginator, const std::string& moveOriginatorAET, uint16_t moveOriginatorID) @@ -503,13 +514,13 @@ throw OrthancException(ErrorCode_NullPointer); } - DicomTransferSyntax inputSyntax; - LookupParameters(sopClassUid, sopInstanceUid, inputSyntax, *dicom); + DicomTransferSyntax sourceSyntax; + LookupParameters(sopClassUid, sopInstanceUid, sourceSyntax, *dicom); std::set<DicomTransferSyntax> accepted; - LookupTranscoding(accepted, sopClassUid, inputSyntax); + LookupTranscoding(accepted, sopClassUid, sourceSyntax, true, preferredTransferSyntax); - if (accepted.find(inputSyntax) != accepted.end()) + if (accepted.find(sourceSyntax) != accepted.end()) { // No need for transcoding Store(sopClassUid, sopInstanceUid, *dicom, @@ -518,54 +529,106 @@ else { // Transcoding is needed - std::set<DicomTransferSyntax> uncompressedSyntaxes; - - if (accepted.find(DicomTransferSyntax_LittleEndianImplicit) != accepted.end()) - { - uncompressedSyntaxes.insert(DicomTransferSyntax_LittleEndianImplicit); - } - - if (accepted.find(DicomTransferSyntax_LittleEndianExplicit) != accepted.end()) - { - uncompressedSyntaxes.insert(DicomTransferSyntax_LittleEndianExplicit); - } - - if (accepted.find(DicomTransferSyntax_BigEndianExplicit) != accepted.end()) - { - uncompressedSyntaxes.insert(DicomTransferSyntax_BigEndianExplicit); - } - IDicomTranscoder::DicomImage source; source.AcquireParsed(dicom.release()); source.SetExternalBuffer(buffer, size); const std::string sourceUid = IDicomTranscoder::GetSopInstanceUid(source.GetParsed()); - + IDicomTranscoder::DicomImage transcoded; - if (transcoder.Transcode(transcoded, source, uncompressedSyntaxes, false)) + bool success = false; + bool isDestructiveCompressionAllowed = false; + std::set<DicomTransferSyntax> attemptedSyntaxes; + + if (accepted.find(preferredTransferSyntax) != accepted.end()) + { + // New in Orthanc 1.9.0: The preferred transfer syntax is + // accepted by the remote modality => transcode to this syntax + std::set<DicomTransferSyntax> targetSyntaxes; + targetSyntaxes.insert(preferredTransferSyntax); + attemptedSyntaxes.insert(preferredTransferSyntax); + + success = transcoder.Transcode(transcoded, source, targetSyntaxes, true); + isDestructiveCompressionAllowed = true; + } + + if (!success) { - if (sourceUid != IDicomTranscoder::GetSopInstanceUid(transcoded.GetParsed())) + // Transcode to either one of the uncompressed transfer + // syntaxes that are accepted by the remote modality + + std::set<DicomTransferSyntax> targetSyntaxes; + + if (accepted.find(DicomTransferSyntax_LittleEndianImplicit) != accepted.end()) + { + targetSyntaxes.insert(DicomTransferSyntax_LittleEndianImplicit); + attemptedSyntaxes.insert(DicomTransferSyntax_LittleEndianImplicit); + } + + if (accepted.find(DicomTransferSyntax_LittleEndianExplicit) != accepted.end()) + { + targetSyntaxes.insert(DicomTransferSyntax_LittleEndianExplicit); + attemptedSyntaxes.insert(DicomTransferSyntax_LittleEndianExplicit); + } + + if (accepted.find(DicomTransferSyntax_BigEndianExplicit) != accepted.end()) + { + targetSyntaxes.insert(DicomTransferSyntax_BigEndianExplicit); + attemptedSyntaxes.insert(DicomTransferSyntax_BigEndianExplicit); + } + + if (!targetSyntaxes.empty()) { - throw OrthancException(ErrorCode_Plugin, "The transcoder has changed the SOP " - "instance UID while transcoding to an uncompressed transfer syntax"); + success = transcoder.Transcode(transcoded, source, targetSyntaxes, false); + isDestructiveCompressionAllowed = false; + } + } + + if (success) + { + std::string targetUid = IDicomTranscoder::GetSopInstanceUid(transcoded.GetParsed()); + if (sourceUid != targetUid) + { + if (isDestructiveCompressionAllowed) + { + LOG(WARNING) << "Because of the use of a preferred transfer syntax that corresponds to " + << "a destructive compression, C-STORE SCU has hanged the SOP Instance UID " + << "of a DICOM instance from \"" << sourceUid << "\" to \"" << targetUid << "\""; + } + else + { + throw OrthancException(ErrorCode_Plugin, "The transcoder has changed the SOP " + "Instance UID while transcoding to an uncompressed transfer syntax"); + } + } + + DicomTransferSyntax transcodedSyntax; + + // Sanity check + if (!FromDcmtkBridge::LookupOrthancTransferSyntax(transcodedSyntax, transcoded.GetParsed()) || + accepted.find(transcodedSyntax) == accepted.end()) + { + throw OrthancException(ErrorCode_InternalError); } else { - DicomTransferSyntax transcodedSyntax; - - // Sanity check - if (!FromDcmtkBridge::LookupOrthancTransferSyntax(transcodedSyntax, transcoded.GetParsed()) || - accepted.find(transcodedSyntax) == accepted.end()) - { - throw OrthancException(ErrorCode_InternalError); - } - else - { - Store(sopClassUid, sopInstanceUid, transcoded.GetParsed(), - hasMoveOriginator, moveOriginatorAET, moveOriginatorID); - } + Store(sopClassUid, sopInstanceUid, transcoded.GetParsed(), + hasMoveOriginator, moveOriginatorAET, moveOriginatorID); } } + else + { + std::string s; + for (std::set<DicomTransferSyntax>::const_iterator + it = attemptedSyntaxes.begin(); it != attemptedSyntaxes.end(); ++it) + { + s += " " + std::string(GetTransferSyntaxUid(*it)); + } + + throw OrthancException(ErrorCode_NotImplemented, "Cannot transcode from " + + std::string(GetTransferSyntaxUid(sourceSyntax)) + + " to one of [" + s + " ]"); + } } } }
--- a/OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.h Wed Jan 20 17:43:15 2021 +0100 +++ b/OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.h Thu Jan 21 17:08:32 2021 +0100 @@ -70,7 +70,7 @@ // "ProposedOriginalClasses" keeps track of the storage classes // that were proposed with a single transfer syntax typedef std::set< std::pair<std::string, DicomTransferSyntax> > ProposedOriginalClasses; - + DicomAssociationParameters parameters_; boost::shared_ptr<DicomAssociation> association_; // "shared_ptr" is for PImpl RegisteredClasses registeredClasses_; @@ -81,7 +81,9 @@ // Return "false" if there is not enough room remaining in the association bool ProposeStorageClass(const std::string& sopClassUid, - const std::set<DicomTransferSyntax>& syntaxes); + const std::set<DicomTransferSyntax>& sourceSyntaxes, + bool hasPreferred, + DicomTransferSyntax preferred); bool LookupPresentationContext(uint8_t& presentationContextId, const std::string& sopClassUid, @@ -89,11 +91,15 @@ bool NegotiatePresentationContext(uint8_t& presentationContextId, const std::string& sopClassUid, - DicomTransferSyntax transferSyntax); + DicomTransferSyntax transferSyntax, + bool hasPreferred, + DicomTransferSyntax preferred); void LookupTranscoding(std::set<DicomTransferSyntax>& acceptedSyntaxes, const std::string& sopClassUid, - DicomTransferSyntax sourceSyntax); + DicomTransferSyntax sourceSyntax, + bool hasPreferred, + DicomTransferSyntax preferred); public: explicit DicomStoreUserConnection(const DicomAssociationParameters& params); @@ -140,6 +146,7 @@ IDicomTranscoder& transcoder, const void* buffer, size_t size, + DicomTransferSyntax preferredTransferSyntax, bool hasMoveOriginator, const std::string& moveOriginatorAET, uint16_t moveOriginatorID);
--- a/OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp Wed Jan 20 17:43:15 2021 +0100 +++ b/OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp Thu Jan 21 17:08:32 2021 +0100 @@ -2195,7 +2195,8 @@ std::string c, k; try { - scu.Transcode(c, k, transcoder, source.c_str(), source.size(), false, "", 0); + scu.Transcode(c, k, transcoder, source.c_str(), source.size(), + DicomTransferSyntax_LittleEndianExplicit, false, "", 0); } catch (OrthancException& e) {
--- a/OrthancServer/Resources/Configuration.json Wed Jan 20 17:43:15 2021 +0100 +++ b/OrthancServer/Resources/Configuration.json Thu Jan 21 17:08:32 2021 +0100 @@ -367,6 +367,18 @@ // DICOM SCP (server) does not answer. "DicomScuTimeout" : 10, + // During a C-STORE SCU request initiated by Orthanc, if the remote + // modality doesn't support the original transfer syntax of some + // DICOM instance, specify which transfer syntax should be preferred + // to transcode this instance (provided the remote modality accepts + // this syntax). In Orthanc between 1.7.0 and 1.8.2, this parameter + // was implicitly set to "Little Endian Implicit" + // (1.2.840.10008.1.2). In Orthanc <= 1.6.1 and in Orthanc >= 1.9.0, + // this parameter is by default set to "Little Endian Explicit" + // (1.2.840.10008.1.2.1). This parameter can possibly correspond to + // a compressed transfer syntax. (new in Orthanc 1.9.0) + "DicomScuPreferredTransferSyntax" : "1.2.840.10008.1.2.1", + // The list of the known Orthanc peers. This option is ignored if // "OrthancPeersInDatabase" is set to "true", in which case you must // use the REST API to define Orthanc peers.
--- a/OrthancServer/Sources/ServerContext.cpp Wed Jan 20 17:43:15 2021 +0100 +++ b/OrthancServer/Sources/ServerContext.cpp Thu Jan 21 17:08:32 2021 +0100 @@ -307,6 +307,7 @@ isIngestTranscoding_(false), ingestTranscodingOfUncompressed_(true), ingestTranscodingOfCompressed_(true), + preferredTransferSyntax_(DicomTransferSyntax_LittleEndianExplicit), deidentifyLogs_(false) { try @@ -347,6 +348,7 @@ LOG(WARNING) << "Incoming DICOM instances will automatically be transcoded to " << "transfer syntax: " << GetTransferSyntaxUid(ingestTransferSyntax_); + // New options in Orthanc 1.8.2 ingestTranscodingOfUncompressed_ = lock.GetConfiguration().GetBooleanParameter("IngestTranscodingOfUncompressed", true); ingestTranscodingOfCompressed_ = lock.GetConfiguration().GetBooleanParameter("IngestTranscodingOfCompressed", true); @@ -370,6 +372,7 @@ LOG(INFO) << "Automated transcoding of incoming DICOM instances is disabled"; } + // New options in Orthanc 1.8.2 if (lock.GetConfiguration().GetBooleanParameter("DeidentifyLogs", true)) { deidentifyLogs_ = true; @@ -387,6 +390,17 @@ deidentifyLogs_ = false; CLOG(INFO, DICOM) << "Deidentification of log contents (notably for DIMSE queries) is disabled"; } + + // New option in Orthanc 1.9.0 + if (lock.GetConfiguration().LookupStringParameter(s, "DicomScuPreferredTransferSyntax") && + !LookupTransferSyntax(preferredTransferSyntax_, s)) + { + throw OrthancException(ErrorCode_ParameterOutOfRange, + "Unknown preferred transfer syntax: " + s); + } + + CLOG(INFO, DICOM) << "Preferred transfer syntax for Orthanc C-STORE SCU: " + << GetTransferSyntaxUid(preferredTransferSyntax_); } jobsEngine_.SetThreadSleep(unitTesting ? 20 : 200); @@ -1718,7 +1732,7 @@ } else { - connection.Transcode(sopClassUid, sopInstanceUid, *this, data, dicom.size(), + connection.Transcode(sopClassUid, sopInstanceUid, *this, data, dicom.size(), preferredTransferSyntax_, hasMoveOriginator, moveOriginatorAet, moveOriginatorId); } }
--- a/OrthancServer/Sources/ServerContext.h Wed Jan 20 17:43:15 2021 +0100 +++ b/OrthancServer/Sources/ServerContext.h Thu Jan 21 17:08:32 2021 +0100 @@ -225,6 +225,7 @@ DicomTransferSyntax ingestTransferSyntax_; bool ingestTranscodingOfUncompressed_; bool ingestTranscodingOfCompressed_; + DicomTransferSyntax preferredTransferSyntax_; // New in Orthanc 1.9.0 StoreStatus StoreAfterTranscoding(std::string& resultPublicId, DicomInstanceToStore& dicom,
--- a/OrthancServer/Sources/ServerJobs/DicomModalityStoreJob.cpp Wed Jan 20 17:43:15 2021 +0100 +++ b/OrthancServer/Sources/ServerJobs/DicomModalityStoreJob.cpp Thu Jan 21 17:08:32 2021 +0100 @@ -72,7 +72,7 @@ LOG(WARNING) << "An instance was removed after the job was issued: " << instance; return false; } - + std::string sopClassUid, sopInstanceUid; context_.StoreWithTranscoding(sopClassUid, sopInstanceUid, *connection_, dicom, HasMoveOriginator(), moveOriginatorAet_, moveOriginatorId_);