[FFmpeg-devel] [PATCH 2/3] lavfi/buffersink: accept parameters as options.
Stefano Sabatini
stefasab at gmail.com
Thu Apr 11 22:49:55 CEST 2013
On date Thursday 2013-04-11 14:58:07 +0200, Nicolas George encoded:
> Move validation from init to query_formats().
> Accept the formats lists as binary options.
>
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
> doc/filters.texi | 22 ++----
> libavfilter/buffersink.c | 187 +++++++++++++++++++++++++++++-----------------
> 2 files changed, 127 insertions(+), 82 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 12a7cf5..319844f 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -1783,9 +1783,10 @@ Below is a description of the currently available audio sinks.
> Buffer audio frames, and make them available to the end of filter chain.
>
> This sink is mainly intended for programmatic use, in particular
> -through the interface defined in @file{libavfilter/buffersink.h}.
> +through the interface defined in @file{libavfilter/buffersink.h}
> +or the options system.
>
> -It requires a pointer to an AVABufferSinkContext structure, which
> +It accepts a pointer to an AVABufferSinkContext structure, which
> defines the incoming buffers' formats, to be passed as the opaque
> parameter to @code{avfilter_init_filter} for initialization.
>
> @@ -1795,13 +1796,6 @@ Null audio sink, do absolutely nothing with the input audio. It is
> mainly useful as a template and to be employed in analysis / debugging
> tools.
>
> - at section abuffersink
> -This sink is intended for programmatic use. Frames that arrive on this sink can
> -be retrieved by the calling program using the interface defined in
> - at file{libavfilter/buffersink.h}.
> -
> -This filter accepts no parameters.
> -
> @c man end AUDIO SINKS
>
> @chapter Video Filters
> @@ -6330,12 +6324,12 @@ Buffer video frames, and make them available to the end of the filter
> graph.
>
> This sink is mainly intended for a programmatic use, in particular
> -through the interface defined in @file{libavfilter/buffersink.h}.
> +through the interface defined in @file{libavfilter/buffersink.h}
> +or the options system.
>
> -It does not require a string parameter in input, but you need to
> -specify a pointer to a list of supported pixel formats terminated by
> --1 in the opaque parameter provided to @code{avfilter_init_filter}
> -when initializing this sink.
> +It accepts a pointer to an AVBufferSinkContext structure, which
> +defines the incoming buffers' formats, to be passed as the opaque
> +parameter to @code{avfilter_init_filter} for initialization.
>
> @section nullsink
>
> diff --git a/libavfilter/buffersink.c b/libavfilter/buffersink.c
> index 0f500d0..60ed436 100644
> --- a/libavfilter/buffersink.c
> +++ b/libavfilter/buffersink.c
> @@ -28,6 +28,7 @@
> #include "libavutil/channel_layout.h"
> #include "libavutil/common.h"
> #include "libavutil/mathematics.h"
> +#include "libavutil/opt.h"
>
> #include "audio.h"
> #include "avfilter.h"
> @@ -35,23 +36,32 @@
> #include "internal.h"
>
> typedef struct {
> + const AVClass *class;
> AVFifoBuffer *fifo; ///< FIFO buffer of video frame references
> unsigned warning_limit;
>
> /* only used for video */
> enum AVPixelFormat *pixel_fmts; ///< list of accepted pixel formats, must be terminated with -1
> + int pixel_fmts_size;
>
> /* only used for audio */
> enum AVSampleFormat *sample_fmts; ///< list of accepted sample formats, terminated by AV_SAMPLE_FMT_NONE
> + int sample_fmts_size;
> int64_t *channel_layouts; ///< list of accepted channel layouts, terminated by -1
> + int channel_layouts_size;
> + int *channel_counts; ///< list of accepted channel counts, terminated by -1
> + int channel_counts_size;
> int all_channel_counts;
> int *sample_rates; ///< list of accepted sample rates, terminated by -1
> + int sample_rates_size;
>
> /* only used for compat API */
> AVAudioFifo *audio_fifo; ///< FIFO for audio samples
> int64_t next_pts; ///< interpolating audio pts
> } BufferSinkContext;
>
> +#define NB_ITEMS(list) (list ## _size / sizeof(*list))
> +
> static av_cold void uninit(AVFilterContext *ctx)
> {
> BufferSinkContext *sink = ctx->priv;
> @@ -68,10 +78,6 @@ static av_cold void uninit(AVFilterContext *ctx)
> av_fifo_free(sink->fifo);
> sink->fifo = NULL;
> }
> - av_freep(&sink->pixel_fmts);
> - av_freep(&sink->sample_fmts);
> - av_freep(&sink->sample_rates);
> - av_freep(&sink->channel_layouts);
> }
>
> static int add_buffer_ref(AVFilterContext *ctx, AVFrame *ref)
> @@ -286,6 +292,7 @@ static int attribute_align_arg compat_read(AVFilterContext *ctx, AVFilterBufferR
> if (ret < 0)
> goto fail;
>
> + AV_NOWARN_DEPRECATED(
> if (ctx->inputs[0]->type == AVMEDIA_TYPE_VIDEO) {
> buf = avfilter_get_video_buffer_ref_from_arrays(frame->data, frame->linesize,
> AV_PERM_READ,
> @@ -304,6 +311,7 @@ static int attribute_align_arg compat_read(AVFilterContext *ctx, AVFilterBufferR
> }
>
> avfilter_copy_frame_props(buf, frame);
> + )
>
> buf->buf->priv = frame;
> buf->buf->free = compat_free_buffer;
> @@ -366,13 +374,11 @@ static av_cold int vsink_init(AVFilterContext *ctx, const char *args, void *opaq
> {
> BufferSinkContext *buf = ctx->priv;
> AVBufferSinkParams *params = opaque;
> + int ret;
>
> - if (params && params->pixel_fmts) {
> - const int *pixel_fmts = params->pixel_fmts;
> -
> - buf->pixel_fmts = ff_copy_int_list(pixel_fmts);
> - if (!buf->pixel_fmts)
> - return AVERROR(ENOMEM);
> + if (params) {
> + if ((ret = av_opt_set_int_list(buf, "pix_fmts", params->pixel_fmts, AV_PIX_FMT_NONE, 0)) < 0)
> + return ret;
> }
>
> return common_init(ctx);
> @@ -381,64 +387,41 @@ static av_cold int vsink_init(AVFilterContext *ctx, const char *args, void *opaq
> static int vsink_query_formats(AVFilterContext *ctx)
> {
> BufferSinkContext *buf = ctx->priv;
> + AVFilterFormats *formats = NULL;
> + unsigned i;
> + int ret;
>
> - if (buf->pixel_fmts)
> - ff_set_common_formats(ctx, ff_make_format_list(buf->pixel_fmts));
> - else
> + if (buf->pixel_fmts_size % sizeof(*buf->pixel_fmts)) {
> + av_log(ctx, AV_LOG_ERROR, "Invalid size for format list\n");
> + return AVERROR(EINVAL);
> + }
can this happen?
> +
> + if (buf->pixel_fmts_size) {
> + for (i = 0; i < NB_ITEMS(buf->pixel_fmts); i++)
> + if ((ret = ff_add_format(&formats, buf->pixel_fmts[i])) < 0)
> + return ret;
> + ff_set_common_formats(ctx, formats);
> + } else {
> ff_default_query_formats(ctx);
> + }
>
> return 0;
> }
>
> -static int64_t *concat_channels_lists(const int64_t *layouts, const int *counts)
> -{
> - int nb_layouts = 0, nb_counts = 0, i;
> - int64_t *list;
> -
> - if (layouts)
> - for (; layouts[nb_layouts] != -1; nb_layouts++);
> - if (counts)
> - for (; counts[nb_counts] != -1; nb_counts++);
> - if (nb_counts > INT_MAX - 1 - nb_layouts)
> - return NULL;
> - if (!(list = av_calloc(nb_layouts + nb_counts + 1, sizeof(*list))))
> - return NULL;
> - for (i = 0; i < nb_layouts; i++)
> - list[i] = layouts[i];
> - for (i = 0; i < nb_counts; i++)
> - list[nb_layouts + i] = FF_COUNT2LAYOUT(counts[i]);
> - list[nb_layouts + nb_counts] = -1;
> - return list;
> -}
> -
> static av_cold int asink_init(AVFilterContext *ctx, const char *args, void *opaque)
> {
> BufferSinkContext *buf = ctx->priv;
> AVABufferSinkParams *params = opaque;
> + int ret;
>
> - if (params && params->sample_fmts) {
> - buf->sample_fmts = ff_copy_int_list(params->sample_fmts);
> - if (!buf->sample_fmts)
> - return AVERROR(ENOMEM);
> - }
> - if (params && params->sample_rates) {
> - buf->sample_rates = ff_copy_int_list(params->sample_rates);
> - if (!buf->sample_rates)
> - return AVERROR(ENOMEM);
> - }
> - if (params && (params->channel_layouts || params->channel_counts)) {
> - if (params->all_channel_counts) {
> - av_log(ctx, AV_LOG_ERROR,
> - "Conflicting all_channel_counts and list in parameters\n");
> - return AVERROR(EINVAL);
> - }
> - buf->channel_layouts = concat_channels_lists(params->channel_layouts,
> - params->channel_counts);
> - if (!buf->channel_layouts)
> - return AVERROR(ENOMEM);
> + if (params) {
> + if ((ret = av_opt_set_int_list(buf, "sample_fmts", params->sample_fmts, AV_SAMPLE_FMT_NONE, 0)) < 0 ||
> + (ret = av_opt_set_int_list(buf, "sample_rates", params->sample_rates, -1, 0)) < 0 ||
> + (ret = av_opt_set_int_list(buf, "channel_layouts", params->channel_layouts, -1, 0)) < 0 ||
> + (ret = av_opt_set_int_list(buf, "channel_counts", params->channel_counts, -1, 0)) < 0 ||
> + (ret = av_opt_set_int(buf, "all_channel_counts", params->all_channel_counts, 0)) < 0)
> + return ret;
> }
> - if (params)
> - buf->all_channel_counts = params->all_channel_counts;
> return common_init(ctx);
> }
>
> @@ -447,32 +430,92 @@ static int asink_query_formats(AVFilterContext *ctx)
> BufferSinkContext *buf = ctx->priv;
> AVFilterFormats *formats = NULL;
> AVFilterChannelLayouts *layouts = NULL;
> + unsigned i;
> + int ret;
>
> - if (buf->sample_fmts) {
> - if (!(formats = ff_make_format_list(buf->sample_fmts)))
> - return AVERROR(ENOMEM);
> + if (buf->sample_fmts_size % sizeof(*buf->sample_fmts) ||
> + buf->sample_rates_size % sizeof(*buf->sample_rates) ||
> + buf->channel_layouts_size % sizeof(*buf->channel_layouts) ||
> + buf->channel_counts_size % sizeof(*buf->channel_counts)) {
> + av_log(ctx, AV_LOG_ERROR, "Invalid size for format lists\n");
> +#define LOG_ERROR(field) \
> + if (buf->field ## _size % sizeof(*buf->field)) \
> + av_log(ctx, AV_LOG_ERROR, " " #field " is %d, should be " \
> + "multiple of %d\n", \
> + buf->field ## _size, (int)sizeof(*buf->field));
> + LOG_ERROR(sample_fmts);
> + LOG_ERROR(sample_rates);
> + LOG_ERROR(channel_layouts);
> + LOG_ERROR(channel_counts);
> +#undef LOG_ERROR
Nit: you could merge check and log error in the same macro, maybe save
some lines.
Also again, if the list is always set by av_opt_set_int_list(), how
could size be wrong?
[...]
LGTM otherwise, thanks.
--
FFmpeg = Faithful and Fanciful Marvellous Programmable Erratic Gladiator
More information about the ffmpeg-devel
mailing list