lvandeve / lodepng

PNG encoder and decoder in C and C++.
zlib License
2.07k stars 422 forks source link

Improve custom allocator support #9

Open golightlyb opened 9 years ago

golightlyb commented 9 years ago

Hi, thanks for LodePNG.

To make the custom allocator functionality (LODEPNG_NO_COMPILE_ALLOCATORS) useful to me, I need the lodepng_malloc/realloc/free functions to accept an extra void *user_arg argument.

I think this can be done without breaking the API by e.g.

unsigned lodepng_decode_memory_using(unsigned char** out, unsigned* w, unsigned* h,
                               const unsigned char* in, size_t insize,
                               LodePNGColorType colortype, unsigned bitdepth, void *allocator_arg)
{
    // normal implementation except using allocator_arg for lodepng_malloc, lodepng_free etc.
}

unsigned lodepng_decode_memory(unsigned char** out, unsigned* w, unsigned* h,
                               const unsigned char* in, size_t insize,
                               LodePNGColorType colortype, unsigned bitdepth)
{
    // wrapper that behaves like the default
    return lodepng_decode_memory_using(out, w, h, in, size, colortype, bitdepth, NULL);
}

#ifdef LODEPNG_COMPILE_ALLOCATORS
static void* lodepng_malloc(size_t size, void *user_arg)
{
    UNUSED(user_arg);
    return malloc(size);
}

However this would break the API for anyone already using custom allocators.

Would such a change, in principle, be accepted by you? If so I'm prepared to make the changes for you and send you a pull request. Many thanks!

lvandeve commented 9 years ago

Hi,

I'll do it in an update soon. I've indeed seen similar allocators with void argument used elsewhere.

Out of curiosity, what do you need it for?

Thanks

lvandeve commented 9 years ago

It does require a lot of changes to pass the user object through all code paths.

If lodepng is called only once at the time, maybe the object can be made global instead so your custom function can get it from there? Would that work for now?

ghost commented 8 years ago

@golightlyb what is that void *user_arg good for? For what do you use it? I'm just curious.

AntTheAlchemist commented 8 years ago

Be careful not to add too much bulk to LodePNG. It's beauty is that it's interface is simple and compact. If we understand what the user_arg is for, it will help decide?

I'm using LODEPNG_NO_COMPILE_ALLOCATORS to link into my garbage collection logic that overrides new & delete in C++. But the problem for me is new & delete can't realloc, so I've had to add padding to manage realloc requests. Is there any chance to disable realloc requests?

lvandeve commented 8 years ago

It'd probably be through the state object, not an extra function argument, if I do it. Not sure when though.

But actually, indeed maybe it's not needed at all? The required state for allocations can be stored in the memory you manage. Is that right?

AntTheAlchemist commented 8 years ago

Hi Lode (nice to meet you).

I'm not sure I understand your question about state objects.

I override new and delete to manage memory. When new fails to allocate, I call a special method that deletes memory (in my case, cached resources such as textures) until the allocation is successful. This guarantees that new will never fail, so I don't need to check every time I make a new object.

So, I use LODEPNG_NO_COMPILE_ALLOCATORS to use my custom new & delete operators so that lodepng never fails to allocate. Works very well. But to make lodepng_realloc() work, I have to store a size_t for every lodepng_malloc(), so I know how much memory to memcpy() into the new_size. It works, but it's a little messy.

This is how I'm storing the size_t:

void * lodepng_malloc(size_t size) { char *n = new char[size + sizeof(size_t)]; *(size_t*)n = size; n += sizeof(size_t); return n; }

No big problem, but without the need for realloc, this would be a lot cleaner for anyone trying to do what I'm doing in C++.

lvandeve commented 8 years ago

realloc is quite convenient :)

What is the reason for not being able to use it?

AFAIK realloc is often able to run in constant time because if memory behind it was free anyway it can immediately use it. Do your allocators take this into account?

Do you use any C++ features like std::strings or std::vectors that also need to allocate more memory sometimes? How do you use those?

Thanks!

AntTheAlchemist commented 8 years ago

What if malloc() or realloc() fail because there is no free memory? I have no way to catch the error and free memory for it to try again.

I could, instead of using 'new', use malloc() and realloc() and call my garbage collection if they fail, then retry, but these allocations are outside of my control if I am not using new & delete operators, which have no way to realloc, which is why in lodepng_realloc() I use new char[new_size], memcpy(), delete old ptr;

I'm not using std::strings or std::vectors. Will the memory they dynamically allocate get caught in my overridden 'new' operator, so I can implement garbage collection on them if they fail due to out of memory?

lvandeve commented 8 years ago

LodePNG will return an error number 83 in case any allocation fails, if that helps :)

LodePNG only allocates primitive types, so C's malloc and free should do.

AntTheAlchemist commented 8 years ago

Actually, that's a good idea. If LodePNG returns 83, I can release some memory and retry. That's even simpler than before, lol.

Okay, forget about dropping realloc. All is ok :)

AntTheAlchemist commented 8 years ago

Just a quick suggestion. Any plans to make constants or #defines for the error codes?