#194 encapsulation is applied for only PixelData

dcmjs-org / dcmjs

Javascript implementation of DICOM manipulation

https://dcmjs.netlify.com/

MIT License

293 stars 112 forks source link

#194 encapsulation is applied for only PixelData #199

Closed codepage949 closed 3 years ago

codepage949 commented 3 years ago

after this fix, overlay is visible on compressed image transfer syntax.

it seems enough that encapsulation is applied for only PixelData.

is there another case to apply encapsulation?

codepage949 commented 3 years ago

from #194

pieper commented 3 years ago

Hi - thanks for digging into this and that solution looks simple. But I haven't got a clear idea whether it would have side effects on other studies.

Can you write a test following the format of the other tests in dcmjs? Were you able to run the existing test suite with your change?

codepage949 commented 3 years ago

@pieper added test and passed with other tests, please review.

pieper commented 3 years ago

Thanks for adding the test 👍

Since this is in a deep part of the code that I know people are using in production I'm trying to be extra thoughtful about merges. @Punzo and @igoroctaviano could you have a quick look to see if you think this is okay to merge?

pieper commented 3 years ago

@codepage949 could you provide some extra context for this?

what is the target use case?
where do these overlays come from?
what system is expected to read them?
how do other toolkits handle this data? Have you tried pydicom, dcmtk, gdcm, others?

Thanks in advance

codepage949 commented 3 years ago

what is the target use case?
- denoting areas over image
where do these overlays come from?
- in my case, made it by myself. please refer to #194
what system is expected to read them?
- ideally, any dicom viewer that complies with standards
how do other toolkits handle this data? Have you tried pydicom, dcmtk, gdcm, others?
- in pydicom, overlay data is inserted without any transformation even when transfer syntax is what needs encapsulation.

[side note]

i opened a dicom file which has private tag of UN vr with dcmjs and saved without any changes.

and then, opened dicom file saved by dcmjs on weasis, aliza viewer, it failed to open.

i guess it is because dcmjs apply encapsulation on private tag of UN vr too. tag.js#L111

codepage949 commented 3 years ago

according to this, i interpreted that pixel data is the only one that can be encapsulated.

Pixel Data (7FE0,0010) may be encapsulated or native.

Overlay Data (60xx,3000) and other tags mentioned in the document. ... shall be encoded in Little Endian.

pieper commented 3 years ago

according to this, i interpreted that pixel data is the only one that can be encapsulated.

Pixel Data (7FE0,0010) may be encapsulated or native.

Overlay Data (60xx,3000) and other tags mentioned in the document. ... shall be encoded in Little Endian.

Yes, I see your logic now @codepage949.

@jmhmd I think this is also related to some code you added. Can you review and comment?

ohif-bot commented 3 years ago

:tada: This PR is included in version 0.18.8 :tada:

The release is available on:

Your semantic-release bot :package::rocket: