Martijn Otto (12021-03-05): > Hello all, > > I have made some changes to get custom allocation functions in ffmpeg. > This is useful to me, as the software I work with easily runs into > congestion on memory allocations in certain cases. > > I hope it is useful to others as well. It would be nice if this could > be part of ffmpeg proper. If you have specific issues with my > implementation, please let me know.
Thanks for the contribution. It may be useful, but it cannot be accepted
as is for several reasons, see below.
> From 802b4aecb77c8a35eb6641aa8dd6d27bbcda1fe2 Mon Sep 17 00:00:00 2001
> From: Martijn Otto <ffm...@martijnotto.nl>
> Date: Thu, 4 Mar 2021 17:30:15 +0100
> Subject: [PATCH] Add custom memory allocation routines.
>
> This feature is a useful optimization strategy. It allows the
> implementation of working with a memory pool for better performance.
> ---
> libavformat/hlsenc.c | 1 +
> libavutil/buffer.c | 41 +++++++++++++++++++++++++++++++++++------
> libavutil/buffer.h | 23 +++++++++++++++++++++++
> 3 files changed, 59 insertions(+), 6 deletions(-)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 7d97ce1789..c8e4281e7b 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -927,6 +927,7 @@ static int hls_mux_init(AVFormatContext *s, VariantStream
> *vs)
> } else {
> ret = hlsenc_io_open(s, &vs->out, vs->base_output_dirname,
> &options);
> }
> + avio_flush(oc->pb);
This is an unrelated change, it needs to be submitted separately.
> av_dict_free(&options);
> }
> if (ret < 0) {
> diff --git a/libavutil/buffer.c b/libavutil/buffer.c
> index 3204f11b68..bd86b38524 100644
> --- a/libavutil/buffer.c
> +++ b/libavutil/buffer.c
> @@ -59,23 +59,52 @@ AVBufferRef *av_buffer_create(uint8_t *data, int size,
> return ref;
> }
>
> +PFNBufferAlloc externalAllocFunc = NULL;
> +PFNBufferDealloc externalDeallocFunc = NULL;
We do not use camelCase for identifiers.
These variables are local to the file and should be declared as such.
More importantly: we are trying to get rid of mutable global state. This
adds to it.
See this:
https://ffmpeg.org/pipermail/ffmpeg-devel/2020-December/274168.html
for my attempt at making global state local.
> +
> +void av_set_buffer_alloc_free_funcs( PFNBufferAlloc externalAlloc,
> PFNBufferDealloc externalDealloc )
> +{
> + externalAllocFunc = externalAlloc;
> + externalDeallocFunc = externalDealloc;
> +}
> +
> void av_buffer_default_free(void *opaque, uint8_t *data)
> {
> av_free(data);
> }
> +void av_buffer_external_free(void *opaque, uint8_t *data)
Missing new line.
This function is not declared. Does it even compile?
If it is not exported, it should be static and not in the av_ namespace.
> +{
> + if (externalDeallocFunc != NULL)
> + externalDeallocFunc(data);
> +}
>
> AVBufferRef *av_buffer_alloc(int size)
> {
> AVBufferRef *ret = NULL;
> uint8_t *data = NULL;
>
> - data = av_malloc(size);
> - if (!data)
> - return NULL;
> + //Give priority to the external allocation function to give the
> application a chance to manage it's own buffer allocations.
> + if (externalAllocFunc != NULL)
> + data = externalAllocFunc(size);
>
> - ret = av_buffer_create(data, size, av_buffer_default_free, NULL, 0);
> - if (!ret)
> - av_freep(&data);
> + if (data) {
> + //Create a buffer and tell it to free it's data using the external
> free function. We've used the external
> + //allocator for allocation, so we need to use external deallocator
> for deallocation.
> + ret = av_buffer_create(data, size, av_buffer_external_free, NULL, 0);
> + if (!ret)
> + av_buffer_external_free(NULL, data);
> + } else {
> + //The external allocation function may return NULL for other reasons
> than out of memory, so
> + //if it did we will fall back to our own allocation function.
Random fallbacks are wrong. The allocation function failed, period.
> + data = av_malloc(size);
> + if (!data)
> + return NULL; //We're out of memory after all.
> +
> + //We've created the buffer data ourselves so we can use our own free
> function.
> + ret = av_buffer_create(data, size, av_buffer_default_free, NULL, 0);
> + if (!ret)
> + av_freep(&data);
> + }
>
> return ret;
> }
> diff --git a/libavutil/buffer.h b/libavutil/buffer.h
> index fd4e381efa..3efe585d56 100644
> --- a/libavutil/buffer.h
> +++ b/libavutil/buffer.h
> @@ -93,6 +93,29 @@ typedef struct AVBufferRef {
> int size;
> } AVBufferRef;
>
> +#if defined(_WIN32)
> + #define FFAPI __stdcall
> +#else
> + #define FFAPI
> +#endif
We never needed this. Why change now?
> +
> +typedef void* (FFAPI *PFNBufferAlloc)( int size );
> +typedef void (FFAPI *PFNBufferDealloc)( void* buffer );
Public types should be in the AV namespace. We do not declare function
types usually anyway.
> +/**
> + * Set allocation functions that can be used to externally manage buffer
> allocations.
> + * During regular playback buffers are continuously being allocated and
> deallocated. In high performance
> + * applications this becomes a problem. When multiple files are playing at
> the same time on different threads
> + * these allocations interlock with eachother causing performance loss due
> to reduced paralellism.
> + * To remedy this these applications may set these allocation/deallocation
> functions which it can use to prevent
> + * this behaviour. It could for example implement a pool allocator from
> which it will source the buffers.
Please wrap at <80 characters.
Also, if you want to solve the problem with parallelism, you will need
to pass an extra parameter to the function.
> + *
> + * @param externalAlloc The function that will be called when a new buffer
> is required. This function can return
> + * NULL if it does not take care of allocating
> buffers of the provided size. In this case FFMPeg will
> + * fall back to it's own allocation function.
> + * @param externalDealloc The function that will be called when a buffer is
> to be deallocated.
> + */
> +void av_set_buffer_alloc_free_funcs( PFNBufferAlloc externalAlloc,
> PFNBufferDealloc externalDealloc );
Unusual spacing.
> +
> /**
> * Allocate an AVBuffer of the given size using av_malloc().
> *
Regards,
--
Nicolas George
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
