[FFmpeg-devel] [PATCH] ffmpeg: generate more specific error message in case of inconsistent filtering options

[Stefano Sabatini]

On date Saturday 2013-11-09 10:22:54 +0100, Nicolas George encoded:
> Le quartidi 14 brumaire, an CCXXII, Stefano Sabatini a écrit :
> > In particular, distinguish if the option was -filter or -filter_script.
> > 
> > Suggested-by: Nicolas George
> > ---
> >  ffmpeg_opt.c | 29 ++++++++++++++---------------
> >  1 file changed, 14 insertions(+), 15 deletions(-)
> > 
> > diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
> > index be9b7ed..23b91d9 100644
> > --- a/ffmpeg_opt.c
> > +++ b/ffmpeg_opt.c
> > @@ -1172,10 +1172,10 @@ static void check_streamcopy_filters(OptionsContext *o, AVFormatContext *oc,
> >  {
> >      if (ost->filters_script || ost->filters) {
> >          av_log(NULL, AV_LOG_ERROR,
> > -               "Filtergraph '%s' or filter_script '%s' was defined for %s output stream "
> > -               "%d:%d but codec copy was selected.\n",
> > -               (char *)av_x_if_null(ost->filters, "(none)"),
> > -               (char *)av_x_if_null(ost->filters_script, "(none)"),
> 
> > +               "%s '%s' was defined for %s output stream %d:%d but codec copy was selected.\n"
> > +               "Filtering and streamcopy cannot be used together.\n",
> > +               ost->filters ? "Filtergraph" : "Filtergraph script",
> 

> You could put the "Filtergraph " part in the format string, but I do not
> insist on it.

I consider the current form slightly preferable in terms of
readability, although more verbose, thus retained.
 
> > +               ost->filters ? ost->filters : ost->filters_script,
> >                 av_get_media_type_string(type),
> >                 ost->file_index, ost->index);
> >          exit_program(1);
> > @@ -1571,17 +1571,16 @@ static void init_output_filter(OutputFilter *ofilter, OptionsContext *o,
> >          exit_program(1);
> >      }
> >  
> > -    if (ost->avfilter) {
> > -        if (ost->filters || ost->filters_script) {
> > -            av_log(NULL, AV_LOG_ERROR,
> > -                   "Filter graph '%s' or filter script '%s' was specified through the -filter/-filter_script/-vf/-af option "
> > -                   "for output stream %d:%d, which is fed from a complex filtergraph.\n"
> > -                   "-filter/-filter_script and -filter_complex cannot be used together for the same stream.\n",
> > -                   (char *)av_x_if_null(ost->filters, "(none)"),
> > -                   (char *)av_x_if_null(ost->filters_script, "(none)"),
> > -                   ost->file_index, ost->index);
> > -            exit_program(1);
> > -        }
> > +    if (ost->avfilter && (ost->filters || ost->filters_script)) {
> > +        const char *opt = ost->filters ? "-vf/-af/-filter" : "-filter_script";
> > +        av_log(NULL, AV_LOG_ERROR,
> > +               "%s '%s' was specified through the %s option "
> > +               "for output stream %d:%d, which is fed from a complex filtergraph.\n"
> > +               "%s and -filter_complex cannot be used together for the same stream.\n",
> > +               ost->filters ? "Filtergraph" : "Filtergraph script",
> 
> Ditto.
> 
> > +               ost->filters ? ost->filters : ost->filters_script,
> > +               opt, ost->file_index, ost->index, opt);
> > +        exit_program(1);
> >      }
> >  
> >      if (configure_output_filter(ofilter->graph, ofilter, ofilter->out_tmp) < 0) {
> 
> LGTM, thanks.

Pushed.
-- 
FFmpeg = Fancy & Fundamentalist Meaningful Perfectionist Elastic Goblin