[FFmpeg-devel] [PATCH] Fix ffmpeg weird 'Unknown option' error messages

Stefano Sabatini stefano.sabatini-lala
Mon Oct 5 21:07:53 CEST 2009


On date Monday 2009-10-05 00:55:39 +0200, Michael Niedermayer encoded:
> On Sun, Oct 04, 2009 at 01:34:00PM -0700, Baptiste Coudurier wrote:
> > On 6/18/09 10:07 PM, Baptiste Coudurier wrote:
> >> Michael Niedermayer wrote:
> >>> On Wed, May 27, 2009 at 11:37:42PM +0200, Stefano Sabatini wrote:
> >>>> On date Wednesday 2009-05-27 04:01:54 +0200, Michael Niedermayer 
> >>>> encoded:
> >>>>> On Tue, May 26, 2009 at 10:13:42PM +0200, Stefano Sabatini wrote:
> >>>>>> On date Tuesday 2009-05-26 03:25:04 +0200, Michael Niedermayer 
> >>>>>> encoded:
> >>>>>>> On Tue, May 26, 2009 at 01:32:44AM +0200, Stefano Sabatini wrote:
> >>>> [...]
> >>>>>>>> Can you see another solution?
> >>>>>>> functions should not print errors that might not be errors to the 
> >>>>>>> appliction
> >>>>>> Sorry but this looks like a distorted way of thinking, functions
> >>>>> so you do know of a single function in the standard c lib that does not
> >>>>> follow this statement?
> >>>>>
> >>>>>> should print errors when it makes sense, they should be the
> >>>>>> applications to adapt themselves to the library and not vice-versa.
> >>>>>>
> >>>>>> That said, I'll revert r18826 if there is no better solution for that.
> >>>> So should I revert it?
> >>>
> >>> if you want
> >>
> >> Well, IMHO av_set_string3 is right to print an error if the option does
> >> not exists, it's an error.
> >>
> >> So IMHO patch is correct, application should not call it this way.
> >>
> >
> > So what about this patch ? Can we please apply it to remove the annoying 
> > nonsense messages ?
> 
> as i said av_set_string*() should not print these messages. They should
> cleanly return the errorcode and let the user application decide or the user
> application should redirect the av_log() output somehow.
> 
> with stefanos design, that is av_set_string() printing errors directly
> for every little thing, things really cannot work. av_find_opt() should
> then also print a "option not found message" for consistencies sake,
> this of course doesnt work so why should av_set_string() print
> something for a failure when av_find_opt() does not print anything
> for a failure ?  Its just not consistent or logic IMHO

No, what av_opt_find() does is:
"Look for an option in obj",

there are only two possible results for that operation, found or not
found and this is perfectly clear from the returned value.

As for av_set_string3() it sets a value in an option whose is given
the key, so there are many kind of failure which can occurr here, so
it makes sense to print an error messages *togheter* with an error
code to detail useful information for the user.

To make another example, avcodec_open() may fail for many reasons, but
only returns -1, if we'll ever manage to return an error code we would
still need an error message telling precisely the failure reason
("option foo is not implemented, samplerate 12345 is not supported,
etc etc).

Now, I proposed the patch when I was implementing
libavfilter.c:parse_key_value_pair(), and I have:

    ret = av_set_string3(ctx, key, val, 1, NULL);

I don't see why I should treat the "option not found case" in a
different way, and have for example this:
        
    ret = av_set_string3(ctx, key, val, 1, NULL);
    if (ret == AVERROR_ENOENT)
       av_log(ctx, AV_LOG_ERROR, "Option '%s' not found\n", key);

So the question is:
* is the case of a not found option in a context to be treated as a
  special case by av_set_string3()?

My opinion is that no it shouldn't, others may prefer the opposite.

Anyway I won't object if someone will revert r18826, but I won't do
that.

Regards.
-- 
FFmpeg = Free and Friendly Marvellous Pacific Elastic Glue



More information about the ffmpeg-devel mailing list