Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable metadata loading from images #3935

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

gpasquie
Copy link

Context

The goal of this PR is to allow the MetadataProvider to read the metadata of dicom images directly without making an extra WADO-RS Metadata request to the PACS.
Internaly it uses dataSetCacheManager of dicomImageLoader to access the DICOM dataset. Then dcmjs parse the dataset and add the metadata to the MetadataStore.
This PR allow the DicomJSON source to provide only the image UID + instance number without the need of putting all the image metadata into the final .json.

You can enable or disable this behavior with the variable fullmetadataset in the JSON datasource.

Changes & Results

Testing

This can be tested using a regular JSON DataSource where only SeriesInstanceUID, StudyInstanceUID, SOPClassUID and InstanceNumber must be present.

Checklist

PR

  • My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

  • My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • [] The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

  • OS: Windows 10
  • Browser: Chrome 121.0.6167.161

Copy link

netlify bot commented Feb 15, 2024

Deploy Preview for ohif-platform-docs ready!

Name Link
🔨 Latest commit 699169a
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-docs/deploys/65cdd7c4dfd27b0008325c19
😎 Deploy Preview https://deploy-preview-3935--ohif-platform-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Feb 15, 2024

Deploy Preview for ohif-dev canceled.

Name Link
🔨 Latest commit 699169a
🔍 Latest deploy log https://app.netlify.com/sites/ohif-dev/deploys/65cdd7c4b623060008277a75

@gpasquie
Copy link
Author

This is a new version of PR #3573. Sorry for the long delay. I tried to take your comments into consideration but not sure if this was exactly what you suggested.

Copy link

codecov bot commented Feb 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (40b590d) 44.44% compared to head (699169a) 44.44%.
Report is 3 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3935   +/-   ##
=======================================
  Coverage   44.44%   44.44%           
=======================================
  Files          80       80           
  Lines        1332     1332           
  Branches      327      327           
=======================================
  Hits          592      592           
  Misses        587      587           
  Partials      153      153           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40b590d...699169a. Read the comment docs.

Copy link
Member

@sedghi sedghi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please provide a public example on how to test this? like can you provide the dicomJSON json file? or at least the structure of it?

@gpasquie
Copy link
Author

Sure. This can be tested using the following JSON datasource :
LIDC-IDRI-0002.json
and using URL: http://localhost:3000/viewer/dicomjson?url=http://localhost:3000/LIDC-IDRI-0002.json

I started from this JSON file : https://ohif-dicom-json-example.s3.amazonaws.com/LIDC-IDRI-0001.json
and just removed all images metadata which are not usually present in a DICOM KOS.

Copy link
Member

@sedghi sedghi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR again,

I'm trying to understand the problem that this PR is solving. I see that you are still using the DICOM JSON format to provide the Study/Series/SOP UIDs, so essentially you are providing the metadata. I am more interested in providing a manifest, and then the viewer loads the rest.

see this example

https://storage.googleapis.com/idc-volview-pilot/idc-test.json

From this project which is for Volview.

I find that really nice and believe it addresses a significant issue.

Do you think this is something you could work on?

@gpasquie
Copy link
Author

In fact, in our project, we are working in an XDS like environment. A central registry gathers pointers (DICOM KOS) to DICOM images which are stored in different PACS. When the user wants to view the images, the first step is to retrieve the DICOM KOS and then to open OHIF. From the DICOM KOS, it's quite easy to generate the OHIF JSON datasource, except for the images metadata (rows, columns,…), which are not usually present in a KOS. This is why we submitted this PR, to be able to open OHIF with a reduced JSON datasource, and without too many changes in OHIF code.

Opening OHIF from a manifest containing only paths to DICOM images would require much more changes in OHIF in my opinion and I'm not sure we would have the bandwidth to do so. If you think this is less complex that I think, could you please provide some guidance, and we can try to have a look

@sedghi
Copy link
Member

sedghi commented Mar 13, 2024

Ok good to know, then I'll review this

@gpasquie
Copy link
Author

Any feedbacks/reviews on this PR ?

@sedghi
Copy link
Member

sedghi commented May 29, 2024

This was on my radar, i will get to it this week, sorry for the delay

@sedghi sedghi self-assigned this May 29, 2024
@kayone
Copy link

kayone commented Jun 3, 2024

Question about getting the viewer to load purely based on an array of DICOM URLs like @sedghi mentioned https://storage.googleapis.com/idc-volview-pilot/idc-test.json

Couldn't it be piped down using a similar code to the local mode (https://viewer.ohif.org/local), but instead of drag and drop, just load the files from a JSON file and pass them to the FileLoaderService?

@sedghi, if you provide some guidance, we could definitely attempt a PR that would implement this.

@sedghi
Copy link
Member

sedghi commented Jun 7, 2024

@kayone and @ sebastian

Here is what I think is a good way to implement it. I think we cannot and should not support the flat URLs since it will take a lot of time to load the whole thing to extract the metadata. So a middle ground would be like this:

{
  "studies": [
    {
      "series": [
        // series 1
        {
          "instances": [
            {
              "url": "dicomweb:https://ohif-dicom-json-example.s3.amazon/1-001.dcm"
            },
            {
              "url": "dicomweb:https://ohif-dicom-json-example.s3.amazon/1-002.dcm"
            },
            {
              "url": "dicomweb:https://ohif-dicom-json-example.s3.amazon/1-003.dcm"
            },
          ]
        },
        // series 2
        {
          "instances": [
            {
              "url": "dicomweb:https://ohif-dicom-json-example.s3.amazon/1-001.dcm"
            },
            {
              "url": "dicomweb:https://ohif-dicom-json-example.s3.amazon/1-002.dcm"
            },
            {
              "url": "dicomweb:https://ohif-dicom-json-example.s3.amazon/1-003.dcm"
            },
          ]
        }
      ],
    }
  ]
}

The series are already grouped, which makes sense since on servers, the DICOMs for a series are typically stored in a folder.

Then there's the issue of sorting. If the instances inside the folder are sorted (by imagePositionPatient), we're good, and we can render one URL from each series to obtain the common metadata, and the rest should work out of the box (we can assume defaults for the imagePositionPatient and cornerstone would just render them properly and since they are already ordered then we just put them one after another)

However, if they are not properly sorted, we need to load all instances of a series to render them in order based on imagePositionPatient, which takes more time. Unfortunately, there's no better alternative.

I hope this explanation makes sense for you to proceed. Since it's not a priority for us at the moment, let me know if you have any questions, and feel free to message me on Slack so I can assist you with this new feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants