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

[Fix] BOT generation for encapsulated data #399

Merged
merged 5 commits into from
Jul 28, 2023
Merged

Conversation

dougyau
Copy link
Contributor

@dougyau dougyau commented Jul 25, 2023

The BOT of the pixel data should point to the beginning of each frame. The original implementation was adding 1 extra index which was wrong as the last entry pointed to the end of the file.

@dougyau dougyau marked this pull request as draft July 25, 2023 00:52
@dougyau dougyau marked this pull request as ready for review July 25, 2023 02:00
@Enet4 Enet4 added bug This is a bug A-lib Area: library labels Jul 25, 2023
@Enet4 Enet4 self-requested a review July 25, 2023 07:47
@Enet4 Enet4 added C-core Crate: dicom-core C-pixeldata Crate: dicom-pixeldata labels Jul 25, 2023
Copy link
Owner

@Enet4 Enet4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks spot on! Thank you for noticing! I will just suggest a minor tweak to the vector construction calls and we can merge afterwards.

core/src/value/fragments.rs Outdated Show resolved Hide resolved
dougyau and others added 2 commits July 28, 2023 11:09
@dougyau
Copy link
Contributor Author

dougyau commented Jul 28, 2023

The issue was kind of tricky to spot, the generated files could be open with Weasis, Aliza, etc. DCMTK did not show any issues nor GDCM. It was cornerstone dicom-parser which failed due to this issue.

@Enet4
Copy link
Owner

Enet4 commented Jul 28, 2023

The issue was kind of tricky to spot, the generated files could be open with Weasis, Aliza, etc. DCMTK did not show any issues nor GDCM. It was cornerstone dicom-parser which failed due to this issue.

That is interesting. Indeed, any parser relying uniquely on the basic offset table to identify the number of frames of an image could choke on this.

In any case, thank you very much @dougyau. I will try to push a patch release soon.

@Enet4 Enet4 merged commit a9a20f4 into Enet4:master Jul 28, 2023
4 checks passed
@dougyau dougyau deleted the bot-fix branch July 28, 2023 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lib Area: library bug This is a bug C-core Crate: dicom-core C-pixeldata Crate: dicom-pixeldata
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants