ImagingDataCommons / highdicom

High-level DICOM abstractions for the Python programming language
https://highdicom.readthedocs.io
MIT License
171 stars 37 forks source link

Issues with jpeg 2000 and `encode_frame` #275

Closed erikogabrielsson closed 6 months ago

erikogabrielsson commented 9 months ago

Looking at the encode_frame-method i can see some issues:

The parameters for saving Jpeg 2000 with Pillow is described here.

Not sure about the intended use of the encode_frame-method, but as it is used in highdicom to produce PixelData elements at least the jp2-issue should be fixed.

CPBridge commented 6 months ago

Hey @erikogabrielsson, I'm sorry that I somehow totally missed this until now. Thank you for letting us know about this.

For the first point, I have created #280. Could you please let me know whether this looks good to you?

For the second point, I am not really understanding. Currently the implementation only supports certain photometric interpretations and will raise an error if you try to specify one that is not supported for the combination of other parameters that you have chosen. Are you just suggesting that we may want to enable YBR_RCT and YBR_ICT for reversible and irreversible multi-component transform, respectively, or is there actually an issue with the current implementation of the supported photometric interpretations?

erikogabrielsson commented 6 months ago

Hi @CPBridge, No problem with the delay.

  1. It looks good'.
  2. From what I remember, my issue was "maybe also YBR_RCT and YBR_ICT should be supported" :)
CPBridge commented 6 months ago

The first point has now been resolved with #280, which will be inthe next release.

We do not have plans at this time to support the other photometric interpretations, since they are not widely used at the moment, so I am closing this issue. If people can point to a use case for those values, then it may be worth implementing them, but I do not know of any