[FFmpeg-devel] [PATCH 1/3] lavfi: remove default config_props() callback and refactor avfilter_config_links()

[Stefano Sabatini]

On date Thursday 2011-08-04 15:47:00 +0300, Mina Nagy Zaki encoded:
> link properties have to be checked after config_props() is called to make sure
> everything is sane, so the default config_props() for output links was
> redundant.
> 
> Also removed channel_layout check as it is now negotiated.
> ---
>  libavfilter/avfilter.c |   59 +++++++++++++++++++++++++++++++++++-------------
>  libavfilter/defaults.c |   23 ------------------
>  2 files changed, 43 insertions(+), 39 deletions(-)
> 
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 7003cdd..23bb26c 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -251,24 +251,51 @@ int avfilter_config_links(AVFilterContext *filter)
>              if ((ret = avfilter_config_links(link->src)) < 0)
>                  return ret;
>  
> -            if (!(config_link = link->srcpad->config_props))
> -                config_link  = avfilter_default_config_output_link;
> -            if ((ret = config_link(link)) < 0)
> +            if (!(config_link = link->srcpad->config_props)) {
> +                if (link->src->input_count != 1) {
> +                    av_log(link->src, AV_LOG_ERROR, "Source filters and filters "
> +                                                    "with more than one input "
> +                                                    "must set config_props() "
> +                                                    "callbacks on all outputs\n");
> +                    return AVERROR(EINVAL);
> +                }
> +            } else if ((ret = config_link(link)) < 0)
>                  return ret;
>  
> -            if (link->time_base.num == 0 && link->time_base.den == 0)
> -                link->time_base = link->src && link->src->input_count ?
> -                    link->src->inputs[0]->time_base : AV_TIME_BASE_Q;
> -
> -            if (link->sample_aspect_ratio.num == 0 && link->sample_aspect_ratio.den == 0)
> -                link->sample_aspect_ratio = link->src->input_count ?
> -                    link->src->inputs[0]->sample_aspect_ratio : (AVRational){1,1};
> -
> -            if (link->sample_rate == 0 && link->src && link->src->input_count)
> -                link->sample_rate = link->src->inputs[0]->sample_rate;
> -
> -            if (link->channel_layout == 0 && link->src && link->src->input_count)
> -                link->channel_layout = link->src->inputs[0]->channel_layout;
> +            switch (link->type) {
> +            case AVMEDIA_TYPE_VIDEO:
> +                if (!link->time_base.num && !link->time_base.den)
> +                    link->time_base = link->src->input_count ?
> +                        link->src->inputs[0]->time_base : AV_TIME_BASE_Q;
> +
> +                if (!link->sample_aspect_ratio.num && !link->sample_aspect_ratio.den)
> +                    link->sample_aspect_ratio = link->src->input_count ?
> +                        link->src->inputs[0]->sample_aspect_ratio : (AVRational){1,1};
> +
> +                if (link->src->input_count) {
> +                    if (!link->w)
> +                        link->w = link->src->inputs[0]->w;
> +                    if (!link->h)
> +                        link->h = link->src->inputs[0]->h;
> +                } else if (!link->w || !link->h) {
> +                    av_log(link->src, AV_LOG_ERROR,
> +                           "Video source filters must set their output link's "
> +                           "width and height\n");
> +                    return AVERROR(EINVAL);
> +                }
> +                break;
> +
> +            case AVMEDIA_TYPE_AUDIO:
> +                if (link->src->input_count) {
> +                    if (!link->sample_rate)
> +                        link->sample_rate = link->src->inputs[0]->sample_rate;
> +                } else if (!link->sample_rate) {
> +                    av_log(link->src, AV_LOG_ERROR,
> +                           "Audio source filters must set their output link's "
> +                           "sample_rate\n");
> +                    return AVERROR(EINVAL);
> +                }
> +            }
>  
>              if ((config_link = link->dstpad->config_props))
>                  if ((ret = config_link(link)) < 0)
> diff --git a/libavfilter/defaults.c b/libavfilter/defaults.c
> index eef9fd1..aadecf2 100644
> --- a/libavfilter/defaults.c
> +++ b/libavfilter/defaults.c
> @@ -174,29 +174,6 @@ void avfilter_default_filter_samples(AVFilterLink *inlink, AVFilterBufferRef *sa
>      inlink->cur_buf = NULL;
>  }
>  

> -/**
> - * default config_link() implementation for output video links to simplify
> - * the implementation of one input one output video filters */
> -int avfilter_default_config_output_link(AVFilterLink *link)
> -{
> -    if (link->src->input_count && link->src->inputs[0]) {
> -        if (link->type == AVMEDIA_TYPE_VIDEO) {
> -            link->w = link->src->inputs[0]->w;
> -            link->h = link->src->inputs[0]->h;
> -            link->time_base = link->src->inputs[0]->time_base;
> -        } else if (link->type == AVMEDIA_TYPE_AUDIO) {
> -            link->channel_layout = link->src->inputs[0]->channel_layout;
> -            link->sample_rate    = link->src->inputs[0]->sample_rate;
> -        }
> -    } else {
> -        /* XXX: any non-simple filter which would cause this branch to be taken
> -         * really should implement its own config_props() for this link. */
> -        return -1;
> -    }
> -
> -    return 0;
> -}

Looks fine, but you should also remove the declaration in avfilter.h
(BTW I noticed that avfilter_default_config_input_link() is never
defined).
-- 
FFmpeg = Frenzy Foolish Miracolous Power Elected Generator