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

[Michael Niedermayer]

On Sun, May 18, 2008 at 09:42:19PM +0200, Stefano Sabatini wrote:
> On date Sunday 2008-05-18 15:39:06 +0200, Michael Niedermayer encoded:
> > On Sun, May 18, 2008 at 01:44:36AM +0200, Stefano Sabatini wrote:
> > > On date Saturday 2008-05-17 19:57:43 +0200, Michael Niedermayer encoded:
> [...]
> > > > an optional 
> > > > av_log(context, AV_LOG_ERROR, error, param, arg, opt.min, opt.max);
> > > 
> > > In this way you have to know how is made the returned string, boring
> > > since you have to document it and not very flexible for example if you
> > > want to change the wording of the message thus the ordering of the
> > > parameters you break it, furthermore if you want to support more than
> > > one message (for example: "The value for '%s' was '%f' which isn't an
> > > integer", only two params) it can't work anymore.
> > 
> > Go read ISO C and POSIX, i could explain how printf() works but ffmpeg-dev
> > really isnt the correct place for this, the docs are clear and
> > understandable.
> [...] 
> > > > vs.
> > > > 
> > > > an optional 
> > > > av_log(context, AV_LOG_ERROR, error);
> > > > and a mandatory
> > > > av_free(error);
> > > > 
> > > > Honestly i dont see how the first is worse, and of course if someone has a
> > > > better idea than these 2 thats welcome as well.
> > > 
> > > Patches attached corresponding to the second approach, the first one
> > > maybe should go in a separate thread.
> > 
> > patches rejected, we will not malloc error messages, the memleaks alone
> > are reason enough why not.
> 
> At this point I'm defeated, I can't see any other ways to resolve the
> problem but:
> 
> 1) use av_log to show the error occurred

That is an option


> 
> 2) allocate a string containing the error in av_set_strin2() and
> mandate the user to free it

That is not an option


> 
> If someone can find a solution corresponding to:
> 
> 3) use a static parametric string containing enough parameter to show

I already suggested that you read the standards about printf()
If you did and still have no clue ill explain how it can be done
but i wont explain it if you are just to lazy to look it up ...


> the various possible error messages:

> Value '%s' for parameter '%s' out of range
> Value '%s' for parameter '%s' is not within %f - %f\n"

redundant


> Value '%s' for parameter '%s' unparsable: %s\n"

Isnt there one %s too much in there ?


> ... possibly others to be added in the future

About which we can think in the future ...


> 
> and a method to correctly fill them outside of av_set_string2() (where
> for example the parsing error messages isn't accessible anymore) 

Here you return the parsing error message


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- 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/20080519/d78104e9/attachment.pgp>