feat: add support for WADO-URI Streaming Volume Loading #354
Conversation
…ated to server or client implementation.
✅ Deploy Preview for cornerstone-3d-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
Of note, the image loading using wado-uri from dcmjs.org is way slower than wado-rs, slower I would think than could be explained by just the larger file size and parsing time. I think at least some of this is related to the server but not sure. It seems to be faster to load directly from and s3 bucket but I haven't done extensive testing here either. |
| */ | ||
| const indexesToPrefetch = [0, middleImageIndex, imageIds.length - 1]; | ||
| for (let i of indexesToPrefetch) { | ||
| await imageLoader.loadImage(imageIds[i], { skipCreateImage: true }).then( |
There was a problem hiding this comment.
I see what you are doing here. Just an FYI we have shifted away from using loadImage directly to using imageLoadPoolManager to handle it (to respect the priority of the queue). You can see an example in the Streaming WADO RS loader. Basically we create a callback (that uses loadImage), but the job is run through imageLoadPoolManager
There was a problem hiding this comment.
One more thing, the { skipCreateImage: true } doesn't do anything at all as the second parameter. In my IDE it even makes it underline red, since it is not matching the type.
There was a problem hiding this comment.
So when adding requests to the imageLoadPoolManager, is there a way to await those prefetched images? The reason is we have to have the metadata for those images cached before going on to construct the volume.
There was a problem hiding this comment.
I just pushed a commit that moves this prefetching logic to cornerstoneStreamingImageVolumeLoader.ts and detects the scheme used. Also now using imageLoadPoolManager as suggested. This seems much cleaner, the only thing is that I had to move all the logic in that file into an async function so that I could await the file prefetches before constructing the volume. The function already returned a promise, so that's ok I think, but now the .decache() and .cancel() functions are also async instead of sync.
packages/streaming-image-volume-loader/src/helpers/sortImageIdsAndGetSpacing.ts
Outdated
Show resolved
Hide resolved
packages/streaming-image-volume-loader/src/helpers/sortImageIdsAndGetSpacing.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I run this and it worked (it gave error for the instanceUIDs, but I got it fixed), the rendering worked fine. So great first iteration I would say
I think since it is streaming into the volume, it belongs to the same package streaming-image-volume-loader. I really like the idea that you are using the same loader scheme cornerstoneStreamingImageVolume, and we don't really care about wadoURI vs wadoRS at volume loader level.
The parts that we need to decide where to go are
-
imageLoader.loadImagefor the three imageIds: I guess thecornerstoneStreamingImageVolumeLoadercan identify the wadoURI scheme and call the load via imageLoadPoolManager before calling thesortImageIdsAndGetSpacing -
Custom extended metadata provider: This one is tricky. I would like to get @wayfarer3130 view on this
|
As for the metadata provider - I am not 100% sure it is necessary, it would only be needed if there is a code path that requests an image's metadata before that image has loaded into the volume. If it only requests metadata from the 1st, middle, and possibly last images prior to loading, those will be prefetched before volume construction and the built-in metadata provider in cswid should be sufficient. |
|
It looks like |
Don't hit cache to determine if all image metadata is preloaded, just detect scheme Don't fall back to 'spacingBetweenSlices'
| return { | ||
| promise: Promise.resolve(streamingImageVolume), | ||
| promise: getStreamingImageVolume(), |
There was a problem hiding this comment.
can we say before return something like const streamingImageVolume = await getStreaming .... and then use it below directly like before?
There was a problem hiding this comment.
Yes -- how about const streamingImageVolumePromise = getStreaming... so that it's explicit? Will defer to your preference.
Remove custom metadata provider
sedghi
changed the title
|
@jmhmd Awesome contribution! |
|
Thanks for merging and the great feedback! Very thankful for the amazing work you all are doing with this library. |
…s#354) * Working wadoURI loading * Working image loading with wadouri, but slow. Not sure if this is related to server or client implementation. * Fix wayward closing paren * Remove unnecessary cacheCheck param from metadata-provider * Refactor extending metadata-provider so it's actually working. * Move the wadouri prefetching logic Don't hit cache to determine if all image metadata is preloaded, just detect scheme Don't fall back to 'spacingBetweenSlices' * Move wadoURI test imageids to demo helpers Remove custom metadata provider * Remove instanceUIDs.ts from example file - moved to helpers * add example to website * fix build
Description
This PR adds support for streaming images into a volume with wadouri, i.e. a static set of dicom part 10 files on a server with a list of URIs pointing to those files.
Motivation
This may be helpful in the case where you have not extracted the series level metadata ahead of time, and you do not (or cannot) set up a dicomweb wado-rs server.
Overview of changes
The main difference is that with wadouri, you do not have a way to pre-fetch the metadata for the entire series/volume ahead of time in order to construct the volume, calculate slice spacing, reorder the images, etc.
So for this to work, we:
Limitations
This is a draft, and I have confirmed that loading works against the dcmjs.org dicomweb server in the provided example file but have not done testing with other more involved examples. I wanted to see if this approach is reasonable or if there are suggestions for improvements before adding more examples/tests.