-
Notifications
You must be signed in to change notification settings - Fork 181
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
DICOM: fix multiframe encoding with explicit length sequences #2691
DICOM: fix multiframe encoding with explicit length sequences #2691
Conversation
OK, tests all check out - though with these changes I am now able to handle more data than previously, which is nice... 😁
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks; would likely have taken me a helluva lot longer to figure this one out.
Confirmed correct operation on the provided problematic dataset; beyond that I can only place my faith in the private DICOM test suite that it doesn't break anything else.
Is there any merit in incorporating the data from @timrosenow into that private dataset? Or, as might be possible given your statement, were there existing data in there that are now working given this fix, and so they would not provide any additional utility?
That was incredibly fast, thank you guys |
Already is... 😜 |
As discussed in #2690
Problem stems from failure to handle cases where nested sequences all end at the same point. Previous implementation only closed the current sequence, without considering whether its own parents also needed to close at that point.
I'll be running my battery of tests shortly to confirm this doesn't introduce any issues with current datasets.