Mercurial > hg > orthanc-dicomweb

changeset 562:cdc202779e94

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
new Experimental mode for SeriesMetadata
author Alain Mazy <am@osimis.io>
date Thu, 27 Apr 2023 12:24:45 +0200
parents 794dc1f9ea6e
children c9cee5d78798 def2b1e25312
files NEWS Plugin/Configuration.cpp Plugin/Configuration.h Plugin/WadoRs.cpp TODO
diffstat 5 files changed, 96 insertions(+), 30 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Wed Apr 26 15:22:59 2023 +0200
+++ b/NEWS	Thu Apr 27 12:24:45 2023 +0200
@@ -2,7 +2,10 @@
 ===============================
 
 * Support "X-Forwarded-Host" and "X-Forwarded-Proto" headers to compute BulkDataURI.
-* speed-up the studies/../series/../metadata routes by a factor of 3 when in "MainDicomTags" mode.
+* small speed-up the studies/../series/../metadata route when in "MainDicomTags" mode.
+* new "Experimental" mode for "SeriesMetadata" that is fetching tags from the DB or
+  from the storage if they are missing (useful if you recently changed the "ExtraMainDicomTags"
+  configuration)
 
 
 Version 1.13 (2023-02-03)
--- a/Plugin/Configuration.cpp	Wed Apr 26 15:22:59 2023 +0200
+++ b/Plugin/Configuration.cpp	Thu Apr 27 12:24:45 2023 +0200
@@ -707,6 +707,7 @@
       static const std::string FULL = "Full";
       static const std::string MAIN_DICOM_TAGS = "MainDicomTags";
       static const std::string EXTRAPOLATE = "Extrapolate";
+      static const std::string EXPERIMENTAL = "Experimental";
       
       std::string key;
       switch (level)
@@ -737,6 +738,10 @@
       {
         return MetadataMode_Extrapolate;
       }
+      else if (value == EXPERIMENTAL)
+      {
+        return MetadataMode_Experimental;
+      }
       else
       {
         throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange,
--- a/Plugin/Configuration.h	Wed Apr 26 15:22:59 2023 +0200
+++ b/Plugin/Configuration.h	Thu Apr 27 12:24:45 2023 +0200
@@ -49,7 +49,13 @@
   {
     MetadataMode_Full,           // Read all the DICOM instances from the storage area
     MetadataMode_MainDicomTags,  // Only use the Orthanc database (main DICOM tags only)
-    MetadataMode_Extrapolate     // Extrapolate user-specified tags from a few DICOM instances
+    MetadataMode_Extrapolate,    // Extrapolate user-specified tags from a few DICOM instances
+
+    MetadataMode_Experimental    // Retrieves all ExtraMainDicomTags from DB or from the storage if required.
+                                 // In terms of performance: This is equivalent to MainDicomTags mode for studies 
+                                 // that have been stored with a correct ExtraMainDicomTags.  This is equivalent
+                                 // to Full mode for older studies that have note been reconstructed.
+
   };
 
 
--- a/Plugin/WadoRs.cpp	Wed Apr 26 15:22:59 2023 +0200
+++ b/Plugin/WadoRs.cpp	Thu Apr 27 12:24:45 2023 +0200
@@ -29,12 +29,30 @@
 #include <Toolbox.h>
 
 #include <memory>
-
+#include <boost/thread/mutex.hpp>
 
 static const char* const MAIN_DICOM_TAGS = "MainDicomTags";
+static const char* const REQUESTED_TAGS = "RequestedTags";
 static const char* const INSTANCES = "Instances";
 static const char* const PATIENT_MAIN_DICOM_TAGS = "PatientMainDicomTags";
 
+static std::string instancesMainDicomTagsList;
+static boost::mutex mainDicomTagsListMutex;
+
+static const std::string& GetInstancesMainDicomTagsList()
+{
+  boost::mutex::scoped_lock lock(mainDicomTagsListMutex);
+  
+  if (instancesMainDicomTagsList.empty())
+  {
+    Json::Value system;
+    OrthancPlugins::RestApiGet(system, "/system", false);
+
+    instancesMainDicomTagsList = system[MAIN_DICOM_TAGS]["Instance"].asString();
+  }
+
+  return instancesMainDicomTagsList;
+}
 
 static std::string GetResourceUri(Orthanc::ResourceType level,
                                   const std::string& publicId)
@@ -689,19 +707,31 @@
 
     bool GetInstance(Orthanc::DicomMap& dicom,
                      OrthancPlugins::MetadataMode mode,
-                     const std::string& orthancId)
+                     const std::string& instanceOrthancId)
     {
-      std::string seriesId, studyId, nope;
+      std::string seriesOrthancId, studyOrthancId, nope;
       
-      return (ReadResource(dicom, seriesId, mode, orthancId, Orthanc::ResourceType_Instance) &&
-              Lookup(dicom, studyId, mode, seriesId, Orthanc::ResourceType_Series) &&
-              Lookup(dicom, nope /* patient id is unused */, mode, studyId, Orthanc::ResourceType_Study));
+      return (ReadResource(dicom, seriesOrthancId, mode, instanceOrthancId, Orthanc::ResourceType_Instance) &&
+              Lookup(dicom, studyOrthancId, mode, seriesOrthancId, Orthanc::ResourceType_Series) &&
+              Lookup(dicom, nope /* patient id is unused */, mode, studyOrthancId, Orthanc::ResourceType_Study));
+    }
+
+    bool GetSeries(Orthanc::DicomMap& dicom,
+                   OrthancPlugins::MetadataMode mode,
+                   const std::string& seriesOrthancId)
+    {
+      std::string studyOrthancId, nope;
+      
+      return (Lookup(dicom, studyOrthancId, mode, seriesOrthancId, Orthanc::ResourceType_Series) &&
+              Lookup(dicom, nope /* patient id is unused */, mode, studyOrthancId, Orthanc::ResourceType_Study));
     }
   };
 }
 
 
 static void WriteInstancesMetadataFromMainDicomTags(OrthancPlugins::DicomWebFormatter::HttpWriter& writer,
+                                                    OrthancPlugins::MetadataMode mode,
+                                                    MainDicomTagsCache& cache,
                                                     const std::string& seriesOrthancId,
                                                     const std::string& studyInstanceUid,
                                                     const std::string& seriesInstanceUid,
@@ -714,13 +744,23 @@
 
   Json::Value instances;
 
-  std::string uri = "/series/" + seriesOrthancId + "/instances?full";
+  Orthanc::DicomMap seriesDicom;
+  if (!cache.GetSeries(seriesDicom, mode, seriesOrthancId))
+  {
+    throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError, "unable to get series");
+  }
 
-  if (!OrthancPlugins::RestApiGet(instances, uri, false))
+  std::string seriesInstancesUri = "/series/" + seriesOrthancId + "/instances?full";
+
+  if (mode == OrthancPlugins::MetadataMode_Experimental)
+  {
+    seriesInstancesUri += "&requestedTags=" + GetInstancesMainDicomTagsList();
+  }
+
+  if (!OrthancPlugins::RestApiGet(instances, seriesInstancesUri, false))
   {
     throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError, "unable to get series instances");
   }
-         
 
   if (instances.type() != Json::arrayValue)
   {
@@ -729,10 +769,19 @@
 
   for (Json::ArrayIndex i = 0; i < instances.size(); ++i)
   {
-    Orthanc::DicomMap dicom;
-    dicom.FromDicomAsJson(instances[i][MAIN_DICOM_TAGS], false /* append */, false /* parseSequences */);
-    writer.AddOrthancMap(dicom);
-    //writer.AddOrthancJson(instances[i][MAIN_DICOM_TAGS]);
+    std::unique_ptr<Orthanc::DicomMap> dicom(seriesDicom.Clone());
+    if (mode == OrthancPlugins::MetadataMode_Experimental)
+    {
+      dicom->FromDicomAsJson(instances[i][REQUESTED_TAGS], true /* append */, false /* parseSequences */);
+    }
+    else
+    {
+      dicom->FromDicomAsJson(instances[i][MAIN_DICOM_TAGS], true /* append */, false /* parseSequences */);
+    }
+
+    // TODO_OPTI: optimize this call since it takes around 1ms per instance which is huge for a CT study with 5x1000 instances.
+    writer.AddOrthancMap(*dicom);
+
   }
 }
 
@@ -1132,9 +1181,11 @@
     if (LocateSeries(output, seriesOrthancId, studyInstanceUid, seriesInstanceUid, request))
     {
       OrthancPlugins::DicomWebFormatter::HttpWriter writer(output, isXml);
-      if (mode == OrthancPlugins::MetadataMode_MainDicomTags)
+      if (/*mode == OrthancPlugins::MetadataMode_MainDicomTags ||*/ mode == OrthancPlugins::MetadataMode_Experimental)
       {
         WriteInstancesMetadataFromMainDicomTags(writer,
+                                                mode,
+                                                cache,
                                                 seriesOrthancId,
                                                 studyInstanceUid,
                                                 seriesInstanceUid,
--- a/TODO	Wed Apr 26 15:22:59 2023 +0200
+++ b/TODO	Thu Apr 27 12:24:45 2023 +0200
@@ -13,23 +13,24 @@
   to "MainDicomTags" and "ExtraMainDicomTags" are configured according to recommandation 
   (from this setup: https://bitbucket.org/osimis/orthanc-setup-samples/src/master/docker/stone-viewer/docker-compose.yml).
   
-  with a 600 instance series with SQLite:
+  with a 600 instance series with SQLite - all timings are performed without verbose logs !!!!:
   - time curl http://localhost:8043/dicom-web/studies/1.2.276.0.7230010.3.1.2.1215942821.4756.1664826045.3529/series/1.2.276.0.7230010.3.1.3.1215942821.4756.1664833048.11984/metadata > /dev/null 
-    -> 0.985 second because the plugin makes 600 calls to /instances/...?full 
+    -> 0.750 second because the plugin makes 600 calls to /instances/...?full 
     to retrieve all main dicom tags from the DB (but this also retrieves other 
     data like attachment info + instances metadata)
-  - the content of series/.../metadata could also be obtained alternatively by 
-    calling tools/find that is, in this case: 9 times faster.  However, right now, 
-    it might not return the response in "Full" json format.
-    time curl http://localhost:8043/tools/find --data-binary '{"Level": "Instance", "Query": {"SeriesInstanceUID": "1.2.276.0.7230010.3.1.3.1215942821.4756.1664833048.11984"}, "Expand": true}' > /dev/null 
-    -> 0.178 second
-  - alternate way: 
-    time curl http://localhost:8043/tools/lookup --data-binary '1.2.276.0.7230010.3.1.3.1215942821.4756.1664833048.11984'
-    -> 0.016 second followed by
-    time curl http://localhost:8043/series/fdd31453-fa62d8cb-b681823c-f294d34c-182bca4b/instances?full > /dev/null
-    -> 0.083 second
-    UPDATE: we are now using this method but, the full duration to call the /metadata route is
-    -> 0.335 second (only 3x faster)
+  - In "Experimental" mode, we are calling this route: 
+    time curl "http://localhost:8043/series/fdd31453-fa62d8cb-b681823c-f294d34c-182bca4b/instances?full&requestedTags=0008,0005;0008,0008;0008,0012;0008,0013;0008,0016;0008,0018;0008,0023;0008,0033;0018,0050;0020,0012;0020,0013;0020,0032;0020,0037;0020,0052;0020,0100;0020,4000;0028,0004;0028,0008;0028,0010;0028,0011;0028,0030;0028,0100;0028,0101;0028,0103;0028,1050;0028,1051;0028,1052;0028,1053;0054,1330" > /dev/null
+    -> 0.175 second
+    BUT the total time required by the /metadata route is still around 0.9 second because we spend 600-700ms formating the output !!! (see TODO_OPTI in WadoRs.cpp)
+    -> 0.835 second in "Experimental" mode (10% slower if tags are actually in DB of course !)
+    -> 0.700 second in "MainDicomTags" mode (10% faster)
+    FURTHER WORK: 
+      - In the plugin SDK, we should probably have a method to convert DicomAsJson_Full to DicomWebJson without creating a ParsedDicomFile in between.
+      - Or in the core: implement /series/.../instances?dicom-web-json (the core has the primitives to write dicom-web-json)
+      - Even if we optimize the DB, the cost for the response serialization will still be very high and that's likely the first thing to work on !!!
+     
+
+
 
   - note that all measurements have been performed on a DB with a single series !  We should repeat 
     that with a more realistic DB
\ No newline at end of file