phoboslab / qoi

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

variable "a" implicitly set to -1 in index_position for RGB channel #200

Closed ghost closed 2 years ago

ghost commented 2 years ago

Hello,

Not sure if it is an intentional behavior but the "a" variable (for alpha) in the hash function index_position is set to -1 to encode images with RGB channels.

I reach this point of confusion because I choose to write a encoder with the specification v1.0 and it says "The colorspace and channel fields are purely informative. They do not change the way data chunks are encoded." and there is not something like "a is set to -1 in case of RGB channels in hash function." I supposed it were egals to 0 and I was suprised when I compare the result of my tool with the files in https://qoiformat.org/.

In any way, I suggest to write it in specification to keep homogeneous qoi files with identical QOI_OP_INDEX in qoi files which are encoded with different tools and for example, have a comparaison of binary files efficient, avoiding useless differences like \xd and \x2. In all case, it is not interfere with the whole process and tools could be quikly updated.


"a" is just ignore in "px" definition in those lines ; so -1 is implicitly defined in RGB cases. https://github.com/phoboslab/qoi/blob/028c75fd26e5e0758c7c711216c00404994c1ad3/qoi.h#L342 https://github.com/phoboslab/qoi/blob/028c75fd26e5e0758c7c711216c00404994c1ad3/qoi.h#L450 https://github.com/phoboslab/qoi/blob/028c75fd26e5e0758c7c711216c00404994c1ad3/qoi.h#L597

Have a nice day

ratchetfreak commented 2 years ago

actually 255 for alpha is assumed for alpha when reading 3 component image.

https://github.com/phoboslab/qoi/blob/028c75fd26e5e0758c7c711216c00404994c1ad3/qoi.h#L419-L420

and in the loop:

https://github.com/phoboslab/qoi/blob/028c75fd26e5e0758c7c711216c00404994c1ad3/qoi.h#L427-L433

This is typical as 255 is fully visible and 0 is fully transparent so on decode if the application requests a 4 component RGBA image it should get 255 as alpha for all pixels.

using 255 comes out to the same result as taking -1 in the hash calculation.

ghost commented 2 years ago

Thank you for explanation. It should be in specification if it is so important for some apps.

phoboslab commented 2 years ago

I understand where the confusion comes from and I probably should mention this in the specification more explicitly. I just want to explain the reasoning here.

The spec says that the channels in the header is irrelevant for en-/decoding of the data. That means you can specify 3 channels in the header and still encode QOI_RGBA chunks if you want to. A decoder might take the channels header as a hint of how the data should be returned to the user, when the user doesn't explicitly state the number of channels they want.

E.g. this reference decoder only takes the channels header into account when the user says "give me the pixel data with the number of channels that are specified in the header" - i.e. calling decode() with channels = 0.

If channels is 0, the number of channels from the file header is used. If channels is 3 or 4 the output format will be forced into this number of channels.

So with the channel header being only "informative", the spec then goes on and says:

The decoder and encoder start with {r: 0, g: 0, b: 0, a: 255} as the previous pixel value.

It does NOT say:

(...) starts with {r: 0, g: 0, b: 0} for RGB or {r: 0, g: 0, b: 0, a: 255} for RGBA

No, it's always a: 255. Omitting the initial value for alpha when channels=3 wouldn't make much sense, as we have established that the channels header does not change the behaviour of the en-/decoder.

So if you then only encode RGB data (with a:255 as the initial value) you never need to emit a QOI_RGBA tag and thus never change the alpha from its initial value of 255. The end.

ghost commented 2 years ago

If I understand :

A header gives informations of a file and in particular how to understand its informations to decode it.

Because here all pixel's datas are chunks and chunks are distinguished dependings on their tags, the channels variable is never involded to understand a chunk and decode it. For example, tags for QOI_OP_RGB and QOI_OP_RGBA make a difference of channels but a channel variable is here useless to decode.

It also could give information of how a file has been encoded but it's the same as decode here, a channel variable is useless.

In fact, it's only important for input data in a encoder and output data in a decoder, which is out of the field of a qoi file even if, logically, a encoder/decoder of qoi file must have a channels variable (from header or not) because it's a necessary variable for other type of image file and/or binary file.

That's how I understand now "The colorspace and channel fields are purely informative. They do not change the way data chunks are encoded."

But

The decoder and encoder start with {r: 0, g: 0, b: 0, a: 255} as the previous pixel value.

is not the same as

The decoder and encoder start with {r: 0, g: 0, b: 0, a: 255} as the previous pixel value and a = 255 in case of a RGB pixel (in input encoder for compatibility with RGBA, homogeneous reasons in qoi file resuting between encoders and avoid a first QOI_OP_RGBA when a first QOI_OP_RGB is enough).

Thank you for your time.

ps : I think it lacks a s at channel in "The colorspace and channelS fields are purely informative."