[FFmpeg-devel] [PATCH] Enhance ffmpeg.c:opt_default()

[Stefano Sabatini]

On date Thursday 2008-05-15 01:26:58 +0200, Michael Niedermayer encoded:
> On Wed, May 14, 2008 at 11:56:18PM +0200, Stefano Sabatini wrote:
[...]
> > I think that this patch implements the correct behaviour, which is:
> > 
> > patch) exit immediately when you find in some context the option X,
> >   but its value is invalid, and tell explicitly that the value is
> >   invalid
> > 
> > as opposed to the current behaviour:
> > 
> > none) if a value is found in a context but it fails to set it, try
> >   again with the following context, finally if the value hasn't been
> >   set because it was invalid in one or more contexts or hasn't been
> >   found in any context then always say the option hasn't been found
> > 
> > To me the actual behaviour looks wrong and confusing for the
> > user.
> 
> Well, true but i do not like the solution.
> av_set_string() should make it clear to the caller if an error happened
> and the code should then not continue blindly.
> These wraper and exit() functions really are ugly.
> 
> a "const char **error" argument instead of av_log() calls seems to be
> an option.

Michael, do you prefer the attached solution? (yes I'll split it if
you approve the general idea.)

I'm defining a new av_set_string2() with this signature:
const AVOption *av_set_string2(void *obj, const char *name, const char *val, const char **error_ptr);

which returns the pointer to the option if found even if the value
wasn't valid, in this case sets error_ptr to a static string
containing an error message: this function uses uses the new
av_set_number2().

In order to preserve the semantic of av_set_{double|q|int} I'm also
keeping a function av_set_number() with the old semantic and
signature.

Comments are welcome.

Regards.
-- 
FFmpeg = Frenzy and Foolish Multimedia Proud EnGraver
-------------- next part --------------
A non-text attachment was scrubbed...
Name: enhance-opt-default-02.patch
Type: text/x-diff
Size: 6562 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080516/4a6fb67e/attachment.patch>