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

improvement: Add pixel data manipulation feature (#169) #182

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

improvement: Add pixel data manipulation feature (#169) #182

wants to merge 5 commits into from

Conversation

WoonchanCho
Copy link
Contributor

Hi @pieper and maintainers,
This is my first effort for implementing DICOM pixel manipulation.
All the related sources for this feature are under new folders (/src/pixelModifier and /libs).

In the current version, you can draw a rectangle (to hide PHI in most cases)
...
const dicomDict = DicomMessage.readFile(buffer);
const pixelModifier = new PixelModifier(dicomDict);
pixelModifier.draw("rectangle", { left:100, top, 200, right: 200, bottom: 200});
...

The /src/pixelModifer consists of three part

  1. decoding - Decode pixel data according to the transfer syntax uid.
    Many of the sources for decoding have been ported from https://github.com/cornerstonejs/cornerstoneWADOImageLoader, which looks very stable.
  2. drawing - Draw rectangle in the pixel
    Currently only "drawing rectangle" is supported but other shapes can be easily added when needed.
  3. encoding - Encode pixel data (only has a skeleton at the moment)
    Encoding is not fully implemented yet. All the decoded pixels will be saved to ExplicitVRLittleEndian.

What to do next would be

  • to utilize web workers
  • to implement an option to encode back to original format

* Decode pixel data according to the transfer syntax uid.
* Draw rectangle in the pixel
* Encode pixel data (only has a skeleton at the moment)
@WoonchanCho
Copy link
Contributor Author

Forgot to mention the supported transfer syntax uids.
The following 13 uids are supported.
I tried to test as many cases as I can, but it's really hard to test every case and I'm kind of new to DICOM world:). So there should be something to improve.

ImplicitVRLittleEndian: "1.2.840.10008.1.2", // Implicit VR Little Endian: Default Transfer Syntax for DICOM
ExplicitVRLittleEndian: "1.2.840.10008.1.2.1", // Explicit VR Little Endian
DeflatedExplicitVRLittleEndian: "1.2.840.10008.1.2.1.99", // Deflated Explicit VR Little Endian
ExplicitVRBigEndian: "1.2.840.10008.1.2.2", // 	Explicit VR Big Endian (Retired)
JPEGBaseline8Bit: "1.2.840.10008.1.2.4.50", // JPEG Baseline (Process 1): Default Transfer Syntax for Lossy JPEG 8 Bit Image Compression
JPEGExtended12Bit: "1.2.840.10008.1.2.4.51", // JPEG Extended (Process 2 & 4): Default Transfer Syntax for Lossy JPEG 12 Bit Image Compression (Process 4 only)
JPEGLossless: "1.2.840.10008.1.2.4.57", // JPEG Lossless, Non-Hierarchical (Process 14)
JPEGLosslessSV1: "1.2.840.10008.1.2.4.70", // JPEG Lossless, Non-Hierarchical, First-Order Prediction (Process 14 [Selection Value 1]): Default Transfer Syntax for Lossless JPEG Image Compression
JPEGLSLossless: "1.2.840.10008.1.2.4.80", // JPEG-LS Lossless Image Compression
JPEGLSNearLossless: "1.2.840.10008.1.2.4.81", // JPEG-LS Lossy (Near-Lossless) Image Compression
JPEG2000Lossless: "1.2.840.10008.1.2.4.90", // JPEG 2000 Image Compression (Lossless Only)
JPEG2000: "1.2.840.10008.1.2.4.91", // JPEG 2000 Image Compression
RLELossless: "1.2.840.10008.1.2.5" // RLE Lossless

@pieper
Copy link
Collaborator

pieper commented Mar 2, 2021

This looks really good @WoonchanCho 👍

I wonder if copying with attribution as you have done is the best approach as opposed to referencing an upstream package, whereby it might be easier to manage versioning.

E.g. did you look at these? https://github.com/chafey/dicom-codec-js ? Perhaps @chafey can advise on a strategy here.

@WoonchanCho
Copy link
Contributor Author

@pieper,
That is a kind of interim solution. I had not found a single good codec library that handle major images types supported by DICOM.
It seems like @chafey is doing amazing work. it's is what I've been looking for.
Once this is fully implemented, this will be extremely useful.
I'll keep track of the library and migrate to the library for JPEG-LS decoding first.

@chafey
Copy link

chafey commented Mar 2, 2021

There is a lot of good logic in cornrestoneWADOImageLoader that should be refactored out and away from cornerstoneJS so it can be re-used. dicom-codec-js is one step in that direction and @dannyrb is doing something similar here: https://github.com/cornerstonejs/codecs.

@WoonchanCho
Copy link
Contributor Author

Thank you so much for the info @chafey. I will take a look at the library further, but I think, (at least) in a long run, your dicom-codec-js fits better for dcmjs because it abstracts the dependencies on the parsing libraries, making it easy to use the with objects parsed with dcmjs. Also, dicom-codec-js takes (will take) care of encoding which we will need to use to encode back to the original form after modifying pixels.

@WoonchanCho
Copy link
Contributor Author

WoonchanCho commented Mar 3, 2021

@pieper, chafey's library is not pushed to npm registry yet, chafey and dannyrb will connect and merge their two separate libraries: chafey/dicom-codec-js#1
I do not know how long it would take for them to make their first output.
Do you think if 1) we will need to wait for them or 2) we will push first and then later update once they register the merged library?

@pieper
Copy link
Collaborator

pieper commented Mar 3, 2021

Those guys are both very good so It would be great to use their packages unless they don't have a chance to wrap things up and you need the features sooner. Rather than implementing a workaround here is there anything you can do to help them get their stuff finished up?

@chafey
Copy link

chafey commented Mar 3, 2021

@WoonchanCho give us more than a few hours to figure this out :)

@WoonchanCho
Copy link
Contributor Author

@pieper got it, the existing two libraries are not ready for us to use for some reasons (e.g, not being pushed to the npm registry) although I can manually handle the issues and use either of them. I will wait till chafey and dannyrb will decide something.
@chafey thank you so much, please take your time and feel free to let me know if there is anything for me to help.

- add allowInvalidVRLength (false by default) in the write options
- add a test case for testing this option
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