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.

diff --git a/src/image.c b/src/image.c
index 786e17aa729c43667b8f4b51be4303fbca85b2c1..853852000492e3413cf8e17f851ee6458c0be9e5 100644 (file)
--- a/src/image.c
+++ b/src/image.c
@@ -20,6 +20,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <assert.h>
+#include <stdbool.h>
 #include <inttypes.h>
 #include <errno.h>
 
@@ -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 10bd71802653542a1f361d79624359a82296675d..05d36608e085fd576344ce2b803f95685acbc84e 100644 (file)
--- 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 dc4697b31568c0f6cb7483f7fca3aabdb4b5ab25..cdba3af56bca64d05dc51a7a90cc996f45ace2ea 100644 (file)
--- 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" : "");
        }