Add support for JPEG-LS encoding/decoding #142
Conversation
|
We could avoid the pylibjpeg dependency by implementing JPEG-LS decoding in pydicom using Pillow (with the extension). |
There was a problem hiding this comment.
Looks good! I am not too concerned about the new dependency.
However, currently while this ability to encode JPEG-LS frames will be available, it will not be used by any of the implemented SOP classes. Seg/pm/sc all have a list of supported transfer syntaxes in their implementations (and docstrings). Should we enable support for JPEG-LS in these classes in this PR too?
|
| @@ -454,7 +489,7 @@ def __init__( | |||
| instance_number=instance_number, | |||
| manufacturer=ref_ds.Manufacturer, | |||
| modality=ref_ds.Modality, | |||
| transfer_syntax_uid=None, # FIXME: frame encoding | |||
| transfer_syntax_uid=transfer_syntax_uid, | |||
There was a problem hiding this comment.
Just a though, should the default behaviour be to copy the transfer syntax from the input images, and not re-encode the pixels?
There was a problem hiding this comment.
I generally agree.
However, this may be a good opportunity to avoid some of the exotic lossless JEPG transfer syntaxes that are not widely supported and thereby facilitate downstream decoding. Thoughts?
There was a problem hiding this comment.
Are you suggesting that if no transfer syntax is explicitly specified, we selectively re-encode some lossless codecs into a more commonly-supported format?
There was a problem hiding this comment.
Yes, or just not compress at all by default (which has been the behavior so far)
There was a problem hiding this comment.
If any of the source images was lossy compressed, we also shouldn't re-compress the enhanced image with a lossy transfer syntax
There was a problem hiding this comment.
Agree completely - if the images have been lossy compressed a different compression algorithm can create artifacts; even the same lossy compression technique with different parameters can create a mess. If the original compressed bytes aren't passed through then a lossless algorithm is the only safe mechanism.
I think there is a DICOM field which designates whether the image has been lossy compressed or not. If the images are transmitted decompressed this field should be set to true so that downstream the consumer knows that the images are not the original image data.
I don't know if the FDA wants the lossy compression ratio put into the image or not - there was a big discussion of this 20 years ago but I haven't been following it since then.
There was a problem hiding this comment.
Right, the main thing to avoid would be to decompress lossy data and then recompress losslessly (or store with no compression, as is currently the default) thus giving the impression of lossless compression when in fact information has been lost. That's why I'm suggesting simply leaving the transfer syntax and compression alone by default in those situations unless the user specifically requests otherwise. However I don't see a harm in silently translating between different lossless formats, although it is a bit magical
There was a problem hiding this comment.
The user can currently not set a lossy transfer syntax. If the pixel data was lossy compressed in the first place, the attribute LossyImageCompression should already reflect that (and should stay unchanged even if pixel data is subsequently encoded using a lossless transfer syntax).
The behaviour so far has been to decode the pixel data and re-encode it uncompressed. We can (and probably should) change that behavior upon refactoring of the legacy conversion package as part of #34
This PR will make it possible to encode and decode images using the JPEG-LS codec (which is much much fast than JPEG2000 for both encoding and decoding).
It would introduce additional dependencies: