[FFmpeg-devel] [PATCH 2/7] lavfi/buffersink: add a flag for retrieving stream parameters

[Nicolas George]

Anton Khirnov (12024-09-23):
> This way, av_buffersink_get_frame_flags() can replace almost all
> av_buffersink_get_*(), including some future parameters we will want to
> export.
> ---
>  doc/APIchanges           |  1 +
>  libavfilter/buffersink.c | 47 ++++++++++++++++++++++++++++++++++++++++
>  libavfilter/buffersink.h | 16 ++++++++++++--
>  3 files changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index b392c756d7..5ddd7189f8 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -4,6 +4,7 @@ API changes, most recent first:
>  
>  2024-09-xx - xxxxxxxxxx - lavfi 10.4.100
>    Buffersink now sets AVFrame.time_base on the frames it outputs.
> +  Add AV_BUFFERSINK_FLAG_PARAMS.
>  
>  2024-09-23 - xxxxxxxxxx - lavc 61.18.100 - avcodec.h
>    Add a new flag AV_CODEC_EXPORT_DATA_ENHANCEMENTS for export_side_data.
> diff --git a/libavfilter/buffersink.c b/libavfilter/buffersink.c
> index 575075ff47..d9cd074f17 100644
> --- a/libavfilter/buffersink.c
> +++ b/libavfilter/buffersink.c
> @@ -128,8 +128,55 @@ static int get_frame_internal(AVFilterContext *ctx, AVFrame *frame, int flags, i
>      }
>  }
>  
> +static int get_frame_params(AVFilterContext *ctx, AVFrame *frame)
> +{
> +    FilterLink *l = ff_filter_link(ctx->inputs[0]);
> +    int ret = 0;
> +
> +    frame->time_base = l->pub.time_base;
> +    frame->format    = l->pub.format;
> +
> +    switch (l->pub.type) {
> +    case AVMEDIA_TYPE_AUDIO:
> +        ret = av_channel_layout_copy(&frame->ch_layout, &l->pub.ch_layout);
> +        if (ret < 0)
> +            goto fail;
> +
> +        frame->sample_rate          = l->pub.sample_rate;
> +        break;
> +    case AVMEDIA_TYPE_VIDEO:
> +        frame->width                = l->pub.w;
> +        frame->height               = l->pub.h;
> +        frame->sample_aspect_ratio  = l->pub.sample_aspect_ratio;
> +        frame->colorspace           = l->pub.colorspace;
> +        frame->color_range          = l->pub.color_range;
> +        break;
> +    default: av_assert0(0);
> +    }
> +
> +    if (l->hw_frames_ctx) {
> +        frame->hw_frames_ctx = av_buffer_ref(l->hw_frames_ctx);
> +        if (!frame->hw_frames_ctx) {
> +            ret = AVERROR(ENOMEM);
> +            goto fail;
> +        }
> +    }
> +
> +    return 0;
> +fail:
> +    av_frame_unref(frame);
> +    return ret;
> +}
> +
>  int attribute_align_arg av_buffersink_get_frame_flags(AVFilterContext *ctx, AVFrame *frame, int flags)
>  {
> +    if (flags & AV_BUFFERSINK_FLAG_PARAMS) {
> +        if (flags & ~AV_BUFFERSINK_FLAG_PARAMS)
> +            return AVERROR(EINVAL);
> +
> +        return get_frame_params(ctx, frame);
> +    }
> +
>      return get_frame_internal(ctx, frame, flags,
>                                ff_filter_link(ctx->inputs[0])->min_samples);
>  }
> diff --git a/libavfilter/buffersink.h b/libavfilter/buffersink.h
> index 361d603679..9c4468af5b 100644
> --- a/libavfilter/buffersink.h
> +++ b/libavfilter/buffersink.h
> @@ -87,14 +87,26 @@ int av_buffersink_get_frame_flags(AVFilterContext *ctx, AVFrame *frame, int flag
>   * reference, but not remove it from the buffer. This is useful if you
>   * need only to read a video/samples buffer, without to fetch it.
>   */
> -#define AV_BUFFERSINK_FLAG_PEEK 1
> +#define AV_BUFFERSINK_FLAG_PEEK         (1 << 0)
>  
>  /**
>   * Tell av_buffersink_get_buffer_ref() not to request a frame from its input.
>   * If a frame is already buffered, it is read (and removed from the buffer),
>   * but if no frame is present, return AVERROR(EAGAIN).
>   */
> -#define AV_BUFFERSINK_FLAG_NO_REQUEST 2
> +#define AV_BUFFERSINK_FLAG_NO_REQUEST   (1 << 1)
> +

> +/**
> + * Retrieve stream parameters rather than frame data.
> + *
> + * When this flag is set, av_buffersink_get_frame_flags() fills the non-data
> + * fields of the supplied frame without causing any filtergraph activity.
> + *
> + * @note While frame data will be NULL, certain other allocated fields may be
> + * filled (e.g. ch_layout, hw_frames_ctx), so the frame should still be
> + * unreffed before reuse.
> + */
> +#define AV_BUFFERSINK_FLAG_PARAMS       (1 << 2)

NAK, this is one of the worst API design I can think of for this task.

>  
>  /**
>   * Set the frame size for an audio buffer sink.

-- 
  Nicolas George