randy408 / libspng

Simple, modern libpng alternative
https://libspng.org
BSD 2-Clause "Simplified" License
742 stars 75 forks source link

SPNG_FMT_PNG is big-endian even while little-endian system #230

Open GhostCore07 opened 2 years ago

GhostCore07 commented 2 years ago

The documentation states:

With SPNG_FMT_PNG the pixel format will [be] identical to the PNG’s format in host-endian

And then in spng.h for spng_format it says:

/ Channels are always in byte-order /

But then just under that

SPNG_FMT_PNG = 256, // host-endian SPNG_FMT_RAW = 512 // big-endian

I'm not sure what host-endian is supposed to mean here. Intuitively I assume it's the same thing as native endian. But on a little-endian system the decoder output is still in big-endian. In other words, for a basic 32-bit RGBA PNG on a little-endian system, SPNG_FMT_RGBA8, SPNG_FMT_PNG and SPNG_FMT_RAW are all identical. It's hard to interpret if this is intended since "always in byte-order" hints at always big-endian, but then there is a distinction made between "host-endian" and "big-endian" right after that. Is it true that there is only big-endian output?

Edit: I just realized that perhaps the distinction that I am not seeing is that the endianess might only apply to 16-bit color depth?

randy408 commented 2 years ago

RGBA Color model

  • In the byte-order scheme, "RGBA" is understood to mean a byte R, followed by a byte G, followed by a byte B, and followed by a byte A. This scheme is commonly used for describing file formats or network protocols, which are both byte-oriented.
  • In the word-order scheme, "RGBA" is understood to represent a complete 32-bit word, where R is more significant than G, which is more significant than B, which is more significant than A.

Which means endianness does not affect the order of channels in a pixel, just the byte order of 16-bit values.

Because PNG only stores 16-bit values as big-endian SPNG_FMT_RAW is always big-endian, makes sense? The rest of the spng_format's are host-endian.

GhostCore07 commented 2 years ago

Thank you for the clarification regarding 16 bit color depth.

The motivation for this discussion then is that Windows bitmaps are "ARGB" or "RGB" in a "word-order scheme". I believe the historically accurate terminology used here is little-endian, but I won't argue it. Since ARGB is only planned we can just consider the RGB case. A block of memory cannot just be provided to the encoder with a pointer, but instead the memory needs to be byte-order swapped first. Back to the link you provided:

In a big-endian system, the two schemes are equivalent. This is not the case for a little-endian system, where the two mnemonics are reverses of each other.

And if such an annoying copying operation such as byte-order swapping is required anyways then "ARGB" support, which is planned, wouldn't be much help because you already have to swap bytes around anyways to get the byte-order expected by the encoder. So in one sense it ties back to ARGB support and in the other sense it's a request to support "word-order scheme".

randy408 commented 2 years ago

ARGB is planned for this exact use case, so it would be a word-order format (probably named SPNG_FMT_ARGB32). That would solve the issue, no?

GhostCore07 commented 2 years ago

yeah sounds good, i would just note that some API call that format BGRA since that's what it is in byte-order.

dinkc64 commented 1 year ago

Hi, Just got introduced to spng yesterday, and, my - what a nice project! I had completely replaced libpng in a project in a little over an hour. libpng has a nice crashy feature on broken/corrupt png which wasn't tolerable for users of the project I'm working on.

Back on topic: This same issue really confused me for a little while, but, I eventually ended up fixing it with a little for loop. Though, BGR (with and without the A) would be a welcomed feature. Here's the for loop for anyone else that might have the same issue:

        for (int i = 0; i < temp_img.width * 3; i += 3) {
            int r = temp_img.rowptr[row_info.row_num][i + 0];
            int g = temp_img.rowptr[row_info.row_num][i + 1];
            int b = temp_img.rowptr[row_info.row_num][i + 2];

            temp_img.rowptr[row_info.row_num][i + 0] = b;
            temp_img.rowptr[row_info.row_num][i + 1] = g;
            temp_img.rowptr[row_info.row_num][i + 2] = r;
        }

best regards,