Header endianness

[sbrki]

Hi!
First of all, very nice work!

I've noticed a potential issue with qoi_header_t. Entire qoi file format is generally Big-endian, but width,height and size integers in the header are represented in the output file either as Little or Big endian, depending on the machine architecture. This unfortunately makes the qoi files non-portable across machines with different byte-orders.

If this was not intentional, could you consider placing htonl/ntohl and htons/ntohs calls before/after packing/unpacking the mentioned integers in qoi_header_t? I can submit a PR if you don't have the time.

[phoboslab]

I guess the values in the header should also be big endian (breaking all three of the .qoi images out there).

I'm leaning towards to just reading/writing those values byte-wise, similar to how stbi does it and as explained by Rob Pike and elaborated by Justine Tunney.

We'll sadly lose the nice, readable way of just casting the header from the raw data (qoi_header_t *header = (qoi_header_t *)data;), but so be it.

Any insights on this?

[sbrki]

Sorry, I forgot to think about portability - htonl and friends are not the way to go. Yes, the way stbi does it (and Rob's approach in general) is the way to do it here.

As you mentioned, if stbi's approach is used, it unfortunately looks like the header will not be nicely packed/unpacked in a single "dereference" into/from bytes array as it is done now, but rather hand-packed with in four steps (magic, width,height,size), while converting the integers to Big-endian unsigned char[{2,4}] with appropriate functions. The decoding is analogous.

I know it is not as nice as the current packing of the header, but it would make it portable :) I even slightly prefer the manual packing/unpacking of the header as it

• makes it consistent with packing/unpacking the rest of the blocks
• does not depend on compiler properly packing the header struct (nitpick)

The downside is that you loose a bit of readability, but cost/benefit is worth it here IMHO.

---

Another approach would be to modify the header definition as:

typedef struct {
	qoi_magic_t magic;
	unsigned char width[2];     // Big-endian unsigned short
	unsigned char height[2];    // Big-endian unsigned short
	unsigned char size[4];      // Big-endian unsigned int
} qoi_header_t;

but in that case you would need to convert width,height and size to corresponding ints everytime you are accessing them, vice versa, which is not ideal.