Nicolas George:
> Part of the code expect valid lists, in particular no duplicates.
Which part of the code fails if there are duplicates?
ff_merge_channel_layouts() can be easily fixed by adding breaks to all
loops, so if it is only this, we could forgo checking as its cost is
quadratic in the number of elements.
> These tests allow to catch bugs in filters (unlikely but possible)
> and to give a clear message when the error comes from the user
> ((a)formats) or the application (buffersink).
>
> Signed-off-by: Nicolas George <geo...@nsup.org>
> ---
> libavfilter/avfiltergraph.c | 50 ++++++++++++++++++++++++++
> libavfilter/formats.c | 71 +++++++++++++++++++++++++++++++++++++
> libavfilter/formats.h | 28 +++++++++++++++
> 3 files changed, 149 insertions(+)
>
> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
> index c8a52b1f47..b9c7669417 100644
> --- a/libavfilter/avfiltergraph.c
> +++ b/libavfilter/avfiltergraph.c
> @@ -313,6 +313,53 @@ static void sanitize_channel_layouts(void *log,
> AVFilterChannelLayouts *l)
> }
> }
>
> +static int filter_link_check_formats(void *log, AVFilterLink *link,
> AVFilterFormatsConfig *cfg)
> +{
> + int ret;
> +
> + switch (link->type) {
> +
> + case AVMEDIA_TYPE_VIDEO:
> + if ((ret = ff_formats_check_pixel_formats(log, cfg->formats)) <
> 0)
> + return ret;
> + break;
> +
> + case AVMEDIA_TYPE_AUDIO:
> + if ((ret = ff_formats_check_sample_formats(log, cfg->formats)) <
> 0 ||
> + (ret = ff_formats_check_sample_rates(log, cfg->samplerates))
> < 0 ||
> + (ret = ff_formats_check_channel_layouts(log,
> cfg->channel_layouts)) < 0)
> + return ret;
> + break;
> +
> + default:
> + av_assert0(!"reached");
> + }
> + return 0;
> +}
> +
> +/**
> + * Check the validity of the formats / etc. lists set by query_formats().
> + *
> + * In particular, check they do not contain any redundant element.
> + */
> +static int filter_check_formats(AVFilterContext *ctx)
> +{
> + unsigned i;
> + int ret;
> +
> + for (i = 0; i < ctx->nb_inputs; i++) {
> + ret = filter_link_check_formats(ctx, ctx->inputs[i],
> &ctx->inputs[i]->outcfg);
> + if (ret < 0)
> + return ret;
> + }
> + for (i = 0; i < ctx->nb_outputs; i++) {
> + ret = filter_link_check_formats(ctx, ctx->outputs[i],
> &ctx->outputs[i]->incfg);
> + if (ret < 0)
> + return ret;
> + }
> + return 0;
> +}
> +
> static int filter_query_formats(AVFilterContext *ctx)
> {
> int ret, i;
> @@ -329,6 +376,9 @@ static int filter_query_formats(AVFilterContext *ctx)
> ctx->name, av_err2str(ret));
> return ret;
> }
> + ret = filter_check_formats(ctx);
> + if (ret < 0)
> + return ret;
>
> for (i = 0; i < ctx->nb_inputs; i++)
> sanitize_channel_layouts(ctx,
> ctx->inputs[i]->outcfg.channel_layouts);
> diff --git a/libavfilter/formats.c b/libavfilter/formats.c
> index 8843200f47..fb32874fb6 100644
> --- a/libavfilter/formats.c
> +++ b/libavfilter/formats.c
> @@ -723,3 +723,74 @@ int ff_parse_channel_layout(int64_t *ret, int *nret,
> const char *arg,
>
> return 0;
> }
> +
> +static int check_list(void *log, const char *name, const AVFilterFormats
> *fmts)
> +{
> + unsigned i, j;
> +
> + if (!fmts)
> + return 0;
> + if (!fmts->nb_formats) {
> + av_log(log, AV_LOG_ERROR, "Empty %s list\n", name);
> + return AVERROR(EINVAL);
> + }
> + for (i = 0; i < fmts->nb_formats; i++) {
> + for (j = i + 1; j < fmts->nb_formats; j++) {
> + if (fmts->formats[i] == fmts->formats[j]) {
> + av_log(log, AV_LOG_ERROR, "Duplicated %s\n", name);
> + return AVERROR(EINVAL);
> + }
> + }
> + }
> + return 0;
> +}
> +
> +int ff_formats_check_pixel_formats(void *log, const AVFilterFormats *fmts)
> +{
> + return check_list(log, "pixel format", fmts);
> +}
> +
> +int ff_formats_check_sample_formats(void *log, const AVFilterFormats *fmts)
> +{
> + return check_list(log, "sample format", fmts);
> +}
> +
> +int ff_formats_check_sample_rates(void *log, const AVFilterFormats *fmts)
> +{
> + if (!fmts || !fmts->nb_formats)
> + return 0;
> + return check_list(log, "sample rate", fmts);
> +}
> +
> +static int layouts_compatible(uint64_t a, uint64_t b)
> +{
> + return a == b ||
> + (KNOWN(a) && !KNOWN(b) && av_get_channel_layout_nb_channels(a) ==
> FF_LAYOUT2COUNT(b)) ||
> + (KNOWN(b) && !KNOWN(a) && av_get_channel_layout_nb_channels(b) ==
> FF_LAYOUT2COUNT(a));
> +}
> +
> +int ff_formats_check_channel_layouts(void *log, const AVFilterChannelLayouts
> *fmts)
> +{
> + unsigned i, j;
> +
> + if (!fmts)
> + return 0;
> + if (fmts->all_layouts < fmts->all_counts ||
> + (!fmts->all_layouts && !fmts->nb_channel_layouts)) {
This check doesn't fit to the error message and it also makes the next
check below dead code.
> + av_log(log, AV_LOG_ERROR, "Inconsistent generic list\n");
> + return AVERROR(EINVAL);
> + }
> + if (!fmts->all_layouts && !fmts->nb_channel_layouts) {
> + av_log(log, AV_LOG_ERROR, "Empty channel layout list\n");
> + return AVERROR(EINVAL);
> + }
> + for (i = 0; i < fmts->nb_channel_layouts; i++) {
> + for (j = i + 1; j < fmts->nb_channel_layouts; j++) {
> + if (layouts_compatible(fmts->channel_layouts[i],
> fmts->channel_layouts[j])) {
> + av_log(log, AV_LOG_ERROR, "Duplicated or redundant channel
> layout\n");
> + return AVERROR(EINVAL);
> + }
> + }
> + }
> + return 0;
> +}
> diff --git a/libavfilter/formats.h b/libavfilter/formats.h
> index ffe7a12d53..40fb2caa85 100644
> --- a/libavfilter/formats.h
> +++ b/libavfilter/formats.h
> @@ -295,4 +295,32 @@ void ff_formats_unref(AVFilterFormats **ref);
> */
> void ff_formats_changeref(AVFilterFormats **oldref, AVFilterFormats
> **newref);
>
> +/**
> + * Check that fmts is a valid pixel formats list.
> + *
> + * In particular, check for duplicates.
> + */
> +int ff_formats_check_pixel_formats(void *log, const AVFilterFormats *fmts);
> +
> +/**
> + * Check that fmts is a valid sample formats list.
> + *
> + * In particular, check for duplicates.
> + */
> +int ff_formats_check_sample_formats(void *log, const AVFilterFormats *fmts);
> +
> +/**
> + * Check that fmts is a valid sample rates list.
> + *
> + * In particular, check for duplicates.
> + */
> +int ff_formats_check_sample_rates(void *log, const AVFilterFormats *fmts);
> +
> +/**
> + * Check that fmts is a valid channel layouts list.
> + *
> + * In particular, check for duplicates.
> + */
> +int ff_formats_check_channel_layouts(void *log, const AVFilterChannelLayouts
> *fmts);
> +
> #endif /* AVFILTER_FORMATS_H */
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
