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

[Vitor Sessak]

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)...

-Vitor