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

Error parsing image - nested sequences? #180

Closed
bisol opened this issue Oct 13, 2015 · 6 comments
Closed

Error parsing image - nested sequences? #180

bisol opened this issue Oct 13, 2015 · 6 comments
Labels
bug Something isn't working
Milestone

Comments

@bisol
Copy link

bisol commented Oct 13, 2015

I'm using DWV 0.11.1, and got this error while trying to display an image:

Error: Missing or empty DICOM image number of columns

I've traced it to these tests on dwv.dicom.DicomParser.prototype.parse

// end of sequence with explicit length
if ( dataElement.vr !== "SQ" && sequences.length !== 0 ) {

Seems like it can't handle a sequence with another sequence as it's last item.

This site limits attachments by file extension, so I uploaded the DICOM file renamed to png :p

6809 anon

@bisol
Copy link
Author

bisol commented Oct 13, 2015

I have tried and failed to commit a new branch. Anyhow, this worked for me:

        // handle nested sequences of explicit length
        if (dataElement.vr == "SQ" && sequences.length > 1 ) {
            sequences[sequences.length - 2].vlCount -= 4;  // offsets the +12 below 
        }

        // end of sequence with explicit length
        if ( dataElement.vr !== "SQ" && sequences.length !== 0 ) {
            var last = sequences.length - 1;
            sequences[last].vlCount += tagOffset;
            // check if we have reached the sequence vl
            while ( sequences.length > 0 && sequences[last].vlCount === sequences[last].vl ) {
                // last count + size of a sequence
                var lastVlCount = sequences[last].vlCount + 12;
                // remove last sequence
                sequences = sequences.slice(0, -1);
                // add nested sequence vl
                if ( sequences.length !== 0 ) {
                    last = sequences.length - 1;
                    sequences[last].vlCount += lastVlCount;
                }
            }
        }

@pervez8ktt
Copy link

there are so many problems with current dicomParser.js

you need to update dicomParser.js with new one that is

https://github.com/chafey/dicomParser

@ivmartel
Copy link
Owner

Thanks for your code @bisol!

@ivmartel ivmartel added the bug Something isn't working label Oct 14, 2015
@ivmartel ivmartel added this to the 0.12.0 milestone Oct 14, 2015
@ivmartel
Copy link
Owner

Your comment is not really helping the issue @pervez8ktt... Feel free to contribute to the dcmbench project to prove your point!

@bisol
Copy link
Author

bisol commented Oct 15, 2015

@ivmartel
I should mention that I don't fully understand the magic numbers (+12 and -4). I tested this with a handful of DICOM images, but that's far from thorough. Maybe it will break with a triple nested sequence...

@ivmartel
Copy link
Owner

Ok, got it, the problem is implicit/explicit transfer syntax. The 4 bytes are used to store the VR in the explicit case (see DICOM sect_7.5.html#table_7.5-1) which was what I tested against. Yours is implicit so no need for those 4 bytes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants