dicomParser icon indicating copy to clipboard operation
dicomParser copied to clipboard

Sequence Delimitation Item not present in the element list

Open Gaelik-git opened this issue 4 years ago • 4 comments

When using the method parseDicom on DICOM containing a SQ element with undefined length I have the feeling it's not correct to not return the SequenceDelimitationItem as an item of said element

In this example : x00080096 (ReferringPhysicianIdentificationSequence) is a Sequence of undefined length It is followed by a SequenceDelimitationItem, but it's nowhere to be found. I think it should be part of the items list of the current tag like the ItemDelimitationItem ("xfffee00d") is for Item ("xfffee000")

image

Is the current behaviour the expected behaviour or do you confirm this is bug ?

Thank you for your work

Gaelik-git avatar Apr 21 '20 10:04 Gaelik-git

I used your example : https://rawgit.com/cornerstonejs/dicomParser/master/examples/dumpWithDataDictionary/index.html with google chrome and a break point juste after the dataSet = dicomParser.parseDicom(byteArray, options); in the index.html to produce the screen shot

Gaelik-git avatar Apr 21 '20 10:04 Gaelik-git

It does look like it used to be that way a long time ago (here are the changes that introduced the change). @chafey any thoughts on this one?

yagni avatar Oct 01 '21 14:10 yagni

This sounds like a bug - the intent of the design is to expose all of the detail of the underlying bitstream and SequenceDelimitationItem should be present as one of the items in a sequence of undefined length. I seem to recall some other bugs related to undefined length sequence parsing - it would love to see someone review this in depth, add tests and refactor if necessary

On Fri, Oct 1, 2021 at 9:23 AM Bryan Cool @.***> wrote:

It does look like it used to be that way a long time ago (here https://github.com/cornerstonejs/dicomParser/commit/f63b40858c110f671ae0867a10ac2ce7624f1b82 are the changes that introduced the change). @chafey https://github.com/chafey any thoughts on this one?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cornerstonejs/dicomParser/issues/143#issuecomment-932275373, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJVXWVZJIN3WDTDENVG74DUEW75PANCNFSM4MNFTS5A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

chafey avatar Oct 01 '21 15:10 chafey

Note that this will be a breaking change, as it will change the reported length of the SQ element.

yagni avatar Sep 27 '22 19:09 yagni