[FFmpeg-devel] [PATCH 1/1] avfilter/buffersink: Add video frame allocation callback

[John Cox]

Hi

On Sat, 22 Jul 2023 17:12:51 +0200, you wrote:

>John Cox (12023-07-20):
>> Add a callback to enable user allocation of video frames on the final
>> stage of a filter chain.
>> 
>> Signed-off-by: John Cox <jc at kynesim.co.uk>
>> ---
>>  libavfilter/buffersink.c | 21 +++++++++++++++++++++
>>  libavfilter/buffersink.h | 27 +++++++++++++++++++++++++++
>>  libavfilter/version.h    |  2 +-
>>  3 files changed, 49 insertions(+), 1 deletion(-)
>
>Hi.
>
>Thanks for the patch.
Thanks for the prompt attention

>> 
>> diff --git a/libavfilter/buffersink.c b/libavfilter/buffersink.c
>> index 306c283f77..070b743186 100644
>> --- a/libavfilter/buffersink.c
>> +++ b/libavfilter/buffersink.c
>> @@ -62,6 +62,11 @@ typedef struct BufferSinkContext {
>>      int sample_rates_size;
>>  
>>      AVFrame *peeked_frame;
>> +
>> +    union {
>> +        av_buffersink_alloc_video_frame * video;
>> +    } alloc_cb;
>
>> +    void * alloc_v;
>
>Here and everywhere: our coding style puts a space before the * but not
>after.
Will fix

>>  } BufferSinkContext;
>>  
>>  #define NB_ITEMS(list) (list ## _size / sizeof(*list))
>> @@ -154,6 +159,21 @@ int attribute_align_arg av_buffersink_get_samples(AVFilterContext *ctx,
>>      return get_frame_internal(ctx, frame, 0, nb_samples);
>>  }
>>  
>> +static AVFrame * alloc_video_buffer(AVFilterLink *link, int w, int h)
>> +{
>> +    AVFilterContext * const ctx = link->dst;
>
>> +    BufferSinkContext * const bs = ctx->priv;
>
>Nit: it is called buf in the rest of this file.
Will fix

>> +    return bs->alloc_cb.video ? bs->alloc_cb.video(ctx, bs->alloc_v, w, h) :
>> +        ff_default_get_video_buffer(link, w, h);
>
>Nit: align ff_ with bs->.
Will fix

>> +}
>> +
>> +void av_buffersink_set_alloc_video_frame(AVFilterContext *ctx, av_buffersink_alloc_video_frame * cb, void * v)
>> +{
>> +    BufferSinkContext * const bs = ctx->priv;
>> +    bs->alloc_cb.video = cb;
>> +    bs->alloc_v = v;
>> +}
>> +
>>  static av_cold int common_init(AVFilterContext *ctx)
>>  {
>>      BufferSinkContext *buf = ctx->priv;
>> @@ -381,6 +401,7 @@ static const AVFilterPad avfilter_vsink_buffer_inputs[] = {
>>      {
>>          .name = "default",
>>          .type = AVMEDIA_TYPE_VIDEO,
>> +        .get_buffer = {.video = alloc_video_buffer},
>>      },
>>  };
>>  
>> diff --git a/libavfilter/buffersink.h b/libavfilter/buffersink.h
>> index 64e08de53e..73f0ddc476 100644
>> --- a/libavfilter/buffersink.h
>> +++ b/libavfilter/buffersink.h
>> @@ -166,6 +166,33 @@ int av_buffersink_get_frame(AVFilterContext *ctx, AVFrame *frame);
>>   */
>>  int av_buffersink_get_samples(AVFilterContext *ctx, AVFrame *frame, int nb_samples);
>>  
>> +/**
>> + * Callback from av_buffersink_set_alloc_video_frame to allocate 
>> + * a frame 
>> + *  
>> + * @param ctx pointer to a context of the abuffersink AVFilter.
>> + * @param v opaque pointer passed to 
>> + *          av_buffersink_set_alloc_video_frame
>> + * @param w width of frame to allocate 
>> + * @param height of frame to allocate 
>> + *  
>> + * @return 
>> + *         - The newly allocated frame
>> + *         - NULL if error
>> + */
>
>> +typedef AVFrame * av_buffersink_alloc_video_frame(AVFilterContext * ctx, void * v, int w, int h);
>
>In the rest of the API, function typedefs are always pointer-to-function
>typedefs.
Fair enough - I've found it convient in the past to be able to use the
function typedef for prototypes of the functions too.

>Also, I find this typedef looks too much like actually a function, maybe
>use CamelCase.
Will do

>> +
>> +/**
>> + * Set a video frame allocation method for buffersink
>> + *
>> + * @param ctx pointer to a context of the abuffersink AVFilter.
>> + * @param cb Callback to the allocation function. If set to NULL 
>> + *           then the default avfilter allocation function will
>> + *           be used.
>> + * @param v  Opaque to pass to the allocation function
>> + */
>> +void av_buffersink_set_alloc_video_frame(AVFilterContext *ctx, av_buffersink_alloc_video_frame * cb, void * v);
>> +
>>  /**
>>   * @}
>>   */
>> diff --git a/libavfilter/version.h b/libavfilter/version.h
>> index c001693e3c..54950497be 100644
>> --- a/libavfilter/version.h
>> +++ b/libavfilter/version.h
>> @@ -31,7 +31,7 @@
>>  
>>  #include "version_major.h"
>>  
>> -#define LIBAVFILTER_VERSION_MINOR   8
>> +#define LIBAVFILTER_VERSION_MINOR   9
>>  #define LIBAVFILTER_VERSION_MICRO 102
>
>A test program would be nice too. I assume you have something to test
>your code: would it be a lot of work to make it public?

Actually my only test code is Kodi (xbmc) on Pi which is already using
it. The patch is simple enough that I didn't feel the need for a
separate test. I can put something together if required though.

Regards

JC