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.

Sign up for GitHub

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

Some refactoring #218

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Some refactoring #218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f506ca4
remove trailing spaces
wx257osn2 Feb 7, 2022
bc4f3d3
make sign correctly
wx257osn2 Feb 7, 2022
7f43140
specify formats using inttypes.h
wx257osn2 Feb 7, 2022
626a2af
suppress warnings for unused parameters
wx257osn2 Feb 7, 2022
8210a10
use appropriate function
wx257osn2 Feb 7, 2022
f0e37a2
add static to translation unit local symbols
wx257osn2 Feb 7, 2022
3a19f11
add diagnostic options
wx257osn2 Feb 7, 2022
1cb2cab
make the narrowing conversion explicitly
wx257osn2 May 29, 2022
a53e75c
use %zu instead of PRIu64
wx257osn2 Jun 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
CC ?= gcc
CFLAGS_BENCH ?= -std=gnu99 -O3
CFLAGS_BENCH ?= -std=gnu99 -O3 -Wall -Wextra
LFLAGS_BENCH ?= -lpng
CFLAGS_CONV ?= -std=c99 -O3
CFLAGS_CONV ?= -std=c99 -O3 -Wall -Wextra -pedantic-errors

TARGET_BENCH ?= qoibench
TARGET_CONV ?= qoiconv
Expand Down
4 changes: 2 additions & 2 deletions qoi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ void *qoi_decode(const void *data, int size, qoi_desc *desc, int channels) {
pixels[px_pos + 0] = px.rgba.r;
pixels[px_pos + 1] = px.rgba.g;
pixels[px_pos + 2] = px.rgba.b;

if (channels == 4) {
pixels[px_pos + 3] = px.rgba.a;
}
Expand Down Expand Up @@ -657,7 +657,7 @@ void *qoi_read(const char *filename, qoi_desc *desc, int channels) {
return NULL;
}

bytes_read = fread(data, 1, size, f);
bytes_read = (int)fread(data, 1, size, f);
Copy link

Choose a reason for hiding this comment

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

fread returns ssize_t. Casting to int truncates to 2 GiB. Change size_t size, bytes_read; above instead.

Also this call to fread is lacking any error checking.

fclose(f);

pixels = qoi_decode(data, bytes_read, desc, channels);
Expand Down
89 changes: 46 additions & 43 deletions qoibench.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,11 @@ static uint64_t ns() {

typedef struct {
int size;
int capacity;
png_size_t capacity;
unsigned char *data;
} libpng_write_t;

void libpng_encode_callback(png_structp png_ptr, png_bytep data, png_size_t length) {
static void libpng_encode_callback(png_structp png_ptr, png_bytep data, png_size_t length) {
libpng_write_t *write_data = (libpng_write_t*)png_get_io_ptr(png_ptr);
if (write_data->size + length >= write_data->capacity) {
ERROR("PNG write");
Expand All @@ -131,7 +131,7 @@ void libpng_encode_callback(png_structp png_ptr, png_bytep data, png_size_t leng
write_data->size += length;
}

void *libpng_encode(void *pixels, int w, int h, int channels, int *out_len) {
static void *libpng_encode(void *pixels, int w, int h, int channels, int *out_len) {
Copy link

Choose a reason for hiding this comment

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

Prefer size_t for all usages as size/quantities, like w, h, channels and for out_len.

png_structp png = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
if (!png) {
ERROR("png_create_write_struct");
Expand Down Expand Up @@ -182,24 +182,26 @@ void *libpng_encode(void *pixels, int w, int h, int channels, int *out_len) {

typedef struct {
int pos;
int size;
png_size_t size;
unsigned char *data;
} libpng_read_t;

void png_decode_callback(png_structp png, png_bytep data, png_size_t length) {
static void png_decode_callback(png_structp png, png_bytep data, png_size_t length) {
libpng_read_t *read_data = (libpng_read_t*)png_get_io_ptr(png);
if (read_data->pos + length > read_data->size) {
ERROR("PNG read %ld bytes at pos %d (size: %d)", length, read_data->pos, read_data->size);
ERROR("PNG read %zu bytes at pos %d (size: %zu)", length, read_data->pos, read_data->size);
}
memcpy(data, read_data->data + read_data->pos, length);
read_data->pos += length;
}

void png_warning_callback(png_structp png_ptr, png_const_charp warning_msg) {
static void png_warning_callback(png_structp png_ptr, png_const_charp warning_msg) {
// Ignore warnings about sRGB profiles and such.
(void)png_ptr;
(void)warning_msg;
}

void *libpng_decode(void *data, int size, int *out_w, int *out_h) {
static void *libpng_decode(void *data, int size, int *out_w, int *out_h) {
png_structp png = png_create_read_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, png_warning_callback);
if (!png) {
ERROR("png_create_read_struct");
Expand All @@ -215,18 +217,18 @@ void *libpng_decode(void *data, int size, int *out_w, int *out_h) {
.size = size,
.data = data
};

png_set_read_fn(png, &read_data, png_decode_callback);
png_set_sig_bytes(png, 0);
png_read_info(png, info);

png_uint_32 w, h;
int bitDepth, colorType, interlaceType;
png_get_IHDR(png, info, &w, &h, &bitDepth, &colorType, &interlaceType, NULL, NULL);

// 16 bit -> 8 bit
png_set_strip_16(png);

// 1, 2, 4 bit -> 8 bit
if (bitDepth < 8) {
png_set_packing(png);
Expand All @@ -235,7 +237,7 @@ void *libpng_decode(void *data, int size, int *out_w, int *out_h) {
if (colorType & PNG_COLOR_MASK_PALETTE) {
png_set_expand(png);
}

if (!(colorType & PNG_COLOR_MASK_COLOR)) {
png_set_gray_to_rgb(png);
}
Expand All @@ -244,36 +246,37 @@ void *libpng_decode(void *data, int size, int *out_w, int *out_h) {
if (png_get_valid(png, info, PNG_INFO_tRNS)) {
png_set_tRNS_to_alpha(png);
}

// make sure every pixel has an alpha value
if (!(colorType & PNG_COLOR_MASK_ALPHA)) {
png_set_filler(png, 255, PNG_FILLER_AFTER);
}

png_read_update_info(png, info);

unsigned char* out = malloc(w * h * 4);
*out_w = w;
*out_h = h;

// png_uint_32 rowBytes = png_get_rowbytes(png, info);
png_bytep row_pointers[h];
for (png_uint_32 row = 0; row < h; row++ ) {
row_pointers[row] = (png_bytep)(out + (row * w * 4));
}

png_read_image(png, row_pointers);
png_read_end(png, info);
png_destroy_read_struct( &png, &info, NULL);

return out;
}


// -----------------------------------------------------------------------------
// stb_image encode callback

void stbi_write_callback(void *context, void *data, int size) {
static void stbi_write_callback(void *context, void *data, int size) {
(void)data;
int *encoded_size = (int *)context;
*encoded_size += size;
// In theory we'd need to do another malloc(), memcpy() and free() here to
Expand All @@ -284,7 +287,7 @@ void stbi_write_callback(void *context, void *data, int size) {
// -----------------------------------------------------------------------------
// function to load a whole file into memory

void *fload(const char *path, int *out_size) {
static void *fload(const char *path, int *out_size) {
FILE *fh = fopen(path, "rb");
if (!fh) {
ERROR("Can't open file");
Expand Down Expand Up @@ -313,14 +316,14 @@ void *fload(const char *path, int *out_size) {
// benchmark runner


int opt_runs = 1;
int opt_nopng = 0;
int opt_nowarmup = 0;
int opt_noverify = 0;
int opt_nodecode = 0;
int opt_noencode = 0;
int opt_norecurse = 0;
int opt_onlytotals = 0;
static int opt_runs = 1;
static int opt_nopng = 0;
static int opt_nowarmup = 0;
static int opt_noverify = 0;
static int opt_nodecode = 0;
static int opt_noencode = 0;
static int opt_norecurse = 0;
static int opt_onlytotals = 0;


typedef struct {
Expand All @@ -341,7 +344,7 @@ typedef struct {
} benchmark_result_t;


void benchmark_print_result(benchmark_result_t res) {
static void benchmark_print_result(benchmark_result_t res) {
res.px /= res.count;
res.raw_size /= res.count;
res.libpng.encode_time /= res.count;
Expand All @@ -358,16 +361,16 @@ void benchmark_print_result(benchmark_result_t res) {
printf(" decode ms encode ms decode mpps encode mpps size kb rate\n");
if (!opt_nopng) {
printf(
"libpng: %8.1f %8.1f %8.2f %8.2f %8ld %4.1f%%\n",
(double)res.libpng.decode_time/1000000.0,
(double)res.libpng.encode_time/1000000.0,
"libpng: %8.1f %8.1f %8.2f %8.2f %8zu %4.1f%%\n",
(double)res.libpng.decode_time/1000000.0,
(double)res.libpng.encode_time/1000000.0,
(res.libpng.decode_time > 0 ? px / ((double)res.libpng.decode_time/1000.0) : 0),
(res.libpng.encode_time > 0 ? px / ((double)res.libpng.encode_time/1000.0) : 0),
res.libpng.size/1024,
((double)res.libpng.size/(double)res.raw_size) * 100.0
);
printf(
"stbi: %8.1f %8.1f %8.2f %8.2f %8ld %4.1f%%\n",
"stbi: %8.1f %8.1f %8.2f %8.2f %8zu %4.1f%%\n",
(double)res.stbi.decode_time/1000000.0,
(double)res.stbi.encode_time/1000000.0,
(res.stbi.decode_time > 0 ? px / ((double)res.stbi.decode_time/1000.0) : 0),
Expand All @@ -377,7 +380,7 @@ void benchmark_print_result(benchmark_result_t res) {
);
}
printf(
"qoi: %8.1f %8.1f %8.2f %8.2f %8ld %4.1f%%\n",
"qoi: %8.1f %8.1f %8.2f %8.2f %8zu %4.1f%%\n",
(double)res.qoi.decode_time/1000000.0,
(double)res.qoi.encode_time/1000000.0,
(res.qoi.decode_time > 0 ? px / ((double)res.qoi.decode_time/1000.0) : 0),
Expand Down Expand Up @@ -405,7 +408,7 @@ void benchmark_print_result(benchmark_result_t res) {
} while (0)


benchmark_result_t benchmark_image(const char *path) {
static benchmark_result_t benchmark_image(const char *path) {
int encoded_png_size;
int encoded_qoi_size;
int w;
Expand All @@ -425,7 +428,7 @@ benchmark_result_t benchmark_image(const char *path) {
void *encoded_png = fload(path, &encoded_png_size);
void *encoded_qoi = qoi_encode(pixels, &(qoi_desc){
.width = w,
.height = h,
.height = h,
.channels = channels,
.colorspace = QOI_SRGB
}, &encoded_qoi_size);
Expand Down Expand Up @@ -468,7 +471,7 @@ benchmark_result_t benchmark_image(const char *path) {
BENCHMARK_FN(opt_nowarmup, opt_runs, res.stbi.decode_time, {
int dec_w, dec_h, dec_channels;
void *dec_p = stbi_load_from_memory(encoded_png, encoded_png_size, &dec_w, &dec_h, &dec_channels, 4);
free(dec_p);
stbi_image_free(dec_p);
});
}

Expand Down Expand Up @@ -501,7 +504,7 @@ benchmark_result_t benchmark_image(const char *path) {
int enc_size;
void *enc_p = qoi_encode(pixels, &(qoi_desc){
.width = w,
.height = h,
.height = h,
.channels = channels,
.colorspace = QOI_SRGB
}, &enc_size);
Expand All @@ -510,14 +513,14 @@ benchmark_result_t benchmark_image(const char *path) {
});
}

free(pixels);
stbi_image_free(pixels);
free(encoded_png);
free(encoded_qoi);

return res;
}

void benchmark_directory(const char *path, benchmark_result_t *grand_total) {
static void benchmark_directory(const char *path, benchmark_result_t *grand_total) {
DIR *dir = opendir(path);
if (!dir) {
ERROR("Couldn't open directory %s", path);
Expand All @@ -541,7 +544,7 @@ void benchmark_directory(const char *path, benchmark_result_t *grand_total) {
}

benchmark_result_t dir_total = {0};

int has_shown_head = 0;
for (int i = 0; (file = readdir(dir)) != NULL; i++) {
if (strcmp(file->d_name + strlen(file->d_name) - 4, ".png") != 0) {
Expand All @@ -555,7 +558,7 @@ void benchmark_directory(const char *path, benchmark_result_t *grand_total) {

char *file_path = malloc(strlen(file->d_name) + strlen(path)+8);
sprintf(file_path, "%s/%s", path, file->d_name);

benchmark_result_t res = benchmark_image(file_path);

if (!opt_onlytotals) {
Expand All @@ -564,7 +567,7 @@ void benchmark_directory(const char *path, benchmark_result_t *grand_total) {
}

free(file_path);

dir_total.count++;
dir_total.raw_size += res.raw_size;
dir_total.px += res.px;
Expand Down
2 changes: 1 addition & 1 deletion qoiconv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ int main(int argc, char **argv) {
else if (STR_ENDS_WITH(argv[2], ".qoi")) {
encoded = qoi_write(argv[2], pixels, &(qoi_desc){
.width = w,
.height = h,
.height = h,
.channels = channels,
.colorspace = QOI_SRGB
});
Expand Down