DCMTK changes that help subclassing

[yiorgos]

Hello,

My name is Yiorgos and I work as a developer at Advantis Medical Imaging.

We have developed a tool based on dcmtk that works as a DICOM to DICOMweb proxy, which we plan to release as free and open source software.

The implementation has been based on a modified version of dcmqrscp.
Now our tool, called dcm2web, receives DICOM requests (as before) but instead of calling the default dcmqrscp handlers, it
wraps the DICOM request and forwards it over HTTP to a DICOMweb endpoint.

Using this tool someone can establish a connection and perform a store, query or get request between a PACS talking DICOM and a PACS or a web servise supporting HTTP DICOMweb.

To develop this tool we had to make some small changes to the original dcmtk source code which make subclassing easier. We needed to subclass in order to override the
default request callback handlers and also the database handler.

On this post I'm providing a link to a PATCH with the changes we have made.

It would be great for us if these changes could be merged to the official dcmtk source code, instead of having to depend on a diverged fork.
The changes are minimal and generic enough, and should help in the development of new tools.

Cheers,
~yiorgos

[yiorgos]

These are the changes we have done:

Code: Select all

diff --git a/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrcbf.h b/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrcbf.h
index cf420c935..d1fc74fba 100644
--- a/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrcbf.h
+++ b/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrcbf.h
@@ -75,7 +75,7 @@ public:
         DcmDataset **responseIdentifiers,
         DcmDataset **stDetail);
 
-private:
+protected:
 
     /// reference to database handle
     DcmQueryRetrieveDatabaseHandle& dbHandle;
diff --git a/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrcbg.h b/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrcbg.h
index e1e5531ca..d8aaac8e1 100644
--- a/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrcbg.h
+++ b/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrcbg.h
@@ -98,7 +98,7 @@ public:
         T_DIMSE_C_GetRSP *response, DcmDataset **stDetail,
         DcmDataset **responseIdentifiers);
 
-private:
+protected:
 
     /// private undefined copy constructor
     DcmQueryRetrieveGetContext(const DcmQueryRetrieveGetContext& other);
diff --git a/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrcbs.h b/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrcbs.h
index c47b4432a..9592f954e 100644
--- a/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrcbs.h
+++ b/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrcbs.h
@@ -87,7 +87,7 @@ public:
         T_DIMSE_C_StoreRSP *rsp,
         DcmDataset **stDetail);
 
-private:
+protected:
 
     void updateDisplay(T_DIMSE_StoreProgress * progress);
 
diff --git a/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrdba.h b/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrdba.h
index 7b9e502f8..c0bf7edd6 100644
--- a/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrdba.h
+++ b/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrdba.h
@@ -192,6 +192,11 @@ public:
    */
   virtual void setIdentifierChecking(OFBool checkFind, OFBool checkMove) = 0;
 
+  /** Make the necessary actions to complete the transaction
+   * with the db or whichever handler you use. It can as a
+   * cleaner after C-FIND, C-STORE, C-GET, C-MOVE requests
+   */
+  virtual OFCondition releaseHandler(void* pValue) = 0;
 };
 
 
diff --git a/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrdbi.h b/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrdbi.h
index dca3a8bf5..a453dcf4a 100644
--- a/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrdbi.h
+++ b/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrdbi.h
@@ -256,6 +256,11 @@ public:
    */
   OFCondition pruneInvalidRecords();
 
+  /** Make necessary actions and release the association
+   *
+   */
+  OFCondition releaseHandler(void* pValue);
+
   // methods not inherited from the base class
 
   /** enable/disable the DB quota system (default: enabled) which causes images
diff --git a/dcmqrdb/include/dcmtk/dcmqrdb/dcmqropt.h b/dcmqrdb/include/dcmtk/dcmqrdb/dcmqropt.h
index 19b03aa62..48a50bf90 100644
--- a/dcmqrdb/include/dcmtk/dcmqrdb/dcmqropt.h
+++ b/dcmqrdb/include/dcmtk/dcmqrdb/dcmqropt.h
@@ -93,6 +93,8 @@ public:
   /// preferred transfer syntax for incoming associations
   E_TransferSyntax  networkTransferSyntax_;
 
+  OFBool            secureConnection_;
+
 #ifndef DISABLE_COMPRESSION_EXTENSION
   /// preferred transfer syntax for outgoing associations
   E_TransferSyntax  networkTransferSyntaxOut_;
@@ -160,6 +162,8 @@ public:
   /// timeout for ACSE operations
   int acse_timeout_;
 
+  /// end of study timeout
+  int endOfStudy_timeout_;
 };
 
 
diff --git a/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrsrv.h b/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrsrv.h
index 5f3ae88bd..aed8a871b 100644
--- a/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrsrv.h
+++ b/dcmqrdb/include/dcmtk/dcmqrdb/dcmqrsrv.h
@@ -85,6 +85,14 @@ public:
     OFBool dbCheckFindIdentifier,
     OFBool dbCheckMoveIdentifier);
 
+  /** Setters for callbacks
+   *
+   */
+  void setFindCallback(DIMSE_FindProviderCallback cbf);
+  void setGetCallback(DIMSE_GetProviderCallback cbg);
+  void setMoveCallback(DIMSE_MoveProviderCallback cbm);
+  void setStoreCallback(DIMSE_StoreProviderCallback cbs);
+
   /** clean up terminated child processes.
    */
   void cleanChildren();
@@ -164,6 +172,11 @@ private:
 
   /// SCP configuration options
   const DcmQueryRetrieveOptions& options_;
+
+  DIMSE_FindProviderCallback findPCallback;
+  DIMSE_StoreProviderCallback storePCallback;
+  DIMSE_GetProviderCallback getPCallback;
+  DIMSE_MoveProviderCallback movePCallback;
 };
 
 #endif
diff --git a/dcmqrdb/libsrc/dcmqrdbi.cc b/dcmqrdb/libsrc/dcmqrdbi.cc
index babcdacfb..e7116601c 100644
--- a/dcmqrdb/libsrc/dcmqrdbi.cc
+++ b/dcmqrdb/libsrc/dcmqrdbi.cc
@@ -3139,6 +3139,11 @@ OFCondition DcmQueryRetrieveIndexDatabaseHandle::pruneInvalidRecords()
     return EC_Normal;
 }
 
+OFCondition DcmQueryRetrieveIndexDatabaseHandle::releaseHandler(void* pValue)
+{
+  return EC_Normal;
+}
+
 
 /* ========================= INDEX ========================= */
 
diff --git a/dcmqrdb/libsrc/dcmqropt.cc b/dcmqrdb/libsrc/dcmqropt.cc
index ed1945cf5..7dca57775 100644
--- a/dcmqrdb/libsrc/dcmqropt.cc
+++ b/dcmqrdb/libsrc/dcmqropt.cc
@@ -41,6 +41,7 @@ DcmQueryRetrieveOptions::DcmQueryRetrieveOptions()
 , maxPDU_(ASC_DEFAULTMAXPDU)
 , net_(NULL)
 , networkTransferSyntax_(EXS_Unknown)
+, secureConnection_(OFFalse)
 #ifndef DISABLE_COMPRESSION_EXTENSION
 ,  networkTransferSyntaxOut_(EXS_Unknown)
 #endif
@@ -71,6 +72,7 @@ DcmQueryRetrieveOptions::DcmQueryRetrieveOptions()
 , blockMode_(DIMSE_BLOCKING)
 , dimse_timeout_(0)
 , acse_timeout_(30)
+, endOfStudy_timeout_(-1)
 {
 }
 
diff --git a/dcmqrdb/libsrc/dcmqrsrv.cc b/dcmqrdb/libsrc/dcmqrsrv.cc
index d0e77fef9..22affd3bc 100644
--- a/dcmqrdb/libsrc/dcmqrsrv.cc
+++ b/dcmqrdb/libsrc/dcmqrsrv.cc
@@ -106,6 +106,10 @@ DcmQueryRetrieveSCP::DcmQueryRetrieveSCP(
 , dbCheckMoveIdentifier_(OFFalse)
 , factory_(factory)
 , options_(options)
+, findPCallback(findCallback)
+, getPCallback(getCallback)
+, movePCallback(moveCallback)
+, storePCallback(storeCallback)
 {
 }
 
@@ -187,6 +191,13 @@ OFCondition DcmQueryRetrieveSCP::dispatch(T_ASC_Association *assoc, OFBool corre
             else if ((cond == DUL_PEERREQUESTEDRELEASE)||(cond == DUL_PEERABORTEDASSOCIATION))
             {
                 // association gone
+                OFCondition cond = dbHandle->releaseHandler(assoc);
+                if(!cond.good()) {
+                    OFString temp_str;
+                    DCMQRDB_ERROR("DIMSE Failure (aborting association): " << DimseCondition::dump(temp_str, cond));
+                    /* some kind of error so abort the association */
+                    cond = ASC_abortAssociation(assoc);
+                }
             }
             else
             {
@@ -278,7 +289,7 @@ OFCondition DcmQueryRetrieveSCP::findSCP(T_ASC_Association * assoc, T_DIMSE_C_Fi
     DCMQRDB_INFO("Received Find SCP:" << OFendl << DIMSE_dumpMessage(temp_str, *request, DIMSE_INCOMING));
 
     cond = DIMSE_findProvider(assoc, presID, request,
-        findCallback, &context, options_.blockMode_, options_.dimse_timeout_);
+        findPCallback, &context, options_.blockMode_, options_.dimse_timeout_);
     if (cond.bad()) {
         DCMQRDB_ERROR("Find SCP Failed: " << DimseCondition::dump(temp_str, cond));
     }
@@ -301,7 +312,7 @@ OFCondition DcmQueryRetrieveSCP::getSCP(T_ASC_Association * assoc, T_DIMSE_C_Get
     DCMQRDB_INFO("Received Get SCP:" << OFendl << DIMSE_dumpMessage(temp_str, *request, DIMSE_INCOMING));
 
     cond = DIMSE_getProvider(assoc, presID, request,
-        getCallback, &context, options_.blockMode_, options_.dimse_timeout_);
+        getPCallback, &context, options_.blockMode_, options_.dimse_timeout_);
     if (cond.bad()) {
         DCMQRDB_ERROR("Get SCP Failed: " << DimseCondition::dump(temp_str, cond));
     }
@@ -324,7 +335,7 @@ OFCondition DcmQueryRetrieveSCP::moveSCP(T_ASC_Association * assoc, T_DIMSE_C_Mo
     DCMQRDB_INFO("Received Move SCP:" << OFendl << DIMSE_dumpMessage(temp_str, *request, DIMSE_INCOMING));
 
     cond = DIMSE_moveProvider(assoc, presID, request,
-        moveCallback, &context, options_.blockMode_, options_.dimse_timeout_);
+        movePCallback, &context, options_.blockMode_, options_.dimse_timeout_);
     if (cond.bad()) {
         DCMQRDB_ERROR("Move SCP Failed: " << DimseCondition::dump(temp_str, cond));
     }
@@ -404,11 +415,11 @@ OFCondition DcmQueryRetrieveSCP::storeSCP(T_ASC_Association * assoc, T_DIMSE_C_S
 
     if (options_.bitPreserving_) { /* the bypass option can be set on the command line */
         cond = DIMSE_storeProvider(assoc, presId, request, imageFileName, (int)options_.useMetaheader_,
-                                   NULL, storeCallback,
+                                   NULL, storePCallback,
                                    (void*)&context, options_.blockMode_, options_.dimse_timeout_);
     } else {
         cond = DIMSE_storeProvider(assoc, presId, request, (char *)NULL, (int)options_.useMetaheader_,
-                                   &dset, storeCallback,
+                                   &dset, storePCallback,
                                    (void*)&context, options_.blockMode_, options_.dimse_timeout_);
     }
 
@@ -930,7 +941,11 @@ OFCondition DcmQueryRetrieveSCP::waitForAssociation(T_ASC_Network * theNet)
 
     if (ASC_associationWaiting(theNet, timeout))
     {
-        cond = ASC_receiveAssociation(theNet, &assoc, (int)options_.maxPDU_);
+        if(options_.endOfStudy_timeout_ == -1)
+            cond = ASC_receiveAssociation(theNet, &assoc, (int)options_.maxPDU_, NULL, NULL, options_.secureConnection_);
+        else
+            cond = ASC_receiveAssociation(theNet, &assoc, (int)options_.maxPDU_, NULL, NULL, options_.secureConnection_, DUL_NOBLOCK, OFstatic_cast(int, options_.endOfStudy_timeout_));
+
         if (cond.bad())
         {
           DCMQRDB_INFO("Failed to receive association: " << DimseCondition::dump(temp_str, cond));
@@ -1098,3 +1113,27 @@ void DcmQueryRetrieveSCP::setDatabaseFlags(
   dbCheckFindIdentifier_ = dbCheckFindIdentifier;
   dbCheckMoveIdentifier_ = dbCheckMoveIdentifier;
 }
+
+void DcmQueryRetrieveSCP::setFindCallback(
+    DIMSE_FindProviderCallback cbf)
+{
+  findPCallback = cbf;
+}
+
+void DcmQueryRetrieveSCP::setGetCallback(
+    DIMSE_GetProviderCallback cbg)
+{
+  getPCallback = cbg;
+}
+
+void DcmQueryRetrieveSCP::setMoveCallback(
+    DIMSE_MoveProviderCallback cbm)
+{
+  movePCallback = cbm;
+}
+
+void DcmQueryRetrieveSCP::setStoreCallback(
+    DIMSE_StoreProviderCallback cbs)
+{
+  storePCallback = cbs;
+}

[Jan Schlamelcher]

I'm not sure that extending dcmqrscp is such a good idea, you should probably look at the DcmSCU and DcmSCP classes instead. I'll test if your patch applies though, since there have been some changes in the code of dcmqrscp during the last few months.