Skip to content

Instantly share code, notes, and snippets.

@urraka

urraka/stb.c

Last active Feb 24, 2021
Embed
What would you like to do?
#define STB_IMAGE_IMPLEMENTATION
#define STB_IMAGE_WRITE_IMPLEMENTATION
#define STBI_ONLY_PNG
#define STBI_ONLY_JPEG
#define STBI_ONLY_BMP
#define STBI_ONLY_GIF
#include "stb_image.h"
#include "stb_image_write.h"
#ifdef __cplusplus
extern "C" {
#endif
typedef struct gif_result_t {
int delay;
unsigned char *data;
struct gif_result_t *next;
} gif_result;
STBIDEF unsigned char *stbi_xload(char const *filename, int *x, int *y, int *frames)
{
FILE *f;
stbi__context s;
unsigned char *result = 0;
if (!(f = stbi__fopen(filename, "rb")))
return stbi__errpuc("can't fopen", "Unable to open file");
stbi__start_file(&s, f);
if (stbi__gif_test(&s))
{
int c;
stbi__gif g;
gif_result head;
gif_result *prev = 0, *gr = &head;
memset(&g, 0, sizeof(g));
memset(&head, 0, sizeof(head));
*frames = 0;
while (gr->data = stbi__gif_load_next(&s, &g, &c, 4))
{
if (gr->data == (unsigned char*)&s)
{
gr->data = 0;
break;
}
if (prev) prev->next = gr;
gr->delay = g.delay;
prev = gr;
gr = (gif_result*) stbi__malloc(sizeof(gif_result));
memset(gr, 0, sizeof(gif_result));
++(*frames);
}
STBI_FREE(g.out);
if (gr != &head)
STBI_FREE(gr);
if (*frames > 0)
{
*x = g.w;
*y = g.h;
}
result = head.data;
if (*frames > 1)
{
unsigned int size = 4 * g.w * g.h;
unsigned char *p = 0;
result = (unsigned char*)stbi__malloc(*frames * (size + 2));
gr = &head;
p = result;
while (gr)
{
prev = gr;
memcpy(p, gr->data, size);
p += size;
*p++ = gr->delay & 0xFF;
*p++ = (gr->delay & 0xFF00) >> 8;
gr = gr->next;
STBI_FREE(prev->data);
if (prev != &head) STBI_FREE(prev);
}
}
}
else
{
result = stbi__load_main(&s, x, y, frames, 4);
*frames = !!result;
}
fclose(f);
return result;
}
#ifdef __cplusplus
}
#endif
@urraka

This comment has been minimized.

Copy link
Owner Author

@urraka urraka commented Sep 13, 2015

Notes:

  • stbi_xload returns a buffer with all frames like [image#0][delay#0][image#1][delay#1][...].
  • Number of frames is returned through frames parameter.
  • The delay for each frame is a 2 bytes little endian unsigned integer.
  • All frames are given in RGBA format.
  • All frames have the same width and height returned through x, y parameters.
  • A single image buffer (with no delay info) is returned for non-gif files and for gifs that have 1 frame.
  • The loading skips an y-flip check stb_image does.
@Flix01

This comment has been minimized.

Copy link

@Flix01 Flix01 commented Feb 2, 2016

@urraka
Is this code free from memory leaks ?
I ask because I'm using a slightly modified version of your code (that's why I'm not sure if the leaks are in your code too :) ), and Valgrind tells me:

388,800 bytes in 1 blocks are definitely lost in loss record 649 of 649
  in ImGui::AnimatedGif::load(char const*) in ...myFilePathAndLine..., but it is correspondent to line 44 of your code
  1: malloc in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so
  2: stbi__malloc(unsigned long) in stb_image.h:899
  3: stbi__gif_load_next(stbi__context*, stbi__gif*, int*, int) in stb_image.h:5801
  4: ImGui::AnimatedGif::load(char const*) in ...myFilePathAndLine..., but it is correspondent to line 44 of your code

To cut the story short, it seems that:
line 44: while (gr->data = stbi__gif_load_next(&s, &g, &c, 4)) calls stbi__gif_load_next(stbi__context *s, stbi__gif *g, int *comp, int req_comp) in stb_image.h
line 5801 in stb_image.h: g->out = (stbi_uc *) stbi__malloc(4 * g->w * g->h); allocated g->out that, according to Valgrind, is never released.

As I said before, I'm not sure if this happens because of my (slight) modification of your code or if the leak happens in your code as well.

I'm using stb_image - v2.10, and I'm here because a link to this gist is present in that file.

@Flix01

This comment has been minimized.

Copy link

@Flix01 Flix01 commented Feb 2, 2016

I've managed to solve the memory leak this way in my code:

gif_result (and gif_result_t) replacement:

struct gif_result : stbi__gif {
unsigned char *data;
struct gif_result *next;
};

Then in lines 44 and below:

        while (gr->data = stbi__gif_load_next(&s, &g, &c, 4))
        {
            STBI_FREE(gr->out);gr->out=NULL; // New!
            if (gr->data == (unsigned char*)&s)
            {
                gr->data = 0;
                break;
            }

            if (prev) prev->next = gr;
            gr->delay = g.delay;
            prev = gr;
            gr = (gif_result*) stbi__malloc(sizeof(gif_result));
            memset(gr, 0, sizeof(gif_result));
            ++(*frames);
        }

        STBI_FREE(gr->out);gr->out=NULL; // New!

        if (gr != &head)   
                STBI_FREE(gr);      

Still, I did these changes in my (slightly modified) code and I ported them to your code.
I don't know if they are appropriate (= work) or not.

@urraka

This comment has been minimized.

Copy link
Owner Author

@urraka urraka commented Feb 11, 2016

@Flix01

I'm sorry I didn't reply earlier. GitHub didn't notify me of this and I just found your comment randomly. I won't claim that my code is free from memory leaks, but I see some issues with your changes.

First, I'm going to assume that where you have gr->out you actually mean gr->data (I'm guessing you renamed it and it's just a typo in the post).

Regarding your first "New!" line: You are freeing the image data as soon as you retrieve it. How could you possibly return any (valid) image data that way? Also note that after that line I'm comparing gr->data with &s. That is done because stbi__gif_load_next will return a pointer to the stbi__context when it finds some kind of terminating chunk from the gif format. You're not supposed to free that pointer because it's allocated in the stack at the beggining of stbi_xload.

Second "New!" line: There are two conditions under which the while loop will end. One is when gr->data is 0 (so no need to free it) and another is when gr->data equals &s (no need to free it either, and it assigns 0 to it before breaking anyway). As far as I can see this line is harmless (free will accept null pointers just fine, if I recall correctly) but it's also useless (the data is always null).

Note that you are supposed to call stbi_image_free on whatever this function returns. Also, when the gif file has only one frame this function will return the result of stbi__gif_load_next directly, so that data shouldn't be freed by stbi_xload in that case.

@Flix01

This comment has been minimized.

Copy link

@Flix01 Flix01 commented Feb 16, 2016

@urraka

I'm sorry I didn't reply earlier.

Sorry from me too (same reason).

First, I'm going to assume that where you have gr->out you actually mean gr->data (I'm guessing you renamed it and it's just a typo in the post).

Actually now gr is an instance of gif_result, that derives from:

typedef struct
{
   int w,h;
   stbi_uc *out, *old_out;             // output buffer (always 4 components)
   int flags, bgindex, ratio, transparent, eflags, delay;
   stbi_uc  pal[256][4];
   stbi_uc lpal[256][4];
   stbi__gif_lzw codes[4096];
   stbi_uc *color_table;
   int parse, step;
   int lflags;
   int start_x, start_y;
   int max_x, max_y;
   int cur_x, cur_y;
   int line_size;
} stbi__gif;

So there is gr->out (I'm using stb_image.h version 2.10).

That told, I'm aware that my code might have problems... some of your observations seem correct to me.

The problem is just that with the code you posted I have memory leaks, and I did not find another way to get rid of them :(.

@urraka

This comment has been minimized.

Copy link
Owner Author

@urraka urraka commented Feb 16, 2016

Woops, I totally missed the inheritance you had there...

Anyway, I don't remember exactly how stbi__gif_load_next works but I looked a bit into it now and it seems there is a leak indeed. As far as I can see all you need is that STBI_FREE(gr->out) you added after the while loop. Freeing inside the loop still seems incorrect because gr->out and gr->data point at the same image data (and that's used later on). Also, in some cases stbi__gif_load_next will need the data from the previous frame.

I think you can remove the gif_result : stbi__gif inheritance and just call STBI_FREE(s.out) after the while loop.

@Flix01

This comment has been minimized.

Copy link

@Flix01 Flix01 commented Feb 16, 2016

Freeing inside the loop still seems incorrect because gr->out and gr->data point at the same image data (and that's used later on). Also, in some cases stbi__gif_load_next will need the data from the previous frame.

I had the same impression... but in the (2 or 3) .gifs I've tried so far, Valgrind still gives me a memory leak if I comment out the STBI_FREE(gr->out) inside the loop. However I think that, as you say, there might be cases in which this approach would fail... I don't know how to find a robust solution to this (and actually I don't know why my code seems to work, if I free gr->out that points to gr->data: but Valgrind should tell me if I free memory twice... and it seems to like my code [so far]...).

I think you can remove the gif_result : stbi__gifinheritance and just call STBI_FREE(s.out) after the while loop.

If s is the stbi__context, in my version of stb_image, it has a lot of buffers, but not s.out:

typedef struct
{
   stbi__uint32 img_x, img_y;
   int img_n, img_out_n;

   stbi_io_callbacks io;
   void *io_user_data;

   int read_from_callbacks;
   int buflen;
   stbi_uc buffer_start[128];

   stbi_uc *img_buffer, *img_buffer_end;
   stbi_uc *img_buffer_original, *img_buffer_original_end;
} stbi__context;

So I don't know which one to try...

@Flix01

This comment has been minimized.

Copy link

@Flix01 Flix01 commented Feb 16, 2016

I'm sorry: on a closer look the memory leaks seem to be still present even with my (horrible and incorrect) 'fix'.
So I don't have any valid solution yet.

@urraka

This comment has been minimized.

Copy link
Owner Author

@urraka urraka commented Feb 16, 2016

Sorry, STBI_FREE(s.out) is wrong. Should be STBI_FREE(g.out) instead. I'll take another look to see if I find why it's still leaking. Maybe I'll do some actual testing too instead of just trying to guess what's wrong.

@urraka

This comment has been minimized.

Copy link
Owner Author

@urraka urraka commented Feb 16, 2016

I tested with some sample gifs. Adding STBI_FREE(g.out) after the while loop seems to fix the leaks here. Make sure you call stbi_image_free on the result of stbi_xload at some point too.

@Flix01

This comment has been minimized.

Copy link

@Flix01 Flix01 commented Feb 17, 2016

I tested with some sample gifs. Adding STBI_FREE(g.out) after the while loop seems to fix the leaks here.

Yes! This worked for me too (tested with 2 different gifs: no more leaks!)

Thanks.
P.S. You should modify your code to reflect the fix.
In any case, for clarity, from Revision 1 of this gist, users should just add STBI_FREE(g.out); after the loop (before: if (gr != &head)).

@urraka

This comment has been minimized.

Copy link
Owner Author

@urraka urraka commented Feb 17, 2016

Cool. It's updated now.

@Gtoyos

This comment has been minimized.

Copy link

@Gtoyos Gtoyos commented Jan 30, 2020

This code is outdated. Multiple functions of stbi_image have been changed so this code doesn't work anymore:

In line 44: stbi__gif_load_next(&s, &g, &c, 4)
New definition: static stbi_uc *stbi__gif_load_next(stbi__context *s, stbi__gif *g, int *comp, int req_comp, stbi_uc *two_back)

In line 98: result = stbi__load_main(&s, x, y, frames, 4);
New definition: static void *stbi__load_main(stbi__context *s, int *x, int *y, int *comp, int req_comp, stbi__result_info *ri, int bpc)

Is there any way to make it work with the current functions? What has changed since 2016? There is still a link to this piece of code as a GIF loader solution.

@urraka

This comment has been minimized.

Copy link
Owner Author

@urraka urraka commented Jan 30, 2020

Hey @Gtoyos, I suggest that you look into stbi_load_gif_from_memory(stbi_uc const *buffer, int len, int **delays, int *x, int *y, int *z, int *comp, int req_comp) (https://github.com/nothings/stb/blob/f67165c2bb2af3060ecae7d20d6f731173485ad0/stb_image.h#L1300). It looks like it's been added somewhere along the way and it returns a buffer with all frames and delays in a separate argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment