OHIF / Viewers

OHIF zero-footprint DICOM viewer and oncology specific Lesion Tracker, plus shared extension packages
https://docs.ohif.org/
MIT License
3.35k stars 3.36k forks source link

Download data button #1130

Closed pieper closed 4 years ago

pieper commented 5 years ago

Request

What feature or change would you like to see made?

I'd like to be able to download the part10 dicom files for selected instances/series/studies.

Why should we prioritize this feature?

It's useful to be able to load data in other programs, like Slicer or Osirix.

Since the user has access already, it's technically possible for them to download another way, but a button would make it easier.

salimkanoun commented 5 years ago

Could this be made optional, defined in the config parameters of OHIF ?

Depending on the usage you want to make, in some case you might want to avoid users to be able to download a copy of the original dicom.

I know that being client side viewer, the raw dicom will always land anyway on the client computer so there will be always a way to make a copy.

Just, I would be interested to not let users be able to make a copy of source data too easily (at least not giving them the idea to do it)

pieper commented 5 years ago

Yes, for sure it can be optional. Or the functionality could be added via a plugin and not in the standard distribution. But I guess we both agree this is only security by obscurity (or security by inconvenience in this case).

Perhaps another feature could be that if you click the download button it asks you to agree to some kind of terms of use, like don't share this data with anyone else. That would make the arrangement more explicit and might better comply with institutional rules (e.g. remind people that they are only seeing the data because they signed an IRB or data use agreement and will be held responsible if they violate the terms). We could even consider watermarking the headers somehow to track who did the download in case the data shows up someplace unexpected.

salimkanoun commented 5 years ago

To me, the optional settings is enough.

I'm not sure the terms of use will be really usefull as someone who will want to hack the system will just have to download the raw data from the DICOMWeb APIs (even without signing the term of use)

To add a tracking security, it's better to manage that in the backend, in my project I put a reverse proxy to allow communication with DICOMWeb orthanc APIs, so I implemented my log / security in this reverse proxy so I can track each call of these APIs.

However it's a good community question, other ideas might appears...

JamesAPetts commented 5 years ago

This is an interesting one, because more often than not we request the headers and pixeldata seperately, and the pixeldata frame-by-frame for multiframe images.

In a lot of cases it'd be easier to just hook up the button to fetch the full P10 from PACs, but this won't always be available, and duplicates download requests. We could reconstruct the dataset using dcmjs.

pieper commented 5 years ago

Good points - and of course some people may want nifti or nrrd volumes too.

dannyrb commented 5 years ago

In a lot of cases it'd be easier to just hook up the button to fetch the full P10 from PACs, but this won't always be available, and duplicates download requests. We could reconstruct the dataset using dcmjs.

I think we'd probably prefer fetching the original from the PACS / Server? We can make sure extensions have access to a service with enough information to reconstruct DICOMWeb urls, and they could pull in @ohif/core's client to make requests.

The configuration for the extension could provide you similar values, but allow you to override the function that constructs the request URL.

This may make sense as a modification to the Download Tool's modal dialog. Additional actions or a small workflow for more control over the format you want.

fedorov commented 4 years ago

Changed to "IDC:priority", since without this feature it is quite difficult to troubleshoot OHIF issues for specific datasets in google healthcare.

dannyrb commented 4 years ago

Related to #1129

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

dannyrb commented 4 years ago

I started to look into this. I believe this is the endpoint we'll need to use:

We'll need to include OAuth info when making the request (we can't leverage an API key on the client).

The GoogleCloudApi.js at /platform/viewer/src/googleCloud/api could be updated to include a new method for downloading the study, or the DICOMWebClient, with google cloud access token set, should be able to make the appropriate DICOMWeb request.

I'll try creating a small debug / helper extension that adds a toolbar button for study downloads using the configured DICOMWeb client. If that doesn't do the trick, we can fallback to the Google Cloud API methods.


Posting this update to ward of stale bot

pieper commented 4 years ago

Hi @dannyrb - a couple things...

Thoughts?

dannyrb commented 4 years ago

The ability to retrieve a study is already at the DICOMweb API level, so we shouldn't need to do anything google-specific.

I agree! My understanding is that's how it should work. I haven't touched the Google API client yet, but it seems to generate the correct token, set it as a authorization bearer token for the DICOMWeb Client, and then it's business as usual.

But also, why download again if we already have the data in the viewer?

I think that would depend on how the viewer is configured. If we request metadata and image data separately, it may be more trouble than its worth to piece it back together again on the client when we can request from the PACS directly. If we're pulling individual DICOM instances, then we could surface them from our cache when a download is requested.

It all gets a bit trickier when we want to allow download of an entire study or series, as we would need to zip/compress them on the client, sometimes requesting missing files to fill in the gaps.

While there may be some nice performance enhancements hiding there, the PACS API is a faster first pass that gets the job done.

I may be missing something obvious, or this may be a good thing to keep in mind as we work to change how we store/manage study/series/instance data in OHIF core to enable improvements here.

pieper commented 4 years ago

Point 1: Yes, I can confirm you can talk to google's dicomweb with just a token in the header (examples here if you haven't seen: https://colab.research.google.com/drive/17I2QUwlSUb-z6JASKeC3GWBFwP06iNcT).

Point 2: Ah, good points - yes, re-downloading makes sense in this case. I guess if the user is interactively viewing then it's a fair corollary that the download speeds are fast enough that we don't need to spend a lot of effort optimizing.

swederik commented 4 years ago

Write an extension which provides the functionality:

Steps:

Etc:

fedorov commented 4 years ago

@pieper do we have this feature enabled in IDC sandbox? Do you know the key that triggers the download?

pieper commented 4 years ago

We tried enabling the hotkey and it worked for a single series, but failed for downloading a whole study.

Downloading DICOM P10 files for references:
downloadAndZip.js:133 
utils.js:98 Failed to create Zip file... Error: No valid reference to be downloaded
    at downloadAndZip.js:170
    at u (runtime.js:45)
    at Generator._invoke (runtime.js:274)
    at Generator.C.forEach.e.<computed> [as next] (runtime.js:97)
    at Ta (app.bundle.7c5f439055ee3aa94832.js:2)
    at o (app.bundle.7c5f439055ee3aa94832.js:2)
    at app.bundle.7c5f439055ee3aa94832.js:2
    at new Promise (<anonymous>)
    at app.bundle.7c5f439055ee3aa94832.js:2
    at Pa (app.bundle.7c5f439055ee3aa94832.js:2)
JamesAPetts commented 4 years ago

@emfol , I believe you wrote this feature. The above endpoints don't seem to work. Could you test + fix all the endpoints?

emfol commented 4 years ago

@emfol , I believe you wrote this feature. The above endpoints don't seem to work. Could you test + fix all the endpoints?

Sure thing!

emfol commented 4 years ago

Just created a pull request with a fix: https://github.com/OHIF/Viewers/pull/1753 After the changes have been merged, we just need to add something like:

{
  commandName: 'downloadAndZipStudy',
  label: 'Download and Zip Study',
  keys: ['d'],
},

To the platform/viewer/public/config/default.js file and it should work (in this specific case, by using the "d" hot key to trigger the download). Since no UI is available for this feature, information and progress updates about the download will be printed to the console (just look for Download and Zip Progress: and Downloading DICOM P10 files for references:) Please let me know if you have any concerns.

fedorov commented 4 years ago

Related PR with the instructions how to download from console: #1735

Christian-e-S commented 3 years ago

Hi @emfol .

The support for downloading studies as a zip file is just great!

What is the reason, to have this just as a debugging feature. I feel it is really essential for many use cases. In the current version, the usability is quite limited since, without the console, there is no user feedback until the complete study has been downloaded. With a slower Internet connection, it leaves the user without any knowledge whether the pressing of "d" did something at all.

Are you intending to extend the functionality with something like a pop-up that informs the user about the progress?