| Message ID | 20230720113955.957942-2-jc@kynesim.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | avfilter/buffersink: Add user video frame allocation | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
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@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. > > 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. > } 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. > + 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->. > +} > + > +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. Also, I find this typedef looks too much like actually a function, maybe use CamelCase. > + > +/** > + * 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? Regards,
Paul B Mahol (12023-07-22): > What about an audio? Are we in the business of refusing a patch adding an interesting feature because we want two interesting features instead? > This works only for sinks mostly, what about filters inside graph? It is already in place, it has been for years. This patch only adds the API to expose it publicly.
On Thu, Jul 20, 2023 at 1:40 PM John Cox <jc@kynesim.co.uk> wrote: > Add a callback to enable user allocation of video frames on the final > stage of a filter chain. > > What about an audio? This works only for sinks mostly, what about filters inside graph? > Signed-off-by: John Cox <jc@kynesim.co.uk> > --- > libavfilter/buffersink.c | 21 +++++++++++++++++++++ > libavfilter/buffersink.h | 27 +++++++++++++++++++++++++++ > libavfilter/version.h | 2 +- > 3 files changed, 49 insertions(+), 1 deletion(-) > > 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; > } 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; > + return bs->alloc_cb.video ? bs->alloc_cb.video(ctx, bs->alloc_v, w, > h) : > + ff_default_get_video_buffer(link, w, h); > +} > + > +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); > + > +/** > + * 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 > > > -- > 2.39.2 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
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@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
On Sat, 22 Jul 2023 17:28:31 +0200, you wrote: >On Thu, Jul 20, 2023 at 1:40?PM John Cox <jc@kynesim.co.uk> wrote: > >> Add a callback to enable user allocation of video frames on the final >> stage of a filter chain. >> >> >What about an audio? I have a use case for video and none for audio, but I can add audio if you want. >This works only for sinks mostly, what about filters inside graph? Buffer allocation can already be set by the next filter in the chain which is the callback I am tapping into. If you want to change that then I guess the right answer would be to move get_buffer.video & audio out of AVFilterPad and into somewhere publicly writeable, but I really don't see the need. It seems sensible to me that buffer allocation internal to graphs is negotiated between those filter elements and is opaque to the user and it is only at the point where a buffer is delivered to the user that the user needs to be able to intervene. Regards JC >> Signed-off-by: John Cox <jc@kynesim.co.uk> >> --- >> libavfilter/buffersink.c | 21 +++++++++++++++++++++ >> libavfilter/buffersink.h | 27 +++++++++++++++++++++++++++ >> libavfilter/version.h | 2 +- >> 3 files changed, 49 insertions(+), 1 deletion(-) >> >> 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; >> } 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; >> + return bs->alloc_cb.video ? bs->alloc_cb.video(ctx, bs->alloc_v, w, >> h) : >> + ff_default_get_video_buffer(link, w, h); >> +} >> + >> +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); >> + >> +/** >> + * 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 >> >> >> -- >> 2.39.2 >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >>
On Sat, Jul 22, 2023 at 5:25 PM Nicolas George <george@nsup.org> wrote: > Paul B Mahol (12023-07-22): > > What about an audio? > > Are we in the business of refusing a patch adding an interesting feature > because we want two interesting features instead? > Double standards, not unexpected from you. > > > This works only for sinks mostly, what about filters inside graph? > > It is already in place, it has been for years. This patch only adds the > API to expose it publicly. > > -- > Nicolas George > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
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; } 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; + return bs->alloc_cb.video ? bs->alloc_cb.video(ctx, bs->alloc_v, w, h) : + ff_default_get_video_buffer(link, w, h); +} + +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); + +/** + * 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
Add a callback to enable user allocation of video frames on the final stage of a filter chain. Signed-off-by: John Cox <jc@kynesim.co.uk> --- libavfilter/buffersink.c | 21 +++++++++++++++++++++ libavfilter/buffersink.h | 27 +++++++++++++++++++++++++++ libavfilter/version.h | 2 +- 3 files changed, 49 insertions(+), 1 deletion(-)