diff OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.cpp @ 5442:ac68a4383e51 debug-telemis

improved C-Store negotiation and logging
author Alain Mazy <am@osimis.io>
date Thu, 23 Nov 2023 16:59:16 +0100
parents 99fa307438e1
children 48b8dae6dc77
line wrap: on
line diff
--- a/OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.cpp	Wed Nov 22 21:18:16 2023 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.cpp	Thu Nov 23 16:59:16 2023 +0100
@@ -253,9 +253,7 @@
     const std::string& sopClassUid,
     DicomTransferSyntax transferSyntax,
     bool hasPreferred,
-    DicomTransferSyntax preferred,
-    bool alwaysRenegotiate,
-    bool enableLogs)
+    DicomTransferSyntax preferred)
   {
     /**
      * Step 1: Check whether this presentation context is already
@@ -264,34 +262,33 @@
 
     if (LookupPresentationContext(presentationContextId, sopClassUid, transferSyntax))
     {
+      CLOG(INFO, DICOM) << "Found an accepted presentation context for SOPClassUID " << sopClassUid << " and transfer syntax " << GetTransferSyntaxUid(transferSyntax);
       return true;
     }
 
     // The association must be re-negotiated
     if (association_->IsOpen())
     {
-      if (enableLogs)
-      {
-        CLOG(INFO, DICOM) << "Re-negotiating DICOM association with "
-                          << parameters_.GetRemoteModality().GetApplicationEntityTitle()
-                          << " for SOPClassUID " << sopClassUid;
-      }
+      CLOG(INFO, DICOM) << "No accepted presentation context found, re-negotiating DICOM association with "
+                        << parameters_.GetRemoteModality().GetApplicationEntityTitle()
+                        << " for SOPClassUID " << sopClassUid << " TransferSyntax =" << GetTransferSyntaxUid(transferSyntax);
 
-      // Don't renegociate if we know that the remote modality was
+      // Check if we know that the remote modality was
       // already proposed this individual transfer syntax (**)
-      if (!alwaysRenegotiate && 
-          proposedOriginalClasses_.find(std::make_pair(sopClassUid, transferSyntax)) != proposedOriginalClasses_.end())
+      if (proposedOriginalClasses_.find(std::make_pair(sopClassUid, transferSyntax)) != proposedOriginalClasses_.end())
       {
-        if (enableLogs)
-        {
-          CLOG(INFO, DICOM) << "The remote modality has already rejected SOP class UID \""
-                            << sopClassUid << "\" with transfer syntax \""
-                            << GetTransferSyntaxUid(transferSyntax) << "\", don't renegotiate";
-        }
-
-        return false;
+        CLOG(INFO, DICOM) << "The remote modality has already rejected SOP class UID \""
+                          << sopClassUid << "\" with transfer syntax \""
+                          << GetTransferSyntaxUid(transferSyntax) << "\", but we will renegotiate anyway";
+        // always renegotiating since 1.12.2 // return false;
       }
     }
+    else
+    {
+      CLOG(INFO, DICOM) << "Negotiating DICOM association with "
+                        << parameters_.GetRemoteModality().GetApplicationEntityTitle()
+                        << " for SOPClassUID " << sopClassUid << " TransferSyntax =" << GetTransferSyntaxUid(transferSyntax);
+    }
 
     association_->ClearPresentationContexts();
     proposedOriginalClasses_.clear();
@@ -379,15 +376,16 @@
                                        DcmFileFormat& dicom,
                                        bool hasMoveOriginator,
                                        const std::string& moveOriginatorAET,
-                                       uint16_t moveOriginatorID,
-                                       bool alwaysRenegotiate)
+                                       uint16_t moveOriginatorID)
   {
     DicomTransferSyntax transferSyntax;
     LookupParameters(sopClassUid, sopInstanceUid, transferSyntax, dicom);
 
+    LOG(INFO) << "Performing C-Store on instance of SOPClassUID '" << sopClassUid << "'";
+
     uint8_t presID;
     if (!NegotiatePresentationContext(presID, sopClassUid, transferSyntax, proposeUncompressedSyntaxes_,
-                                      DicomTransferSyntax_LittleEndianExplicit, alwaysRenegotiate, true))
+                                      DicomTransferSyntax_LittleEndianExplicit))
     {
       throw OrthancException(ErrorCode_NetworkProtocol,
                              "No valid presentation context was negotiated for "
@@ -469,8 +467,7 @@
                                        size_t size,
                                        bool hasMoveOriginator,
                                        const std::string& moveOriginatorAET,
-                                       uint16_t moveOriginatorID,
-                                       bool alwaysRenegotiate)
+                                       uint16_t moveOriginatorID)
   {
     std::unique_ptr<DcmFileFormat> dicom(
       FromDcmtkBridge::LoadFromMemoryBuffer(buffer, size));
@@ -480,7 +477,7 @@
       throw OrthancException(ErrorCode_InternalError);
     }
     
-    Store(sopClassUid, sopInstanceUid, *dicom, hasMoveOriginator, moveOriginatorAET, moveOriginatorID, alwaysRenegotiate);
+    Store(sopClassUid, sopInstanceUid, *dicom, hasMoveOriginator, moveOriginatorAET, moveOriginatorID);
   }
 
 
@@ -489,18 +486,20 @@
                                                    const std::string& sopClassUid,
                                                    DicomTransferSyntax sourceSyntax,
                                                    bool hasPreferred,
-                                                   DicomTransferSyntax preferred,
-                                                   bool alwaysRenegotiate)
+                                                   DicomTransferSyntax preferred)
   {
     acceptedSyntaxes.clear();
+    std::map<DicomTransferSyntax, uint8_t> contexts;
 
     // Make sure a negotiation has already occurred for this transfer
-    // 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, hasPreferred, preferred, alwaysRenegotiate, false);
+    // syntax if we have not negotiated yet. 
+    // We don't use the return code: Transcoding is possible even if the "sourceSyntax" is not supported.
+    if (!association_->IsOpen() || !association_->LookupAcceptedPresentationContext(contexts, sopClassUid))
+    {
+      uint8_t presID;
+      NegotiatePresentationContext(presID, sopClassUid, sourceSyntax, hasPreferred, preferred);
+    }
 
-    std::map<DicomTransferSyntax, uint8_t> contexts;
     if (association_->LookupAcceptedPresentationContext(contexts, sopClassUid))
     {
       for (std::map<DicomTransferSyntax, uint8_t>::const_iterator
@@ -522,8 +521,7 @@
                                            DicomTransferSyntax preferredTransferSyntax,
                                            bool hasMoveOriginator,
                                            const std::string& moveOriginatorAET,
-                                           uint16_t moveOriginatorID,
-                                           bool alwaysRenegotiate)
+                                           uint16_t moveOriginatorID)
   {
     std::unique_ptr<DcmFileFormat> dicom(FromDcmtkBridge::LoadFromMemoryBuffer(buffer, size));
     if (dicom.get() == NULL ||
@@ -536,19 +534,19 @@
     LookupParameters(sopClassUid, sopInstanceUid, sourceSyntax, *dicom);
 
     std::set<DicomTransferSyntax> accepted;
-    LookupTranscoding(accepted, sopClassUid, sourceSyntax, true, preferredTransferSyntax, alwaysRenegotiate);
+    LookupTranscoding(accepted, sopClassUid, sourceSyntax, true, preferredTransferSyntax);
 
     if (accepted.size() == 0)
     {
-      throw OrthancException(ErrorCode_NoPresentationContext, "Cannot store instance of SOPClassUID " + 
-                              sopClassUid + ", the destination has not accepted any TransferSyntax for this SOPClassUID.");
+      throw OrthancException(ErrorCode_NoPresentationContext, "Cannot C-Store an instance of SOPClassUID " + 
+                             sopClassUid + ", the destination has not accepted any TransferSyntax for this SOPClassUID.");
     }
 
     if (accepted.find(sourceSyntax) != accepted.end())
     {
       // No need for transcoding
       Store(sopClassUid, sopInstanceUid, *dicom,
-            hasMoveOriginator, moveOriginatorAET, moveOriginatorID, alwaysRenegotiate);
+            hasMoveOriginator, moveOriginatorAET, moveOriginatorID);
     }
     else
     {
@@ -564,6 +562,8 @@
       bool isDestructiveCompressionAllowed = false;
       std::set<DicomTransferSyntax> attemptedSyntaxes;
 
+      LOG(INFO) << "Transcoding is required to C-Store an instance of SOPClassUID '" << sopClassUid << "', preferredTransferSyntax is " << GetTransferSyntaxUid(preferredTransferSyntax);
+
       if (accepted.find(preferredTransferSyntax) != accepted.end())
       {
         // New in Orthanc 1.9.0: The preferred transfer syntax is
@@ -637,7 +637,7 @@
         else
         {
           Store(sopClassUid, sopInstanceUid, transcoded.GetParsed(),
-                hasMoveOriginator, moveOriginatorAET, moveOriginatorID, alwaysRenegotiate);
+                hasMoveOriginator, moveOriginatorAET, moveOriginatorID);
         }
       }
       else
@@ -667,11 +667,10 @@
                                            size_t size,
                                            bool hasMoveOriginator,
                                            const std::string& moveOriginatorAET,
-                                           uint16_t moveOriginatorID,
-                                           bool alwaysRenegotiate)
+                                           uint16_t moveOriginatorID)
   {
     Transcode(sopClassUid, sopInstanceUid, transcoder, buffer, size, DicomTransferSyntax_LittleEndianExplicit,
-              hasMoveOriginator, moveOriginatorAET, moveOriginatorID, alwaysRenegotiate);
+              hasMoveOriginator, moveOriginatorAET, moveOriginatorID);
   }
 #endif
 }