Encode: lossless encoding broken

[boxerab]

For various combinations of image size and number of resolutions, with default settings i.e. mathematically lossless, image resulting from round trip encode/decode does not equal original. See issue #891 for an example of a broken lossless encode.

Until this is fixed, the library should not be used for lossless encoding, as it is unpredictable whether result is in fact lossless.

[boxerab]

Also, even if encoding is lossy, this bug will reduce PSNR on resulting image

[kieranjol]

Hi, how long has this issue been around? Has it been around for a while and just recently spotted?

[boxerab]

@kieranjol this issue would probably lie in the guts of the codec, so would guess that it has been around pretty much forever. Nobody has really touched this part of the code for years.

[boxerab]

We also still have broken lossless encode with modes RESTART and BYPASS, more details in issue #674

[boxerab]

And we also have broken tiled lossless encode, as described in issue #737

[detonin]

@boxerab Title of this issue is misleading. We have indeed a bug spotted in #891 occuring for certain combination of image size. This should be addressed as soon as possible and if you have any input on this, please do so in #891. But AFAIU it is not related to the specific lossless capability of the codec. Saying that the library should not be used for lossless encoding until this is fixed appears completely overkill to me.
Then we have also #674 and #737 that are still open indeed, thanks for reminding us. If I may ask, apart from this kind reminder, what's the purpose of the present issue ?

[retokromer]

@detonin May I disagree? From an archival point of view, this is indeed a major issue, therefore the recommendation to wait until it is fixed is the accurate one, in my opinion.

[boxerab]

I am alarmed by any issue that turns lossless into lossy; I opened this issue to get other people alarmed. Seems to be working :) In addition to impact on archive, any loss of data in medical imaging has serious legal and patient safety ramifications.

[detonin]

@retokromer I'm not saying this is not an important issue. My point is:

• this is not strictly related to lossless encoding: the bug occurs when a remaining tile on the last row or column has a dimension smaller than 2^(number of wavelet decomposition-1). So even if not satisfying at all, we know at least when the bug triggers.
• the present issue does not bring anything new compared to opj_compress v2.1.2 can create images opj_decompress cannot read #891.
  Anyway, any input to solve opj_compress v2.1.2 can create images opj_decompress cannot read #891 is welcome ( @mayeut any idea ?)

[fechen123]

One solution to this issue is to have less number of decomposition for the tile that has a dimension smaller than 2^(number of wavelet decomposition) during the encoding. Write a COD marker in the header of this kind of tiles to indicate the number of decomposition for the tile. The decoder then can get the correct number of decomposition for this tile from the COD marker and decode accordingly.

[boxerab]

Interesting idea. But, I think this would only mask the bug - better to understand why this is happening and fix the root cause. The bug might have other symptoms that we are not aware of.

[fechen123]

As stated in the previous comments, the root cause of this issue is that the tile dimension at the last row or column can be smaller than 2^(number of wavelet decomposition-1), which is kind of common. Thus these tiles cannot have the same number of resolutions as other tiles (dimension >= 2^(number of wavelet decomposition-1)). The JPEG2000 spec allows COD marker in the header of the tile to indicate the different number of resolutions than the others. That is the way to encode this kind of tiles.

For the current release, the workaround to this issue is to carefully select the tilesize parameter and number_of_resolutions parameter so that the dimensions of the tiles at the last row or column is not smaller than 2^(number of wavelet decomposition-1). Note: number_of_resolutions = number_of_wavelet_decompostion +1.

[boxerab]

We don't know the root cause. It is perfectly legal according to the standard to have as many as 32 decomposition levels on a tile. So, better to fix the bug than mask it with a COD marker.

[fechen123]

If the tile dimension size is less then 2^32, it cannot have 32 decomposition levels because each wavelet decomposition reduces the size (x and y) by half.

[boxerab]

It is legal to have 32 decompositions, for precisely the situation we have here, where tiles on the boundary can be as little as one pixel deep or wide, so there are in a sense too many decompositions for these tiles, and yet according to the standard, we don't need to define a special COD marker for those tiles. Some of these subbands will be empty, but that is perfectly acceptable.

[fechen123]

If we want to automatically adjust the number of decompositions in the encoder/decoder, we could have the following fix in tcd.c, replace line 2210 in the master_2.1.2 release version:
l_tilec->numresolutions = l_tccp->numresolutions;
with
OPJ_UINT32 nrx, nry;

    nrx = (OPJ_UNIT32)opj_int_floorlog2(l_tilec->x1 - l_tilec->x0)+1;
    nry = (OPJ_UNIT32)opj_int_floorlog2(l_tilec->y1 - l_tilec->y0)+1;
    if (nrx < l_tccp->numresolutions || nry < l_tccp->numresolutions)
       l_tccp->numresolutions = opj_uint_min(nrx, nry);
    l_tilec->numresolutions = l_tccp->numresolutions;

I don't have the environment to upload the fix through github. It worked for me locally on the master_2.1.2 release version. Anyone can help try the fix and see if it works on the main branch?

[detonin]

@fechen123 Thanks for the proposal. However I indeed tend to agree with @boxerab, number of decompositions can be as high as 32, even for images / tiles smaller than 32x32. Using your fix would mask the bug but not solve it.

[fechen123]

That is all right. I guess I have different understanding of the standard. I would be interested in seeing the real fix.

[rouault]

Fixed per #949