[FFmpeg-devel] [PATCH 3/3] lavfi: support multiple rounds of format negotiation.

[Stefano Sabatini]

On date Thursday 2013-02-21 20:44:15 +0100, Nicolas George encoded:
> Remove the temporary hack for amerge and replace it with a
> generic solution.
> 
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
>  doc/filter_design.txt       |    5 +++
>  libavfilter/avfiltergraph.c |  103 ++++++++++++++++++++++++++++++++-----------
>  2 files changed, 83 insertions(+), 25 deletions(-)
> 
> 
> The logic is this: the query_formats() function will fail with
> AVERROR(EINVAL) if some filters could not be configured yet but progress was

Do you mean AVERROR(EAGAIN)?

> made, and the calling site will loop on that condition. It is as simple as
> that, all the noise in the diff is mostly connected to making sure that
> filters are not configured twice (most of them would cause crashes).
> 
> I believe that with this feature, overlay can peek at its inputs supported
> formats (and possibly output too) and decide automatically between the
> yuv420/yuv444/rgb mode.
> 
> Note that that feature must be used parsimoniously, because the mode filters
> use it, the more likely it is that the process will fail because of a
> circular dependency.
> 
[...]
> @@ -238,27 +240,46 @@ static int filter_query_formats(AVFilterContext *ctx)
>      return 0;
>  }
>  
> +static int formats_queried(AVFilterContext *f)
> +{
> +    int i;
> +
> +    for (i = 0; i < f->nb_inputs; i++) {
> +        if (!f->inputs[i]->out_formats)
> +            return 0;
> +        if (f->inputs[i]->type == AVMEDIA_TYPE_AUDIO &&
> +            !(f->inputs[i]->out_samplerates &&
> +              f->inputs[i]->out_channel_layouts))
> +            return 0;
> +    }
> +    for (i = 0; i < f->nb_outputs; i++) {
> +        if (!f->outputs[i]->in_formats)
> +            return 0;
> +        if (f->outputs[i]->type == AVMEDIA_TYPE_AUDIO &&
> +            !(f->outputs[i]->in_samplerates &&
> +              f->outputs[i]->in_channel_layouts))
> +            return 0;
> +    }
> +    return 1;
> +}
> +
>  static int query_formats(AVFilterGraph *graph, AVClass *log_ctx)
>  {
>      int i, j, ret;
>      int scaler_count = 0, resampler_count = 0;
> +    int merge_done = 0, merge_already = 0, merge_delayed = 0;

I suggest: merge_already -> merge_already_done

>  
> -    for (j = 0; j < 2; j++) {
> -    /* ask all the sub-filters for their supported media formats */
>      for (i = 0; i < graph->filter_count; i++) {
> -        /* Call query_formats on sources first.
> -           This is a temporary workaround for amerge,
> -           until format renegociation is implemented. */
> -        if (!graph->filters[i]->nb_inputs == j)
> +        AVFilterContext *f = graph->filters[i];
> +        if (formats_queried(f))
>              continue;
> -        if (graph->filters[i]->filter->query_formats)
> -            ret = filter_query_formats(graph->filters[i]);
> +        if (f->filter->query_formats)
> +            ret = filter_query_formats(f);
>          else
> -            ret = ff_default_query_formats(graph->filters[i]);
> -        if (ret < 0)
> +            ret = ff_default_query_formats(f);
> +        if (ret < 0 && ret != AVERROR(EAGAIN))
>              return ret;
>      }
> -    }
>  
>      /* go through and merge as many format lists as possible */
>      for (i = 0; i < graph->filter_count; i++) {
> @@ -271,20 +292,32 @@ static int query_formats(AVFilterGraph *graph, AVClass *log_ctx)
>              if (!link)
>                  continue;
>  
> -            if (link->in_formats != link->out_formats &&
> -                !ff_merge_formats(link->in_formats,
> -                                        link->out_formats))
> -                convert_needed = 1;
> +#define MERGE_DISPATCH(field, statement)                                     \
> +            if (!(link->in_ ## field && link->out_ ## field)) {              \
> +                merge_delayed++;                                             \
> +            } else if (link->in_ ## field == link->out_ ## field) {          \
> +                merge_already++;                                             \
> +            } else {                                                         \
> +                merge_done++;                                                \
> +                statement                                                    \
> +            }
> +            MERGE_DISPATCH(formats,
> +                if (!ff_merge_formats(link->in_formats, link->out_formats))
> +                    convert_needed = 1;
> +            )
>              if (link->type == AVMEDIA_TYPE_AUDIO) {
> -                if (link->in_channel_layouts != link->out_channel_layouts &&
> -                    !ff_merge_channel_layouts(link->in_channel_layouts,
> +                MERGE_DISPATCH(channel_layouts,
> +                    if (!ff_merge_channel_layouts(link->in_channel_layouts,
>                                                link->out_channel_layouts))
> -                    convert_needed = 1;
> -                if (link->in_samplerates != link->out_samplerates &&
> -                    !ff_merge_samplerates(link->in_samplerates,
> -                                          link->out_samplerates))
> -                    convert_needed = 1;
> +                        convert_needed = 1;
> +                )
> +                MERGE_DISPATCH(samplerates,
> +                    if (!ff_merge_samplerates(link->in_samplerates,
> +                                              link->out_samplerates))
> +                        convert_needed = 1;
> +                )
>              }
> +#undef MERGE_DISPATCH
>  
>              if (convert_needed) {
>                  AVFilterContext *convert;
> @@ -363,6 +396,24 @@ static int query_formats(AVFilterGraph *graph, AVClass *log_ctx)
>          }
>      }
>  
> +    av_log(graph, AV_LOG_DEBUG,
> +           "query_formats: %d done, %d already, %d delayed\n",
> +           merge_done, merge_already, merge_delayed);
> +    if (merge_delayed) {
> +        AVBPrint bp;
> +
> +        if (merge_done)
> +            return AVERROR(EAGAIN);
> +        av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC);
> +        for (i = 0; i < graph->filter_count; i++)
> +            if (!formats_queried(graph->filters[i]))
> +                av_bprintf(&bp, "%s%s", bp.len ? ", " : "",
> +                          graph->filters[i]->name);
> +        av_log(graph, AV_LOG_ERROR,
> +               "The following filters could not choose their formats: %s\n"
> +               "Consider inserting the (a)format filter.\n", bp.str);
> +        return AVERROR(EIO);
> +    }

Looks good from a second reading apart from the nits, thanks.
-- 
FFmpeg = Fascinating Faithless Multipurpose Pacific Educated Goblin