[FFmpeg-devel] [RFC] avfilter_request_frame() crash

[Stefano Sabatini]

On date Friday 2009-01-16 01:49:12 +0100, Stefano Sabatini encoded:
> On date Thursday 2009-01-15 23:12:12 +0100, Michael Niedermayer encoded:
> > On Thu, Jan 15, 2009 at 09:37:39PM +0100, Vitor Sessak wrote:
> > > Stefano Sabatini wrote:
> > > > Hi all,
> > > > 
> > > > when calling avfilter_request_frame() on a link with no source it
> > > > crashes, for example with:
> > > > 
> > > > ffplay -f video4linux -s 320x240 /dev/video -vfilters "[in] setpts=PTS-STARTPTS, hflip, setpts=PTS-STARTPTS [foo]"
> > > > 
> > > > Should we:
> > > > 1) add a check in avfilter_request_frame(), so that it will return -1
> > > >    if the link source is NULL
> > > > 2) ensure that no dangling links exist before to start to extract
> > > >    filters from the filterchain
> > > > 3) both 1) and 2)
> > > 
> > > I'd say (2) or (3), I don't think "[in] setpts=PTS-STARTPTS, hflip, 
> > > setpts=PTS-STARTPTS [foo]" should be considered a valid description of a 
> > > filter chain.
> > 
> > agree
> 
> With the patch:
> 
> stefano at geppetto ~/s/l/ffmpeg> ffplay -f video4linux -s 320x240 /dev/video -vfilters "[in] setpts=PTS-STARTPTS, hflip, setpts=PTS-STARTPTS [foo]"
> FFplay version SVN-r16562, Copyright (c) 2003-2009 Fabrice Bellard, et al.
> [...]
> Output pad "default" for the filter "Parsed filter 2" of type "setpts" not connected to any destination
> 
> If we want to avoid 1) we can request the user to call the function
> before to start any actual processing, saying that in the docs should
> be sufficient.
> 
> Regards.
> -- 
> FFmpeg = Faboulous Friendly Mind-dumbing Prodigious Everlasting Guide

> Index: libavfilter-soc/ffmpeg/libavfilter/avfiltergraph.c
> ===================================================================
> --- libavfilter-soc.orig/ffmpeg/libavfilter/avfiltergraph.c	2009-01-16 00:49:00.000000000 +0100
> +++ libavfilter-soc/ffmpeg/libavfilter/avfiltergraph.c	2009-01-16 01:45:33.000000000 +0100
> @@ -46,6 +46,36 @@
>      return 0;
>  }
>  
> +int avfilter_graph_check_validity(AVFilterGraph *graph, AVClass *log_ctx)
> +{
> +    AVFilterContext *filt;
> +    int i, j;
> +
> +    for (i=0; i < graph->filter_count; i++) {
> +        filt = graph->filters[i];
> +
> +        for (j = 0; j < filt->input_count; j++) {
> +            if (!filt->inputs[j] || !filt->inputs[j]->src) {
> +                av_log(log_ctx, AV_LOG_ERROR,
> +                       "Input pad \"%s\" for the filter \"%s\" of type \"%s\" not connected to any source\n",
> +                       filt->input_pads[j].name, filt->name, filt->filter->name);
> +                return -1;
> +            }
> +        }
> +
> +        for (j = 0; j < filt->output_count; j++) {
> +            if (!filt->outputs[j] || !filt->outputs[j]->dst) {
> +                av_log(log_ctx, AV_LOG_ERROR,
> +                       "Output pad \"%s\" for the filter \"%s\" of type \"%s\" not connected to any destination\n",
> +                       filt->output_pads[j].name, filt->name, filt->filter->name);
> +                return -1;
> +            }
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  AVFilterContext *avfilter_graph_get_filter(AVFilterGraph *graph, char *name)
>  {
>      int i;
> Index: libavfilter-soc/ffmpeg/libavfilter/avfiltergraph.h
> ===================================================================
> --- libavfilter-soc.orig/ffmpeg/libavfilter/avfiltergraph.h	2009-01-16 00:49:00.000000000 +0100
> +++ libavfilter-soc/ffmpeg/libavfilter/avfiltergraph.h	2009-01-16 01:06:39.000000000 +0100
> @@ -45,6 +45,16 @@
>  int avfilter_graph_add_filter(AVFilterGraph *graphctx, AVFilterContext *filter);
>  
>  /**
> + * Check for the validity of \p graph.
> + *
> + * A graph is considered valid if all the input pads and all the
> + * output pads of it are connected.
> + *
> + * @return 0 in case of success, a negative value otherwise
> + */
> +int avfilter_graph_check_validity(AVFilterGraph *graphctx, AVClass *log_ctx);
> +
> +/**
>   * Configure the formats of all the links in the graph.
>   */
>  int avfilter_graph_config_formats(AVFilterGraph *graphctx);

> Index: libavfilter-soc/ffmpeg/ffplay.c
> ===================================================================
> --- libavfilter-soc.orig/ffmpeg/ffplay.c	2009-01-16 01:35:41.000000000 +0100
> +++ libavfilter-soc/ffmpeg/ffplay.c	2009-01-16 01:40:43.000000000 +0100
> @@ -1624,6 +1624,8 @@
>      }
>      avfilter_graph_add_filter(graph, filt_src);
>      avfilter_graph_add_filter(graph, filt_out);
> +
> +    if(avfilter_graph_check_validity(graph, NULL))           goto the_end;
>      if(avfilter_graph_config_formats(graph))                 goto the_end;
>      if(avfilter_config_links(filt_out))                      goto the_end;
>  

Ping?
-- 
FFmpeg = Funny Fundamental Mega Pure Erroneous Gadget