syoyo/tinyexr

[TODO] Memory limit feature

syoyo opened this issue · 4 comments

syoyo commented

Describe the issue

Currently TinyEXR does not support capping memory consumption.
Without such feature, the app may trigger OOM for large EXR images.

To Reproduce

Run poc2 and poc2 in issue 197.

Expected behavior

Add feature to specify the maximum memory to use. This requires

  • Define new API(current API does not support providing any loader options)
  • Set the maximum memory in define macro, e.g.
    • `#define TINYEXR_MAX_MAMORY (102410241024*64)

It also require implementing tracking memory consumption in TinyEXR .

Environment

N/A

Hi, I'm currently looking into adding custom allocator support to TinyEXR. I started by simply replacing malloc/free with TINYEXR_MALLOC / TINYEXR_FREE macros that I can define within the cpp implementation file, to route allocations to a custom global allocator. This is a rather unintrusive change, easy to whip up a small PR for.
However, the STB libraries implement allocator overrides with a user-supplied context void *, which allows to use a different allocator depending on use case / thread index / etc. That would be my preferred way to handle it in TinyEXR. Doing that would mean passing down the pointer to all functions that allocate memory. Would you agree that it would be worthwhile? Maybe this is something you thought about doing anyway for this particular issue?
Since I don't want to maintain my own fork, I'd rather not do this work without a chance that it can be contributed back.
Looking forward to hearing your thoughts, and thanks for your work on the library!

👍 > I'm currently looking into adding custom allocator support to TinyEXR

the STB libraries implement allocator overrides with a user-supplied context void *

Where can I find such allocator override in STB project?
I have glimpsed stb_image.h but allocators used there is macro-based approach(STB_MALLOC, STB_FREE, ...).

https://github.com/nothings/stb/blob/master/stb_image.h

Anyway, yes, having userdata pointer is the preferred approach.
So we'd be better to introduce callback functions for memory allocations, something like...

struct EXRMemoryAllocatorCallback
{
  void* (*malloc)(size_t n, void *userdata);
  void* (*realloc)(void *p, size_t newn, void *userdata);
  void (*free)(void *ptr, void *userdata);
};

(FYI stb_image uses this approach in io callback https://github.com/nothings/stb/blob/ae721c50eaf761660b4f90cc590453cdb0c2acd0/stb_image.h#L410 )

We'll then need to introduce new API which takes this callback in its argument

bool load_exr(..., EXRMemoryAllocatorCallback *cb, void *userdata);
void exr_free_image(..., EXRMemoryAllocatorCallback *cb, void *userdata);

For backward compatibility, existing API uses default allocator.

Where can I find such allocator override in STB project? I have glimpsed stb_image.h but allocators used there is macro-based approach(STB_MALLOC, STB_FREE, ...).

You're right, it's not consistent in all the STB libs. I saw this in stb_image_resize2.h:
https://github.com/nothings/stb/blob/ae721c50eaf761660b4f90cc590453cdb0c2acd0/stb_image_resize2.h#L69

So we'd be better to introduce callback functions for memory allocations, something like...

That's definitely the cleanest approach! realloc isn't used in TinyEXR though, correct? I'd be careful about adding it as a required part of the callback since some custom allocators don't support it, and in my experience there are subtleties to how realloc is expected to behave in various cases. But it could be optional, ie. if it's not supplied emulate it using malloc -> memcpy -> free. As long as you don't see a need for it though, maybe just stick to malloc and free?

Oh, Thanks! > I saw this in stb_image_resize2.h

realloc isn't used in TinyEXR though, correct?

Yes, realloc is not used in TinyEXR, so we can omit it. just malloc and free!