I've probed the clang analyzer on the code and it gives one warning. As the code is .. hm .. complex .. I can't judge if this is a valid one, so I simply show it here:

report.zip

Hello,

In the example below previous frames are wrongly preserved in subsequent frames:

=>

I tested it in the python version. Thanks for looking into it.

Is there a problem with this GIF?

https://github.com/robert-ancell/pygif/blob/master/test-suite/extra-pixels.gif

The following simple program crashes here with a memory corruption:

#include <cstdio>

#include "gif_load.h"

void frame_cb( void *data_, GIF_WHDR *whdr_ ) {
  printf( "frame_cb: %s, %p  frame #%ld/%ld, %ld x %ld, delay: %ld\n",
    (char *)data_, (void *)whdr_, whdr_->ifrm, whdr_->nfrm,
     whdr_->frxd, whdr_->fryd, whdr_->time );
}

int main( int argc_, char *argv_[] ) {
  if ( argc_ <= 1 ) return 0;
  printf( "load %s\n", argv_[1] );
  FILE *gif = fopen( argv_[1], "r" );
  long len = 0;
  char *buf = 0;
  if ( !( gif && fseek( gif, 0, SEEK_END ) >= 0 &&
        ( len = ftell( gif ) ) >= 0             &&
        ( buf = (char *)malloc( (size_t)len ) ) &&
        fseek( gif, 0, SEEK_SET ) >= 0          &&
        fread( buf, 1, (size_t)len, gif ) == (size_t)len ) ) {
    perror( argv_[1] );
    free( buf );
    if ( gif ) fclose( gif );
    return 1;
  }
  fclose( gif );

  printf( "decode '%s', buf = %p, len = %lu\n", argv_[1], buf, len );
  long ret = GIF_Load( buf, len, frame_cb, 0, argv_[1], 0 );
  printf( "ret = %ld\n", ret );

  return 0;
}

output:

load ../pygif/test-suite/extra-pixels.gif
decode '../pygif/test-suite/extra-pixels.gif', buf = 0x558f4942e8b0, len = 60
frame_cb: ../pygif/test-suite/extra-pixels.gif, 0x7fff97f3d480  frame #0/1, 1 x 1, delay: 0
free(): invalid next size (normal)

Strangely the demo program (from README) outputs the tgacorrectly and does not crash.

This is with Ubuntu 18.10.

First, thanks for the awesome work.

I noticed that the transparency is not preserved after the first frame on certain gifs. In the example below the transparent layer disappears after the first frame.

this ones transparency is even more problematic:

Thanks for the help in advance.

Thanks for this awesome work!

I use GIFLIB currently for decoding, but am tempted to switch to this header only masterpiece.

There is only one minor flaw using it in class contexts:

I am including gif_load.h in the source file of my class. Declaring the private callback method static MyClass::gifLoadCallback(void *, GIF_WHDR *) in the corresponding header is unfortunately not possible, as one cannot forward declare a typedef struct.

[Declaring the callback locally as a non member function in the source file is not an option, because called methods by MyClass would need to be declared public and with a void *argument for GIF_WHDR - both is not very nice].

As a workaround I have changed gif_load.h by declaring the struct without typedef as _GIF_WHDR and follow it with a typedef struct _GIF_WHDR GIF_WHDR and use _GIF_WHDR for the forward declaration.

Here is the diff:

diff --git a/gif_load.h b/gif_load.h
index e2cace7..28e3200 100644
--- a/gif_load.h
+++ b/gif_load.h
@@ -46,7 +46,7 @@ extern "C" {
 #define _GIF_SWAP(h) ((GIF_BIGE)? ((uint16_t)(h << 8) | (h >> 8)) : h)
 
 #pragma pack(push, 1)
-typedef struct {                 /** ======== frame writer info: ======== **/
+struct _GIF_WHDR {               /** ======== frame writer info: ======== **/
     long xdim, ydim, clrs,       /** global dimensions, palette size      **/
          bkgd, tran,             /** background index, transparent index  **/
          intr, mode,             /** interlace flag, frame blending mode  **/
@@ -56,8 +56,9 @@ typedef struct {                 /** ======== frame writer info: ======== **/
     struct {                     /** [==== GIF RGB palette element: ====] **/
         uint8_t R, G, B;         /** [color values - red, green, blue   ] **/
     } *cpal;                     /** current palette                      **/
-} GIF_WHDR;
+};
 #pragma pack(pop)
+typedef struct _GIF_WHDR GIF_WHDR;
 
 enum {GIF_NONE = 0, GIF_CURR = 1, GIF_BKGD = 2, GIF_PREV = 3};
 

Do you think this is a good solution or do you have a better idea?

Frame #140 contains some artefacts:

Code:

void FrameCallback(GIF_GHDR *ghdr, GIF_FHDR *curr, GIF_FHDR *prev,
                   GIF_RGBX *cpal, long clrs, uint8_t *bptr, void *data,
                   long nfrm, long tran, long time, long indx) {
    uint32_t *pict, x, y, yoff, iter, ifin, dsrc, ddst;
    uintptr_t *file = (uintptr_t*)data;
    uint8_t head[18] = {};

    #define BGRA(i) (cpal[bptr[i]].R << 16) | (cpal[bptr[i]].G << 8) \
                   | cpal[bptr[i]].B | ((i != tran)? 0xFF000000 : 0)
    if (!indx) {
        /** this is the very first frame, so we must write the header **/
        head[ 2] = 2;
        head[12] = (ghdr->xdim     ) & 0xFF;
        head[13] = (ghdr->xdim >> 8) & 0xFF;
        head[14] = ((labs(1) * ghdr->ydim)     ) & 0xFF;
        head[15] = ((labs(1) * ghdr->ydim) >> 8) & 0xFF;
        head[16] = 32;   /** 32 bits depth **/
        head[17] = 0x20; /** top-down flag **/
        write(file[0], head, 18);
        file[1] = (uintptr_t)calloc(ghdr->xdim * ghdr->ydim, sizeof(uint32_t));
    }
    /** interlacing support **/
    iter = (curr->flgs & GIF_FINT)? 0 : 4;
    ifin = (curr->flgs & GIF_FINT)? 4 : 5;

    pict = (uint32_t*)file[1];
    if ((uintptr_t)prev > (uintptr_t)sizeof(prev)) {
        /** background: previous frame with a hole **/
        ddst = ghdr->xdim * prev->yoff + prev->xoff;
        for (y = 0; y < prev->ydim; y++)
            for (x = 0; x < prev->xdim; x++)
                pict[ghdr->xdim * y + x + ddst] = BGRA(ghdr->bkgd);
    }
    /** [TODO:] the frame is assumed to be inside global bounds,
                however it might exceed them in some GIFs; fix me. **/
    ddst = ghdr->xdim * curr->yoff + curr->xoff;
    for (dsrc = -1; iter < ifin; iter++)
        for (yoff = 16 >> ((iter > 1)? iter : 1), y = (8 >> iter) & 7;
             y < curr->ydim; y += yoff)
            for (x = 0; x < curr->xdim; x++)
                if (tran != (long)bptr[++dsrc])
                    pict[ghdr->xdim * y + x + ddst] = BGRA(dsrc);
static int counter=0;
if( counter++ == 140 )
    write(file[0], pict, ghdr->xdim * ghdr->ydim * sizeof(uint32_t));
    #undef BGRA
}
int main(int argc, char *argv[])
{
    intptr_t file[2];
    void *data;

    if ((file[0] = open("/tmp/t.gif", O_RDONLY)) > 0) {
        file[1] = lseek(file[0], 0, SEEK_END);
        lseek(file[0], 0, SEEK_SET);
        read(file[0], data = malloc(file[1]), file[1]);
        close(file[0]);
        if ((file[0] = open("/tmp/t.tga", O_CREAT | O_WRONLY, 0644)) > 0) {
            GIF_Load(data, file[1], 0, FrameCallback, (void*)file);
            free((void*)file[1]); /** gets rewritten in FrameCallback() **/
            close(file[0]);
        }
        free(data);
        return 0;
    }
    return 1;
}

Thanks for your great repo firstly! However, since I barely know anything about GIF, it still bothers me how to read a gif file into a data structure like above?

This is just a request for a feature you may want to support:

The standard says, that both global and local color tables are no required blocks. In that case the application can use a default colormap.

Currently gif_load.h does not parse such images.

I have made some changes in my copy to support it:

(1) Continue parsing when clrs is 0. (2) Give access to the palette definition in the GIF_WHDR structure. (3) Return the value of the GIF header field color resultion. As I read it, it can be used in such a case to determine how many colors the palette has in order to supply a suitable replacement.

(1) and (2) would be nice to have, (3) could in principle also be done in the application, because the value is at a fixed position in the file.

These are my changes:

diff --git a/gif_load.h b/gif_load.h
index 12cc402..99324bb 100644
--- a/gif_load.h
+++ b/gif_load.h
@@ -48,12 +48,13 @@ extern "C" {
 #pragma pack(push, 1)
 struct GIF_WHDR {                /** ======== frame writer info: ======== **/
     long xdim, ydim, clrs,       /** global dimensions, palette size      **/
+         cres,                   /** color resolution: 2^(cres+1) = #clrs **/
          bkgd, tran,             /** background index, transparent index  **/
          intr, mode,             /** interlace flag, frame blending mode  **/
          frxd, fryd, frxo, fryo, /** current frame dimensions and offset  **/
          time, ifrm, nfrm;       /** delay, frame number, frame count     **/
     uint8_t *bptr;               /** frame pixel indices or metadata      **/
-    struct {                     /** [==== GIF RGB palette element: ====] **/
+    struct CPAL {                /** [==== GIF RGB palette element: ====] **/
         uint8_t R, G, B;         /** [color values - red, green, blue   ] **/
     } *cpal;                     /** current palette                      **/
 };
@@ -225,6 +226,7 @@ GIF_EXTR long GIF_Load(void *data, long size,
     || ((buff[4] != 55) && (buff[4] != 57)) || (buff[5] != 97) || !gwfr)
         return 0;
 
+    whdr.cres = (ghdr->flgs >> 4) & 7;
     buff = (uint8_t*)(ghdr + 1) /** skipping the global header and palette **/
          + _GIF_LoadHeader(ghdr->flgs, 0, 0, 0, 0, 0L) * 3L;
     if ((size -= buff - (uint8_t*)ghdr) <= 0)
@@ -238,7 +240,7 @@ GIF_EXTR long GIF_Load(void *data, long size,
         if (desc == GIF_FHDM) {
             fhdr = (struct GIF_FHDR*)whdr.bptr;
             if (_GIF_LoadHeader(ghdr->flgs, &whdr.bptr, (void**)&whdr.cpal,
-                                fhdr->flgs, &blen, sizeof(*fhdr)) <= 0)
+                                fhdr->flgs, &blen, sizeof(*fhdr)) < 0)
                 break;
             whdr.frxd = _GIF_SWAP(fhdr->frxd);
             whdr.fryd = _GIF_SWAP(fhdr->fryd);
@@ -258,7 +260,7 @@ GIF_EXTR long GIF_Load(void *data, long size,
             *(void**)&whdr.cpal = (void*)(ghdr + 1); /** interlaced? -^ **/
             whdr.clrs = _GIF_LoadHeader(ghdr->flgs, &buff, (void**)&whdr.cpal,
                                         fhdr->flgs, &size, sizeof(*fhdr));
-            if ((skip <= ++whdr.ifrm) && ((whdr.clrs <= 0)
+            if ((skip <= ++whdr.ifrm) && ((whdr.clrs < 0)
             ||  (_GIF_LoadFrame(&buff, &size,
                                  whdr.bptr, whdr.bptr + blen) < 0)))
                 size = -(whdr.ifrm--) - 1; /** failed to load the frame **/

In my program I'm doing something like that, when there are no colors:

void frame_cb(void *data_, GIF_WHDR *whdr_) {
  // ...
  if (!whdr_.clrs) {
    // no colors: use default table
    static struct GIF_WHDR::CPAL defClrs[256];
    whdr_.clrs = 1 << (whdr_.cres + 1);
    whdr_.cpal = defClrs;
    memset(defClrs, 0, sizeof(defClrs)); // Note: also sets first color to black
    defClrs[1].R = defClrs[1].G = defClrs[1].B = 0xff; // white
    for (int i = 2; i < whdr_.clrs; i++)
      defClrs[i].R = defClrs[i].G = defClrs[i].B = (uchar)(255 * i / (whdr_.clrs - 1));
  }
  // ...
}