From e58493762dbd70870b7bdc1fd9fc922485c91c2c Mon Sep 17 00:00:00 2001 From: Nick Bowler Date: Thu, 30 Jan 2014 22:00:27 -0500 Subject: [PATCH] liblbx: Clean up embedded palette handling We don't need to read the embedded palette information with the rest of the header, we can defer that to much later when the palette is actually needed. This makes more sense conceptually, simplifies the code a bit, and saves some bytes in the image state structure. Since we're basically rewriting lbx_img_getpalette anyway, take this opportunity to give it a better interface. It now returns the number of embedded palette entries, allowing us to eliminate the palette size feature in lbx_img_getinfo. --- src/image.c | 101 +++++++++++++++++++++------------------------------ src/image.h | 7 ++-- src/lbximg.c | 86 +++++++++++++++++++++++-------------------- 3 files changed, 91 insertions(+), 103 deletions(-) diff --git a/src/image.c b/src/image.c index 786e17a..8538520 100644 --- a/src/image.c +++ b/src/image.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -52,14 +53,11 @@ struct lbx_image_priv { unsigned short wtf, flags; unsigned char wtf2; - unsigned short palstart, palcount; const struct lbx_file_ops *fops; int (*dtor)(void *handle); void *f; - long paloff; - /* State of frame readout */ unsigned currentx, currenty, currentn; int read_state; @@ -143,27 +141,6 @@ struct lbx_image *lbx_img_open(void *f, const struct lbx_file_ops *fops, img->offsets[i] = unpack_32_le(buf); } - if (img->flags & FLAG_PALETTE) { - unsigned char buf[4]; - - if (fops->read(buf, sizeof buf, f) != sizeof buf) { - if (fops->eof(f)) - lbx_error_raise(LBX_EEOF); - free(img); - return NULL; - } - - img->palstart = unpack_16_le(buf+0); - img->palcount = unpack_16_le(buf+2); - img->paloff = fops->tell(f); - - if (img->palstart + img->palcount > 256) { - lbx_error_raise(LBX_EFORMAT); - free(img); - return NULL; - } - } - return &img->pub; } @@ -345,29 +322,29 @@ long lbx_img_read_row_data(struct lbx_image *pub, void *buf) return img->currentn; } -int -lbx_img_loadpalette(void *f, const struct lbx_file_ops *fops, - struct lbx_colour palette[static 256]) +static int read_palette(void *f, const struct lbx_file_ops *fops, + struct lbx_colour *palette, unsigned count, + bool external) { - unsigned char entry[4]; - int i; + assert(count <= 256); + for (unsigned i = 0; i < count; i++) { + unsigned char buf[4]; - for (i = 0; i < 256; i++) { - if (fops->read(entry, sizeof entry, f) != sizeof entry) { + if (fops->read(buf, 4, f) != 4) { if (fops->eof(f)) lbx_error_raise(LBX_EEOF); return -1; } - if (entry[0] != 1) { + if (buf[0] != external) { lbx_error_raise(LBX_EFORMAT); return -1; } palette[i] = (struct lbx_colour) { - .red = entry[1] & 0x3f, - .green = entry[2] & 0x3f, - .blue = entry[3] & 0x3f, + .red = buf[1] & 0x3f, + .green = buf[2] & 0x3f, + .blue = buf[3] & 0x3f, .active = 1, }; } @@ -375,42 +352,48 @@ lbx_img_loadpalette(void *f, const struct lbx_file_ops *fops, return 0; } -int -lbx_img_getpalette(struct lbx_image *pub, struct lbx_colour palette[static 256]) +int lbx_img_loadpalette(void *f, const struct lbx_file_ops *fops, + struct lbx_colour *palette) +{ + return read_palette(f, fops, palette, 256, true); +} + +int lbx_img_getpalette(struct lbx_image *pub, struct lbx_colour *out) { struct lbx_image_priv *img = (struct lbx_image_priv *)pub; - unsigned char entry[4]; - unsigned int i; - size_t rc; + unsigned long palette_start, palette_count, palette_offset; + unsigned char buf[4]; + int rc; /* Do nothing if the image doesn't have embedded palette data. */ if (!(img->flags & FLAG_PALETTE)) return 0; - if (img->fops->seek(img->f, img->paloff, SEEK_SET)) { + palette_offset = 16 + 4ul * img->pub.frames; + if (img->fops->seek(img->f, palette_offset, SEEK_SET)) { return -1; } - for (i = 0; i < img->palcount; i++) { - rc = img->fops->read(entry, sizeof entry, img->f); - if (rc < sizeof entry) { - goto readerr; - } + /* Read embedded palette header */ + if (img->fops->read(buf, 4, img->f) < 4) + goto readerr; - if (entry[0] != 0) { - lbx_error_raise(LBX_EFORMAT); - return -1; - } + palette_start = unpack_16_le(buf+0); + palette_count = unpack_16_le(buf+2); + if (palette_start + palette_count > 256) { + lbx_error_raise(LBX_EFORMAT); + return -1; + } - palette[img->palstart + i] = (struct lbx_colour){ - .red = entry[1], - .green = entry[2], - .blue = entry[3], - .active = 1, - }; + if (out) { + rc = read_palette(img->f, img->fops, + out+palette_start, palette_count, + false); + if (rc < 0) + return -1; } - return 0; + return palette_count; readerr: if (img->fops->eof(img->f)) lbx_error_raise(LBX_EEOF); @@ -421,9 +404,7 @@ void lbx_img_getinfo(struct lbx_image *pub, struct lbx_imginfo *info) { struct lbx_image_priv *img = (struct lbx_image_priv *)pub; - *info = (struct lbx_imginfo) { - .palettesz = (img->flags & FLAG_PALETTE) ? img->palcount : 0, - }; + *info = (struct lbx_imginfo) { 0 }; /* There seems to be two ways of specifying that an image loops. */ if (img->flags & FLAG_LOOPING) { diff --git a/src/image.h b/src/image.h index 10bd718..05d3660 100644 --- a/src/image.h +++ b/src/image.h @@ -16,7 +16,7 @@ struct lbx_colour { struct lbx_imginfo { unsigned char loopstart; - int palettesz, looping; + int looping; }; struct lbx_image *lbx_img_open(void *f, const struct lbx_file_ops *fops, @@ -28,10 +28,9 @@ int lbx_img_seek(struct lbx_image *img, unsigned frame); long lbx_img_read_row_header(struct lbx_image *img, unsigned *x, unsigned *y); long lbx_img_read_row_data(struct lbx_image *img, void *buf); +int lbx_img_getpalette(struct lbx_image *img, struct lbx_colour *palette); int lbx_img_loadpalette(void *f, const struct lbx_file_ops *fops, - struct lbx_colour palette[static 256]); -int lbx_img_getpalette(struct lbx_image *img, - struct lbx_colour palette[static 256]); + struct lbx_colour *palette); void lbx_img_getinfo(struct lbx_image *img, struct lbx_imginfo *info); diff --git a/src/lbximg.c b/src/lbximg.c index dc4697b..cdba3af 100644 --- a/src/lbximg.c +++ b/src/lbximg.c @@ -206,66 +206,70 @@ int output(unsigned int frameno, const struct img_format *fmt, static int loadoverride(FILE *f, struct lbx_colour palette[static 256]) { - struct lbx_image *overimg = lbx_img_open(f, &lbx_default_fops, NULL); - struct lbx_imginfo info; + struct lbx_image *img; + int rc, ret = 0; - if (!overimg) { + img = lbx_img_open(f, &lbx_default_fops, NULL); + if (!img) { tool_err(-1, "failed to open override image: %s", lbx_errmsg()); return -1; } - lbx_img_getinfo(overimg, &info); - - if (!info.palettesz) { - tool_err(-1, "override image has no palette."); - lbx_img_close(overimg); - return -1; - } - if (lbx_img_getpalette(overimg, palette) == -1) { + rc = lbx_img_getpalette(img, palette); + if (rc < 0) { tool_err(-1, "error reading override palette: %s", lbx_errmsg()); - lbx_img_close(overimg); - return -1; + ret = -1; + } else if (rc == 0) { + tool_err(-1, "override image has no palette."); + ret = -1; } - lbx_img_close(overimg); - return 0; + lbx_img_close(img); + return ret; } -static int loadpalette(struct lbx_image *img, struct lbx_imginfo *info, - FILE *palf, FILE *override, - struct lbx_colour palette[static 256]) +static int loadpalette(struct lbx_image *img, FILE *palf, FILE *override, + struct lbx_colour *palette) { - int i; - - /* For sanity. */ - if (!palf && !info->palettesz && !override) { - tool_err(-1, "no palette available."); - return -1; - } + int rc, ret = -1; /* Default the palette to a wonderful pink. */ - for (i = 0; i < 256; i++) { + for (unsigned i = 0; i < 256; i++) { palette[i] = (struct lbx_colour){0x3f, 0x00, 0x3f}; } /* Read the external palette, if any. */ - if (palf && lbx_img_loadpalette(palf, &lbx_default_fops, palette) != 0) { - tool_err(-1, "error reading external palette: %s", lbx_errmsg()); - return -1; + if (palf) { + rc = lbx_img_loadpalette(palf, &lbx_default_fops, palette); + if (rc < 0) { + tool_err(-1, "error reading external palette: %s", lbx_errmsg()); + return -1; + } + + ret = 0; } - /* Read the embedded palette, if any. */ - if (info->palettesz && lbx_img_getpalette(img, palette) == -1) { + /* Read the embedded palette */ + rc = lbx_img_getpalette(img, palette); + if (rc < 0) { tool_err(-1, "error reading embedded palette: %s", lbx_errmsg()); return -1; + } else if (rc > 0) { + ret = 0; } /* Read the override palette, if any. */ - if (override && loadoverride(override, palette) == -1) { - return -1; + if (override) { + rc = loadoverride(override, palette); + if (rc < 0) + return -1; + ret = 0; } - return 0; + /* If we literally have no palette data at all, may as well fail. */ + if (ret < 0) + tool_err(-1, "no palette available."); + return ret; } /* Return true iff a divides b. */ @@ -341,7 +345,6 @@ decode(struct lbx_image *img, FILE *palf, FILE *override, int fmt, char **argv) { unsigned char *pixels = NULL, *pixel_mask = NULL, *framebits = NULL; struct lbx_colour palette[256]; - struct lbx_imginfo info; int rc, ret = EXIT_FAILURE; int extracted = 0; unsigned int i; @@ -349,8 +352,6 @@ decode(struct lbx_image *img, FILE *palf, FILE *override, int fmt, char **argv) assert(fmt >= 0 && fmt < sizeof formats / sizeof formats[0]); - lbx_img_getinfo(img, &info); - npixels = img->width; if (img->height && npixels >= SIZE_MAX / img->height) { tool_err(-1, "image too large"); @@ -392,7 +393,7 @@ decode(struct lbx_image *img, FILE *palf, FILE *override, int fmt, char **argv) } if (usepalette) { - if (loadpalette(img, &info, palf, override, palette) == -1) { + if (loadpalette(img, palf, override, palette) == -1) { ret = EXIT_FAILURE; goto err; } @@ -537,14 +538,21 @@ int main(int argc, char **argv) if (verbose || mode == MODE_IDENT) { struct lbx_imginfo info; + int palette_count; if (!file) file = "stdin"; + palette_count = lbx_img_getpalette(img, NULL); + if (palette_count < 0) { + tool_err(-1, "error reading image: %s", lbx_errmsg()); + return EXIT_FAILURE; + } + lbx_img_getinfo(img, &info); printf("%s is %hux%hu LBX image, %hhu frame(s)%s%s%s\n", file, img->width, img->height, img->frames, - info.palettesz ? ", embedded palette" : "", + palette_count ? ", embedded palette" : "", img->chunk ? ", chunked" : "", info.looping ? ", loops" : ""); } -- 2.43.0