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

Michael Niedermayer michaelni
Thu May 15 01:26:58 CEST 2008


On Wed, May 14, 2008 at 11:56:18PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2008-05-13 02:16:11 +0200, Michael Niedermayer encoded:
> > On Mon, May 12, 2008 at 11:53:48PM +0200, Stefano Sabatini wrote:
> > > Hi all,
> > > 
> > > actually opt_default() will keep trying to set an option even if it
> > > fails to set it in some context.
> > > 
> > > The correct (or a more correct behaviour) seems to simply give up when
> > > the option is found the first time but its value is invalid for that
> > > context, in this case the program immediatly exits.
> > > 
> > > Also the patch make opt_default() distinguishes if an option wasn't
> > > found in all the checked contexts or if the option was found in some
> > > context but its value was invalid.
> > 
> > Nice so the patch does nothing except making the code more complex?
> > patch rejected
> 
> 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     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- 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/20080515/5656216c/attachment.pgp>



More information about the ffmpeg-devel mailing list