Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement QOI image format #10

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

Rangi42
Copy link
Contributor

@Rangi42 Rangi42 commented Nov 28, 2023

Spec: https://qoiformat.org/qoi-specification.pdf

Tested on the PNG and QOI images in qoi_test_images.zip, and on mandelbrot.png for a large 32768x32768 test case that the reference encoder can't handle (it's limited to 400-megapixel input, whereas this implementation is not).

After this is merged, fuzz.c should be updated to handle QOI files.

After the next version is released, the qoi repo should be PRed to list libplum in the supporting software.

This pull request is free and unencumbered software released into the public domain, under the same Unlicense as libplum.

@Rangi42 Rangi42 marked this pull request as draft November 28, 2023 05:50
@Rangi42 Rangi42 marked this pull request as ready for review November 28, 2023 06:40
@@ -0,0 +1,59 @@
#include "proto.h"

#define pixelcolor(px) (((uint32_t) (px).a << 24) | ((uint32_t) (px).b << 16) | ((uint32_t) (px).g << 8) | (uint32_t) (px).r)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is basically PLUM_COLOR_VALUE_32, but (a) then I'd have to do PLUM_COLOR_VALUE_32(px.r, px.g, px.b, px.a), and (b) none of your src files actually use any of the convenience macros in header/color.h anyway.

@@ -147,3 +147,10 @@ struct PNM_image_header {
size_t datastart;
size_t datalength;
};

struct QOI_pixel {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did consider using four local uint8_t variables, or one packed uint32_t, but the overall complexity and line count would be higher, as values need to be packed into a lookup table and then unpacked for per-channel manipulation.


#define pixelcolor(px) (((uint32_t) (px).a << 24) | ((uint32_t) (px).b << 16) | ((uint32_t) (px).g << 8) | (uint32_t) (px).r)

void load_QOI_data (struct context * context, unsigned flags, size_t limit) {
Copy link
Contributor Author

@Rangi42 Rangi42 Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core QOI read/load/decode and write/generate/decode loops are simple enough that I'm not filling them with commentary; you have the spec and qoi.h for reference.

prev = px;
}
}
output += byteappend(output, 0, 0, 0, 0, 0, 0, 0, 1);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered using a memset for the 0s and then *output = 1;, but it would be more lines.

}
return node -> data;
}

static inline bool bit_depth_less_than (uint32_t depth, uint32_t target) {
// formally "less than or equal to", but that would be a very long name
Copy link
Contributor Author

@Rangi42 Rangi42 Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"bit_depth_lte" would be shorter and not need a clarifying comment.

@@ -51,6 +51,16 @@ static inline void * append_output_node (struct context * context, size_t size)
return node -> data;
}

static inline void * resize_output_node (struct context * context, void * data, size_t size) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alternative I saw to "allocate one large-as-possible node and then shrink it after QOI compression is done" was "allocate lots of little nodes for every QOI chunk", and this seemed better.

Comment on lines +27 to +28
for (size_t cell = 0; cell < framesize; cell ++) {
struct QOI_pixel px = {.r = data[cell], .g = data[cell] >> 8, .b = data[cell] >> 16, .a = data[cell] >> 24};
Copy link
Contributor Author

@Rangi42 Rangi42 Dec 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other possibilities which would use an extra line:

  uint32_t * dataend = data + framesize;
  for (uint32_t * val = data; val < data; val ++) {
    struct QOI_pixel px = {.r = *val, .g = *val >> 8, .b = *val >> 16, .a = *val >> 24};
...
      if (run == 62 || val == dataend - 1) {
  for (size_t cell = 0; cell < framesize; cell ++) {
    uint32_t val = data[cell];
    struct QOI_pixel px = {.r = val, .g = val >> 8, .b = val >> 16, .a = val >> 24};
...
      if (run == 62 || cell == framesize - 1) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant