imi-bigpicture / wsidicomizer

Python library for converting WSI files to DICOM
Apache License 2.0
54 stars 7 forks source link

Transfer Syntax and lossless JPEG2000 #69

Closed Daniel300000 closed 1 year ago

Daniel300000 commented 1 year ago

Hi!

According to the comments, one should use "encoding_quality = 100" for lossless JPEG2000:

encoding_quality: int = 90
    Quality to use if re-encoding. Do not use > 95 for jpeg. Use 100
    for lossless jpeg2000.

However, in the class "Jpeg2000Encoder", the DICOM transfer syntax "JPEG2000Lossless" is used for values < 1 only:

class Jpeg2000Encoder(Encoder):
    def __init__(self, quality: float = 20.0) -> None:
        self._quality = quality
        if self.quality < 1:
            self._transfer_syntax = JPEG2000Lossless
        else:
            self._transfer_syntax = JPEG2000

When setting "encoding_quality = 100" and then debugging, encoder.transfer_syntax is "1.2.840.10008.1.2.4.91". For lossless JPEG2000, I would expect "1.2.840.10008.1.2.4.90".

Also, in _jpeg2k.pyx from the "imagecodecs" package, quality < 1 seems to be expected for lossless JPEG2000:

if quality < 1 or quality > 1000:
    # lossless
    quality = 0.0
    if reversible is None:
        irreversible = 0
[...]
if quality == 0.0:
    # lossless
    parameters.irreversible = 0
    parameters.cp_disto_alloc = 1
    parameters.tcp_rates[0] = 0

However, debugging shows that jpeg2k_encode() is also called with level = 100:

return jpeg2k_encode(
    data,
    level=self._quality,
    codecformat="J2K",
)

I guess the comments should be: "Use 0 for lossless jpeg2000"? Or am I missing something important?

erikogabrielsson commented 1 year ago

Hi @Daniel300000,

You are correct in that the doc-string for the encoding quality parameter is wrong. The correct should be, as you noted, that < 1 (or > 1000) results in lossless JPEG2000. Will fix this in #70.

erikogabrielsson commented 1 year ago

@Daniel300000 can you test 0.9.2 and report if the transfer syntax is as expected (given the new argument doc-string that quality should be < 1 for lossless)?

Daniel300000 commented 1 year ago

Yes, I can confirm that the TransferSyntaxUID in the final .dcm file is "1.2.840.10008.1.2.4.90" when using "encoding_quality = 0.0".

I also found that there are two wrong doc-string comments left in "wsidicomizer.py" and "cli.py".

erikogabrielsson commented 1 year ago

Thanks @Daniel300000 for finding the missed doc-strings. I have updated those in #73.