phoboslab / qoi

The “Quite OK Image Format” for fast, lossless image compression
MIT License
6.94k stars 330 forks source link

Possible edge case in decoder when very first OP is QOI_OP_RUN #258

Open phoboslab opened 1 year ago

phoboslab commented 1 year ago

As recently pointed out by @andrews05, there is an edge in QOI files that could be handled differently by different decoders.

This edge case can produce a transparent background when fully opaque black was intended. This may happen, when decoding to RGBA (4 channels) and the very first OP of a QOI file is a QOI_OP_RUN, that repeats the initial opaque black pixel.

Some decoders omit putting the initial black pixel into the index in this case. Later in the image the decoder may refer to this to the index position where the opaque black should have been stored, but that instead is empty – i.e. just the initial transparent black that comes from the zero initialized index.

As the spec points out, admittedly not very clearly:

Each pixel that is seen by the encoder and decoder is put into this array at the position formed by a hash function of the color value.

... this must include the initial opaque black when the very first OP is a QOI_OP_RUN.

I have updated the qoi_test_images.zip to include an image where this edge-case is present, aptly named edgecase.qoi.

As further pointed out by @andrews05, the reference encoder in this repository does not produce QOI images that have this edge-case (it never starts a QOI file with a QOI_OP_RUN), but other encoders might.

After going through the decoders listed in this readme, I have identified a few implementations where this edge case is not handled according to the spec – again, mea culpa, it's not the clearest wording. I will notify those implementations now.

Thanks again to @andrews05 for pointing it out.

pfusik commented 8 months ago

If the encoded data starts with QOI_OP_INDEX | 0x35, is it a black or transparent pixel?

phoboslab commented 8 months ago

Not sure I understand the question correctly. Per the spec, the 64 elements of the index are zero-initialized. I.e. all colors in the index are initially transparent black.

If the very first decode is QOI_OP_INDEX | 0x35 and you're emitting RGBA (as opposed to RGB), the pixel will be transparent.

pfusik commented 8 months ago

QOI_OP_RUN normally writes pixels same as the pixel before, therefore there is no need to update the index. The only exception is if QOI_OP_RUN is the first code, where it writes black pixels that are not in the index. I considered initializing the index so that the black pixel is there to start with. Your answer clarifies that it would break QOI_OP_INDEX as the first code, which unfortunately means that QOI_OP_RUN needs to update index just for this edge case.

pfusik commented 8 months ago

Another solution is to look-ahead QOI_OP_RUN and handle the index outside the main loop.

phoboslab commented 8 months ago

Yes, this is somewhat unfortunate and maybe could have been solved with some more thought about the initial values for previous pixel and the index...

For what it's worth, this implementation here only sets the index at the start of a run; it doesn't calculate the hash when emitting pixels within a run. I assume the performance impact is negligible.

alexriegler12 commented 7 months ago

Would it work if you always inserted the initial opaque black into the array and not just if the first chunk is a run?

pfusik commented 7 months ago

That would break QOI_OP_INDEX which, as discussed above, must produce a transparent pixel.

wx257osn2 commented 7 months ago

@phoboslab

I have updated the qoi_test_images.zip to include an image where this edge-case is present, aptly named edgecase.qoi.

There is edgecase.qoi and edgecase.png in qoi_test_images.zip , but edgecase.png is 3-channel png even though edgecase.qoi has an alpha channel. So, we can detect the issue by roundtrip verifying with edgecase.qoi, but we can't with edgecase.png for some implementations that doesn't handle alpha channel on 3-channel decoder. Would you replace the edgecase.png with 4-channel one?