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

Encapsulated Data Without End Delimiter #59

Open
keean opened this issue Feb 16, 2017 · 10 comments
Open

Encapsulated Data Without End Delimiter #59

keean opened this issue Feb 16, 2017 · 10 comments

Comments

@keean
Copy link

keean commented Feb 16, 2017

I have several DICOM images that use encapsulated JPEG, but the pixel-element length is the actual length and not "4294967295". The following change to "dicomParser.readDicomElementExplicit" allows these to be read correctly:

        //if(element.length === 4294967295)
        //{
            if(element.tag === 'x7fe00010') {
                dicomParser.findEndOfEncapsulatedElement(byteStream, element, warnings);
                return element;
            }   else if(element.vr === 'UN') {
                dicomParser.findAndSetUNElementLength(byteStream, element);
                return element;
            } else if(element.length === 4294967295) {
                dicomParser.findItemDelimitationItemAndSetElementLength(byteStream, element);
                return element;
            }
        //}

Is this the best way to do this, and can we get this (or an equivalent change) accepted into the library?

@chafey
Copy link
Collaborator

chafey commented Feb 16, 2017

Hi Keean - This sounds like a violation of the DICOM standard. The strategy for this library is to be strictly DICOM compliant. That being said, we do want to expose hooks so applications like yours can implement such workarounds if desired. If you have a design in mind for these hooks, feel free to propose it.

@keean
Copy link
Author

keean commented Feb 16, 2017

It seems the only way we know it is an encapsulated image is because of the transfer encoding. What I need is a way to defragment the data in the pixel element.

If I could do:

switch(transfer_syntax) {
    case '1.2.840.10008.1.2.4.70':
        var pixel_data_element = data_set.elements.x7fe00010
        var pixels = dicomParser.getEncapsulatedData(pixel_data_element);
        // pixels is an ArrayBuffer of the defragmented data.
        ...
}

Because as far as I know if the data is compressed it must be encapsulated.

Is there a way to do this with the current library API?

@chafey
Copy link
Collaborator

chafey commented Feb 16, 2017

You can work around this today by replacing the dicomParser.readDicomElementExplicit function with your own implementation. While this works, we could reduce the code you have to maintain for this specific issue by moving the logic for determining if pixel data is encapsulated to a function dicomParser.isEncapsulated() that you could override with your own implementation.

@keean
Copy link
Author

keean commented Feb 16, 2017

I like the idea of "dicomParser.isEncapsulated()"

@chafey
Copy link
Collaborator

chafey commented Feb 16, 2017

OK, feel free to submit a PR!

@keean
Copy link
Author

keean commented Feb 17, 2017

Can I get access to DICOM properties from inside "readDicomElementExplicit"? For example I want to check the TransferSyntax, but the dataSet is not passed into "readDicomElementExplicit" as a parameter, is it available as on object property of the dicomParser object (this)?

@keean
Copy link
Author

keean commented Feb 17, 2017

There is a problem with getting the data needed for "isEncapsulated()". The normal method requires the element.length which is straightforward.

The patched version will need the byte-stream (to check if it starts xfffee000) and the transferSyntax, as if we know the image is a JPEG (which starts xd8ff), and the pixel-element actually starts xfffee000 we know it must be encapsulated. In this way we avoid false detection of encapsulation on an uncompressed image that starts with the pixels (xfe xff x00 xe0), which although unlikely is an issue with the current implementation that does not get passed the transferSyntax.

So the current implementation would require "dicomParser.isEncapsulated(element, byteStream)", but somehow we need to find a way for it to access the transferSyntax. As transferSyntax is read from the meta-header, the ideal option would seem to be to store it as a property "dicomParser.transferSyntax",
and then it can be read from "isEncapsulated" without having to thread it into the arguments of every function call down from the top-level of the parser.

If this is an acceptable solution I can send a pull request.

@chafey
Copy link
Collaborator

chafey commented Feb 17, 2017

We don't want to store any data in the dicomParser object itself (it is stateless). Your best option is to store the transfer syntax in the byteStream object. You can do this by passing the transferSyntax to the ByteStream() constructor when it is created in getDataSetByteStream().

Ideally the entire dataset would be available to isEncapsulated() so the logic can reference any attribute already parsed. The complexity and effort needed to do this though is hard to justify given the low chance anyone would every use it

@keean
Copy link
Author

keean commented Feb 19, 2017

I have submitted a pull request. Does it look okay?

@keean
Copy link
Author

keean commented Feb 27, 2017

Any news on the pull request? I am about to deploy the fixes, and I would like to know if these changes are going to get incorporated into the dicomParser library, as I don't want to have to maintain a fork of the library. If they are not going to get incorporated, It would be better to know sooner rather than later so that I can look for a different way to do it.

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

No branches or pull requests

3 participants