Cannot load DICOM instance where SQ is using VR:UN with undefined length

[malaterre]

For some reason I can load a CT Image when all sequences are using defined length (VR:UN) but I cannot load the same instance when the sequences are using undefined length (VR:UN).

• CT-SQ-UN-IVRLE-undeflen.dcm: not OK
• CT-SQ-UN-IVRLE-deflen.dcm: OK

CT-SQ-UN-IVRLE.zip

Reference:

• CP 246 - Parts 5 - Undefined length UN VR value is always implicit VR - Standard - 2001. ftp://medical.nema.org/medical/dicom/final/cp246_ft.pdf

[malaterre]

Someone may want to move this issue in the other project "dicomParser"

[dannyrb]

@malaterre, transferred. Thanks for the report. This feels similar to a report we had for "private fields" where we did not know the length, but I could be mistaken.

[yagni]

@malaterre Are you referencing the unpkg build by any chance? I think it might be corrupted or out of date. If I use the dump with data dictionary example, dropping your undeflen file on it works just fine unless I switch to using the unpkg build, at which point I get a buffer overrun.

[yagni]

@malaterre Can you confirm whether this is still an issue? I can successfully drop both of the above files into the dump with data dictionary example. Is there some other way the undefined length file is failing to load?

[malaterre]

@yagni Can you post the dump somewhere please ?

[yagni]

@malaterre After trying again, I noticed both don't throw any errors during dumpWithDataDictionary, but the file with defined sequence lengths doesn't parse the sequences. Instead, they're treated as binary, as shown below:

The corresponding sequences in the file with undefined lengths:

Aside from the sequence differences, the rest of the files dump the same. Is this what you were referring to? If so, I think that's correct behavior: note 5 in section 6.2.2 of the standard says you can parse a UN element as if it were a SQ, but only if it has undefined length.

Here's the undeflen dump and here's the deflen dump

[malaterre]

@yagni In this case you should specify (somewhere in the documentation) that dicomParser will never convert CP-246 encoded SQ into SQ (so one can never access the contained Data Element). I find the behavior quite surprising since this would prevent the library from reading any needed nested dataelement from a know SQ attribute.

[yagni]

@malaterre I'd like to make a PR for this, so please bear with me while I understand what the desired behavior is here. The difficulty here is dicomParser doesn't know an attribute is SQ if the VR isn't explicit (or if it's explicit and marked UN) since we don't have a data dictionary unless the user provides a VR callback. If a UN element has undefined length, we know it's SQ because SQ is the only element (other than an item, and that has a specific tag) that can have an undefined length. However, if it has a defined length, it can either be a sequence or some other kind of element.

Given that, would this satisfy a standards-compliant behavior for a UN element:

1. If undefined length or VRCallback returns SQ, parse as implicit VR little endian sequence
2. Otherwise, treat as a single element.

If I'm missing something or there's a better way, please let me know.

P.S. Thanks for all your hard work on GDCM--what a contribution to the field!

[malaterre]

@yagni Thanks for your kind words.

Reading Implicit Transfer Syntax without a data dictionary is identical to the behavior you describe above. So I suspect the right fix is implement "UN + defined Length + Explicit Transfer Syntax", just as any "SQ element in Implicit Transfer Syntax". Sorry I do not know the codebase. You need to convert "on-the-fly" your binary data to SQ when given a dictionary (just as in the case of implicit).

Let me know if the above make sense.

Re-reading this thread (see @dannyrb message) I also vaguely recall a hack about private attributes. So I suspect the implementation is already ready for UN + defined length + private attribute.