| Message ID | 20230722170357.964313-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: > 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(-) > > diff --git a/libavfilter/buffersink.c b/libavfilter/buffersink.c > index 306c283f77..e99d444530 100644 > --- a/libavfilter/buffersink.c > +++ b/libavfilter/buffersink.c > @@ -62,6 +62,11 @@ typedef struct BufferSinkContext { > int sample_rates_size; > > AVFrame *peeked_frame; > + > + union { > + AVBuffersinkAllocVideoFrameFunc 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 buf = ctx->priv; > + return buf->alloc_cb.video ? buf->alloc_cb.video(ctx, buf->alloc_v, w, h) : > + ff_default_get_video_buffer(link, w, h); > +} > + > +void av_buffersink_set_alloc_video_frame(AVFilterContext *ctx, AVBuffersinkAllocVideoFrameFunc cb, void *v) > +{ > + BufferSinkContext *const buf = ctx->priv; > + buf->alloc_cb.video = cb; > + buf->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..2419d1bd80 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 *(*AVBuffersinkAllocVideoFrameFunc)(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, AVBuffersinkAllocVideoFrameFunc 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 Not commenting on the actual patch, but when bumping minor, you need to reset micro. - Andreas
On Sat, 22 Jul 2023 21:14:24 +0200, you wrote: >John Cox: >> 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(-) >> >> diff --git a/libavfilter/buffersink.c b/libavfilter/buffersink.c >> index 306c283f77..e99d444530 100644 >> --- a/libavfilter/buffersink.c >> +++ b/libavfilter/buffersink.c >> @@ -62,6 +62,11 @@ typedef struct BufferSinkContext { >> int sample_rates_size; >> >> AVFrame *peeked_frame; >> + >> + union { >> + AVBuffersinkAllocVideoFrameFunc 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 buf = ctx->priv; >> + return buf->alloc_cb.video ? buf->alloc_cb.video(ctx, buf->alloc_v, w, h) : >> + ff_default_get_video_buffer(link, w, h); >> +} >> + >> +void av_buffersink_set_alloc_video_frame(AVFilterContext *ctx, AVBuffersinkAllocVideoFrameFunc cb, void *v) >> +{ >> + BufferSinkContext *const buf = ctx->priv; >> + buf->alloc_cb.video = cb; >> + buf->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..2419d1bd80 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 *(*AVBuffersinkAllocVideoFrameFunc)(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, AVBuffersinkAllocVideoFrameFunc 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 > >Not commenting on the actual patch, but when bumping minor, you need to >reset micro. To zero? or one? (I'm guessing 0) Ta JC
John Cox: > On Sat, 22 Jul 2023 21:14:24 +0200, you wrote: > >> John Cox: >>> 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(-) >>> >>> diff --git a/libavfilter/buffersink.c b/libavfilter/buffersink.c >>> index 306c283f77..e99d444530 100644 >>> --- a/libavfilter/buffersink.c >>> +++ b/libavfilter/buffersink.c >>> @@ -62,6 +62,11 @@ typedef struct BufferSinkContext { >>> int sample_rates_size; >>> >>> AVFrame *peeked_frame; >>> + >>> + union { >>> + AVBuffersinkAllocVideoFrameFunc 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 buf = ctx->priv; >>> + return buf->alloc_cb.video ? buf->alloc_cb.video(ctx, buf->alloc_v, w, h) : >>> + ff_default_get_video_buffer(link, w, h); >>> +} >>> + >>> +void av_buffersink_set_alloc_video_frame(AVFilterContext *ctx, AVBuffersinkAllocVideoFrameFunc cb, void *v) >>> +{ >>> + BufferSinkContext *const buf = ctx->priv; >>> + buf->alloc_cb.video = cb; >>> + buf->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..2419d1bd80 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 *(*AVBuffersinkAllocVideoFrameFunc)(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, AVBuffersinkAllocVideoFrameFunc 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 >> >> Not commenting on the actual patch, but when bumping minor, you need to >> reset micro. > > To zero? or one? (I'm guessing 0) > 100 - Andreas
On 7/22/2023 2:03 PM, John Cox wrote: > 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(-) > > diff --git a/libavfilter/buffersink.c b/libavfilter/buffersink.c > index 306c283f77..e99d444530 100644 > --- a/libavfilter/buffersink.c > +++ b/libavfilter/buffersink.c > @@ -62,6 +62,11 @@ typedef struct BufferSinkContext { > int sample_rates_size; > > AVFrame *peeked_frame; > + > + union { > + AVBuffersinkAllocVideoFrameFunc 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 buf = ctx->priv; > + return buf->alloc_cb.video ? buf->alloc_cb.video(ctx, buf->alloc_v, w, h) : > + ff_default_get_video_buffer(link, w, h); Does the AVBuffersinkAllocVideoFrameFunc function have access to the AVFilterLink ff_default_get_video_buffer() gets? I assume it'd be needed to get values like pixel format. If so, it should be documented how, but maybe it's easier to just pass link here instead of the AVFilterContext, and let the caller access link->dst if needed. Also, looking at ff_default_get_video_buffer(), a hardware enabled filter may populate hw_frames_ctx, and that's a field only available in the AVFilterLink and that should not be accessed from outside lavfi, which includes a caller defined function used for this callback. > +} > + > +void av_buffersink_set_alloc_video_frame(AVFilterContext *ctx, AVBuffersinkAllocVideoFrameFunc cb, void *v) > +{ > + BufferSinkContext *const buf = ctx->priv; > + buf->alloc_cb.video = cb; > + buf->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..2419d1bd80 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 *(*AVBuffersinkAllocVideoFrameFunc)(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, AVBuffersinkAllocVideoFrameFunc 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 > >
Hi >On 7/22/2023 2:03 PM, John Cox wrote: >> 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(-) >> >> diff --git a/libavfilter/buffersink.c b/libavfilter/buffersink.c >> index 306c283f77..e99d444530 100644 >> --- a/libavfilter/buffersink.c >> +++ b/libavfilter/buffersink.c >> @@ -62,6 +62,11 @@ typedef struct BufferSinkContext { >> int sample_rates_size; >> >> AVFrame *peeked_frame; >> + >> + union { >> + AVBuffersinkAllocVideoFrameFunc 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 buf = ctx->priv; >> + return buf->alloc_cb.video ? buf->alloc_cb.video(ctx, buf->alloc_v, w, h) : >> + ff_default_get_video_buffer(link, w, h); > >Does the AVBuffersinkAllocVideoFrameFunc function have access to the >AVFilterLink ff_default_get_video_buffer() gets? I assume it'd be needed >to get values like pixel format. If so, it should be documented how, but >maybe it's easier to just pass link here instead of the AVFilterContext, >and let the caller access link->dst if needed. Unless I'm misreading stuff you can get at AVFilterLink from from AVFilterContext so I think AVFilterContext is good. If you are suggesting that the documentation of AVFilterContext & Link could be improved I feel that is outside the scope of this patch. >Also, looking at ff_default_get_video_buffer(), a hardware enabled >filter may populate hw_frames_ctx, and that's a field only available in >the AVFilterLink and that should not be accessed from outside lavfi, >which includes a caller defined function used for this callback. I'm afraid I don't quite understand what you are suggesting here. What would you have me do differently? Regards JC >> +} >> + >> +void av_buffersink_set_alloc_video_frame(AVFilterContext *ctx, AVBuffersinkAllocVideoFrameFunc cb, void *v) >> +{ >> + BufferSinkContext *const buf = ctx->priv; >> + buf->alloc_cb.video = cb; >> + buf->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..2419d1bd80 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 *(*AVBuffersinkAllocVideoFrameFunc)(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, AVBuffersinkAllocVideoFrameFunc 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 >> >> >_______________________________________________ >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".
James Almer (12023-07-23): > Does the AVBuffersinkAllocVideoFrameFunc function have access to the > AVFilterLink ff_default_get_video_buffer() gets? I assume it'd be needed to > get values like pixel format. If so, it should be documented how, but maybe > it's easier to just pass link here instead of the AVFilterContext, and let > the caller access link->dst if needed. The API of buffersink already exposes all the necessary format information. Regards,
On 7/23/2023 4:06 PM, Nicolas George wrote: > James Almer (12023-07-23): >> Does the AVBuffersinkAllocVideoFrameFunc function have access to the >> AVFilterLink ff_default_get_video_buffer() gets? I assume it'd be needed to >> get values like pixel format. If so, it should be documented how, but maybe >> it's easier to just pass link here instead of the AVFilterContext, and let >> the caller access link->dst if needed. > > The API of buffersink already exposes all the necessary format > information. What about when FF_FILTER_FLAG_HWFRAME_AWARE filters are present in the graph? hw_frames_ctx from AVFilterLink can't be accessed from outside lavfi. Is vf_hwdownload meant to be added to the graph before buffersink?
James Almer (12023-07-23): > What about when FF_FILTER_FLAG_HWFRAME_AWARE filters are present in the > graph? hw_frames_ctx from AVFilterLink can't be accessed from outside lavfi. > Is vf_hwdownload meant to be added to the graph before buffersink? I do not know how hardware acceleration works at all. (The tidbits of discussion I catch left me the impression all of it is very badly designed, but I have low confidence in that impression.) If this API only works with filters that output software frames, it is already very useful. Regards,
Paul B Mahol (12023-07-23): > - missing audio support You realize that direct rendering is order of magnitude more useful for video frames than for audio, right? > - missing full internal buffers allocation replacement support Nonsense. > - missing/untested hardware acceleration support Direct rendering is especially useful for software, that is the point.
On Sun, Jul 23, 2023 at 9:26 PM Nicolas George <george@nsup.org> wrote: > James Almer (12023-07-23): > > What about when FF_FILTER_FLAG_HWFRAME_AWARE filters are present in the > > graph? hw_frames_ctx from AVFilterLink can't be accessed from outside > lavfi. > > Is vf_hwdownload meant to be added to the graph before buffersink? > > I do not know how hardware acceleration works at all. (The tidbits of > discussion I catch left me the impression all of it is very badly > designed, but I have low confidence in that impression.) If this API > only works with filters that output software frames, it is already very > useful. > Patch is only marginally useful: - missing audio support - missing full internal buffers allocation replacement support - missing/untested hardware acceleration support > Regards, > > -- > 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". >
On Sun, Jul 23, 2023 at 9:36 PM Nicolas George <george@nsup.org> wrote: > Paul B Mahol (12023-07-23): > > - missing audio support > > You realize that direct rendering is order of magnitude more useful for > video frames than for audio, right? > > > - missing full internal buffers allocation replacement support > > Nonsense. > Your comment is extremely rude and unhelpful. The patch at best only replaces last get_buffer call of buffersink. > > > - missing/untested hardware acceleration support > > Direct rendering is especially useful for software, that is the point. > > -- > 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". >
On Sun, Jul 23, 2023 at 9:36 PM Nicolas George <george@nsup.org> wrote: > Paul B Mahol (12023-07-23): > > - missing audio support > > You realize that direct rendering is order of magnitude more useful for > video frames than for audio, right? > That does not make patch more complete. > > > - missing full internal buffers allocation replacement support > > Nonsense. > > > - missing/untested hardware acceleration support > > Direct rendering is especially useful for software, that is the point. > That does not make other cases and this case suddenly resolved. > > -- > 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". >
On 7/23/2023 4:40 PM, Paul B Mahol wrote: > On Sun, Jul 23, 2023 at 9:26 PM Nicolas George <george@nsup.org> wrote: > >> James Almer (12023-07-23): >>> What about when FF_FILTER_FLAG_HWFRAME_AWARE filters are present in the >>> graph? hw_frames_ctx from AVFilterLink can't be accessed from outside >> lavfi. >>> Is vf_hwdownload meant to be added to the graph before buffersink? >> >> I do not know how hardware acceleration works at all. (The tidbits of >> discussion I catch left me the impression all of it is very badly >> designed, but I have low confidence in that impression.) If this API >> only works with filters that output software frames, it is already very >> useful. >> > > Patch is only marginally useful: > > - missing audio support Trivially added if needed and when needed. alloc_cb is a union that can get a new callback typedef field for audio. > - missing full internal buffers allocation replacement support What is the benefit of supporting a custom allocator for all filters in the chain? Internally, it's already using a very optimized buffer pool. The caller only cares about how what they get out of buffersink is allocated. > - missing/untested hardware acceleration support This however i agree about. We need to know how it will behave in this scenario. How does buffersink currently handle things when the previous filter in the chain propagates hardware frames? > > >> Regards, >> >> -- >> 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". >> > _______________________________________________ > 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".
Paul B Mahol (12023-07-23): > Your comment is extremely rude and unhelpful. I am sorry for any perceived rudeness. Shall I point each time I find your comments to me extremely rude and/or unhelpful too? > The patch at best only replaces last get_buffer call of buffersink. Yes, indeed, that is exactly what this patch does, and that is exactly why it is useful, anybody involved in the library as you are should be able to spot it in an instant.
On Mon, Jul 24, 2023 at 10:21 AM Nicolas George <george@nsup.org> wrote: > Paul B Mahol (12023-07-23): > > Your comment is extremely rude and unhelpful. > > I am sorry for any perceived rudeness. Shall I point each time I find > your comments to me extremely rude and/or unhelpful too? > > > The patch at best only replaces last get_buffer call of buffersink. > > Yes, indeed, that is exactly what this patch does, and that is exactly > why it is useful, anybody involved in the library as you are should be > able to spot it in an instant. > If you have nothing constructive to add I advise you to just leave conversation and even better this project, because you are lately just keeping being abusive and manipulative. > -- > 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". >
> What is the benefit of supporting a custom allocator for all filters in > the chain? Internally, it's already using a very optimized buffer pool. > The caller only cares about how what they get out of buffersink is > You may want to reuse a larger existing pool instead of FFmpeg have its own. Kieran
On 7/24/2023 6:07 AM, Paul B Mahol wrote: > On Mon, Jul 24, 2023 at 10:21 AM Nicolas George <george@nsup.org> wrote: > >> Paul B Mahol (12023-07-23): >>> Your comment is extremely rude and unhelpful. >> >> I am sorry for any perceived rudeness. Shall I point each time I find >> your comments to me extremely rude and/or unhelpful too? >> >>> The patch at best only replaces last get_buffer call of buffersink. >> >> Yes, indeed, that is exactly what this patch does, and that is exactly >> why it is useful, anybody involved in the library as you are should be >> able to spot it in an instant. >> > > If you have nothing constructive to add I advise you to just leave > conversation and even better this project, because you are lately just > keeping being abusive and manipulative. Please, drop it.
On Sun, 23 Jul 2023 17:04:55 -0300, you wrote: >On 7/23/2023 4:40 PM, Paul B Mahol wrote: >> On Sun, Jul 23, 2023 at 9:26?PM Nicolas George <george@nsup.org> wrote: >> >>> James Almer (12023-07-23): >>>> What about when FF_FILTER_FLAG_HWFRAME_AWARE filters are present in the >>>> graph? hw_frames_ctx from AVFilterLink can't be accessed from outside >>> lavfi. >>>> Is vf_hwdownload meant to be added to the graph before buffersink? >>> >>> I do not know how hardware acceleration works at all. (The tidbits of >>> discussion I catch left me the impression all of it is very badly >>> designed, but I have low confidence in that impression.) If this API >>> only works with filters that output software frames, it is already very >>> useful. >>> >> >> Patch is only marginally useful: >> >> - missing audio support > >Trivially added if needed and when needed. alloc_cb is a union that can >get a new callback typedef field for audio. Now added in v4 >> - missing full internal buffers allocation replacement support > >What is the benefit of supporting a custom allocator for all filters in >the chain? Internally, it's already using a very optimized buffer pool. >The caller only cares about how what they get out of buffersink is >allocated. A case was made that you might want to tweak the number of buffers in the pool, but I remain to be convinced that the substantial extra complexity of trying to do this is worth it. >> - missing/untested hardware acceleration support > >This however i agree about. We need to know how it will behave in this >scenario. How does buffersink currently handle things when the previous >filter in the chain propagates hardware frames? It just delivers them to the user The case that this code was written for and I suspect pretty much the only case it will be used for is where the user wants the frame in a special buffer that it can pass directly to display or other hardware and doesn't want the overhead of copying. Assuming that to be the case then there are two possibilities if the final filter stage produces hardware frames: a) The frames are in the right sort of h/w buffer in which case no custom allocator is wanted - code unused b) they need copying / conversion to the right sort of buffer - again custom allocation doesn't help So given that I could do the same as what avcodec ff_getbuffer does and if there is an existing h/w frame allocator then the user function isn't called. Would that be OK? If not then I'd like some constructive suggestions as to what exactly would be OK please. Regards JC >> >>> Regards, >>> >>> -- >>> 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". >>> >> _______________________________________________ >> 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". >_______________________________________________ >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..e99d444530 100644 --- a/libavfilter/buffersink.c +++ b/libavfilter/buffersink.c @@ -62,6 +62,11 @@ typedef struct BufferSinkContext { int sample_rates_size; AVFrame *peeked_frame; + + union { + AVBuffersinkAllocVideoFrameFunc 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 buf = ctx->priv; + return buf->alloc_cb.video ? buf->alloc_cb.video(ctx, buf->alloc_v, w, h) : + ff_default_get_video_buffer(link, w, h); +} + +void av_buffersink_set_alloc_video_frame(AVFilterContext *ctx, AVBuffersinkAllocVideoFrameFunc cb, void *v) +{ + BufferSinkContext *const buf = ctx->priv; + buf->alloc_cb.video = cb; + buf->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..2419d1bd80 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 *(*AVBuffersinkAllocVideoFrameFunc)(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, AVBuffersinkAllocVideoFrameFunc 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(-)