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

[Michael Niedermayer]

On Sun, Jan 25, 2009 at 02:52:24PM +0100, Vitor Sessak wrote:
> Stefano Sabatini wrote:
> > 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?
> 
> I like it (don't know if Michael would want to comment, though)...

i like it as well


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090125/7fb48646/attachment.pgp>