Skip to content

fix(dicom): build valid DICOM when reading tags#737

Merged
floryst merged 1 commit intomainfrom
fix-opt-dicom-loading
May 12, 2025
Merged

fix(dicom): build valid DICOM when reading tags#737
floryst merged 1 commit intomainfrom
fix-opt-dicom-loading

Conversation

@floryst
Copy link
Contributor

@floryst floryst commented May 9, 2025

  • dicom parser now exposes element hooks
  • extract implicit or explicit VR
  • build an empty PixelData tag for metadata parsing

@floryst floryst requested a review from PaulHax May 9, 2025 18:36
@netlify
Copy link

netlify bot commented May 9, 2025

Deploy Preview for volview-dev ready!

Name Link
🔨 Latest commit d8ffbf8
🔍 Latest deploy log https://app.netlify.com/sites/volview-dev/deploys/681e4b2ddf2f7d00082040a1
😎 Deploy Preview https://deploy-preview-737--volview-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@PaulHax PaulHax left a comment

Choose a reason for hiding this comment

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

All fixed now =)

Comment on lines +88 to +92
// itk.wasm/GDCM requires valid pixel data to be present, so we need to
// generate fake pixel data. Valid means valid length and VR.
// It turns out that encapsulated pixel data structures are parsed, even if
// the pixel data itself is not touched. This does not work well with
// metadata streaming.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice, how did you put your finger on this? Debug into WASM?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A mix of debugging WASM and looking at GDCM source. In particular, I found that GDCM requires the PixelData tag (https://github.com/InsightSoftwareConsortium/ITK/blob/b3f104ddae4061de221dd5dfb23d51a98d4b2092/Modules/ThirdParty/GDCM/src/gdcm/Source/MediaStorageAndFileFormat/gdcmSerieHelper.cxx#L137C3-L140C23) and that ImageReader was crashing on parsing an incomplete Encapsulated Pixel Data value.

@PaulHax PaulHax linked an issue May 12, 2025 that may be closed by this pull request
@floryst floryst added this pull request to the merge queue May 12, 2025
Merged via the queue into main with commit 67b6296 May 12, 2025
8 of 9 checks passed
@zachmullen zachmullen deleted the fix-opt-dicom-loading branch November 11, 2025 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Modality OPT/OCT DICOM file does not load when streamed

2 participants