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

WIP: Dicom Parsing Example Proof-of-Concept #252

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Conversation

agirault
Copy link
Collaborator

@agirault agirault commented Sep 5, 2019

Early work in progress showcasing an example that parses and organizes DICOM instances before reading them with ITK.

Opening this PR mainly to enable discussions. I might create a new one once we have a clearer picture of how this will get integrated into ITK (in-source vs npm dependency), dicomParser vs daikon, etc...

Related discussions: itk discourse, #250, #251
cc: @thewtex, @floryst, @zachmullen, @pieper

@agirault agirault force-pushed the dicom-parsing branch 3 times, most recently from 03c6006 to c41ac4c Compare September 9, 2019 17:14
@agirault agirault force-pushed the dicom-parsing branch 5 times, most recently from c21b417 to 655c321 Compare September 12, 2019 15:52
@agirault
Copy link
Collaborator Author

@pieper would you be able to show me where in dcmjs or OHIF are the instance tags extracted to generate the data necessary for volume visualization (pixel data, spacing, origin, orientation, dimensions, etc...)?

This is how it is done in Girder, leveraging daikon instead of dicom-parser: DicomViewer.vue. It's done slice per slice however (one image data per slice), and there is no 3d space/ z-depth considered.

I believe Slicer does some of that here.

I am asking because the work in this branch can be used to organize patient/study/series and extract the files for a specific series, which are then passed to ITK when are once again read and parsed by DCMTK (with emscripten). Ideally, we would avoid reading and parsing them twice, and just extract the data that matters from the series instead of the list of files: this could then be used as a module by ITK, VTK, Girder, even dcmjs and OHIF if there is any use for it.

@agirault agirault force-pushed the dicom-parsing branch 2 times, most recently from 39783ea to 972ce66 Compare September 13, 2019 15:48
@pieper
Copy link

pieper commented Sep 14, 2019

Hi @agirault -

It's a complex topic, as you know! Especially the Slicer part, since many cases are handled. Happy to talk if over if that would help.

Yes, for sure we should be treating all of this consistently and efficiently and I appreciate the effort you are putting into it.

We typically rely on the server to sort patient/study/series in the database (e.g. https://github.com/dcmjs-org/dicomweb-server uses CouchDB and Slicer uses SQLite via CTK https://github.com/commontk/CTK/tree/master/Libs/DICOM/Core.)

Once you have the instances for a series, in dcmjs the mapping to volumes is here:

https://github.com/dcmjs-org/dcmjs/blob/master/src/normalizers.js

The idea is that a "normalized" dicom is a multiframe conversion, often referred to as a 'dataset' in the code. This has all the image data in an array that is effectively exactly what ITK and VTK expect as pixel arrays.

Here's the code to map the dataset to a vtkImageData, which is simple as you can see.

https://github.com/dcmjs-org/dcmjs/blob/master/examples/vtkDisplay/index.html#L448-L475

I'm going to put on my opinionated hat here, and say that dcmjs does things the right way. It is my favorite of all the implementations for a lot of reasons (that I'm happy to describe at length if prompted!). In a nutshell dcmjs reflects a more comprehensive perspective on various quantitative imaging use cases.

-Steve

@agirault
Copy link
Collaborator Author

agirault commented Sep 16, 2019

Hi @pieper. Thanks a lot for sharing this.

Once you have the instances for a series, in dcmjs the mapping to volumes is here. The idea is that a "normalized" dicom is a multiframe conversion, often referred to as a 'dataset' in the code. This has all the image data in an array that is effectively exactly what ITK and VTK expect as pixel arrays. I'm going to put on my opinionated hat here, and say that dcmjs does things the right way.

Indeed, it seems dcmjs does it right, and I don't want to reinvent the wheel so I'll hold off on continuing implementing this in this current example.

We typically rely on the server to sort patient/study/series in the database (e.g. dicomweb-server uses CouchDB and Slicer uses SQLite via CTK.)

We need to be able to do this sorting client-side since in our use-case the user uploads DICOM files and selects a series that will be rendered on the client, and eventually sent over to the server afterward. My current proof-of-concept in this branch achieves that by leveraging dicomParser: do you see a way we could integrate that in the dcmjs workflow, to be able to combine the parsing of dicomParser and the dcmjs normalization of the selected series all on the client?

Also worth noting that the current implementation I have in this branch is about 6x (with reslice intercept and/or slope) to 17x faster (without reslice) than the ITK readImageDICOMFileSeries for reading the files, parsing them, and extracting the pixel data as a typed array.

@agirault
Copy link
Collaborator Author

agirault commented Sep 23, 2019

FYI @finetjul @aylward

@agirault agirault changed the title WIP: Dicom Parsing Example WIP: Dicom Parsing Example Proof-of-Concept Sep 23, 2019
@pieper
Copy link

pieper commented Sep 23, 2019

@agirault sorry for the slow reply, I was out last week.

Regarding the browser-centric workflow I entirely agree. I'd still stick with the database approach to organizing the series though, for consistency. We haven't fleshed it out yet, but I would like to see the same dicomweb-server code applied via PouchDB so that the 'client' side of the browser can use DICOMweb API calls even though it is talking to a local facade.

I agree that doing this in native javascript is a better idea than using cross-compiled C++. Not just for speed, but also because there are a lot of use cases for DICOM that still need work to represent well and there's no reason to limit ourselves to what has been implemented for imaging in the past. We can address more general quantitative imaging tasks like segmentation and annotation more cleanly using the dcmjs approach.

Would you be interested in having a chat sometime to compare notes? There are several projects that probably share some core needs and maybe we can make progress together.

@agirault agirault force-pushed the dicom-parsing branch 2 times, most recently from b9ab3c6 to 96ebc58 Compare February 13, 2020 17:00
@agirault
Copy link
Collaborator Author

agirault commented Feb 13, 2020

Updated to handle rescale intercept, rescale slope, and masking, and added a chrono to log timings and compare. cc: @thewtex @floryst @aylward

@agirault
Copy link
Collaborator Author

Updated to speed up rescaling and parsing (less function calls, more classic loops), thanks @floryst for the suggestions.

@agirault agirault force-pushed the dicom-parsing branch 3 times, most recently from ca29579 to d91f87f Compare April 9, 2020 21:16
Alexis Girault added 12 commits April 21, 2020 17:08
Not when instantiating a DICOMEntity class
TODO: update the tag dictionary to associate the tags with a module
(patient, study, serie or image), to automatically pull in all the tags
for each module, instead of manually listing the tags to extract
(currently that case in the DICOMEntity classes)
Limitations:
- does not support compressed/encapsulated pixel data
(see non-raw dicom transfer syntaxes, ex: JPEG2000)
- assumes slicethickness is spacing
- harcode pixel type and dimension in imageType
const direction = {
rows: 3,
columns: 3,
data: [
Copy link
Contributor

Choose a reason for hiding this comment

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

For me, parsing the same tags and using this code, direction.data ended up being: [...iDirCos, ...jDirCos, ...kDirCos]. Just a heads up!

Copy link
Member

Choose a reason for hiding this comment

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

@WillCMcC thanks for the note!

In itk-wasm, a direction ordering issue was addressed -- it may have addressed this.

@thewtex
Copy link
Member

thewtex commented Mar 30, 2023

For compatibility with DICOM, AWS, etc, the metadata could be structured similar to the ImageSet metadata described here:

https://github.com/aws-samples/amazon-healthlake-imaging-samples/blob/88666ee00a0830b3a7cab403267887b8ff6de662/pixel-data-verification/index.js#L43-L49

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.

None yet

6 participants