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

[Stefano Sabatini]

On date Saturday 2008-05-17 19:57:43 +0200, Michael Niedermayer encoded:
> On Sat, May 17, 2008 at 06:49:11PM +0200, Stefano Sabatini wrote:
> > On date Saturday 2008-05-17 17:04:02 +0200, Michael Niedermayer encoded:
> > > On Sat, May 17, 2008 at 04:38:26PM +0200, Stefano Sabatini wrote:
> > > > On date Saturday 2008-05-17 16:14:32 +0200, Michael Niedermayer encoded:
> > > > > On Sat, May 17, 2008 at 12:24:58PM +0200, Stefano Sabatini wrote:
> > > > > > On date Friday 2008-05-16 18:24:11 +0200, Michael Niedermayer encoded:
> > > > > > > On Fri, May 16, 2008 at 01:57:09AM +0200, Stefano Sabatini wrote:
> > > > [...]
> > > > > > > > Index: libavcodec/opt.c
> > > > [...]
> > > > > > > > -static const AVOption *av_set_number(void *obj, const char *name, double num, int den, int64_t intnum){
> > > > > > > > +static const AVOption *av_set_number2(void *obj, const char *name, double num, int den, int64_t intnum, const char **error_ptr){
> > > > > > > >      const AVOption *o= av_find_opt(obj, name, NULL, 0, 0);
> > > > > > > > +    static char error[128];
> > > > > > > > +
> > > > > > > 
> > > > > > > rejected
> > > > > > 
> > > > > > Well I know it sucks like a solution, what about to allocate the error
> > > > > > buffer and leave to the av_set_string2() user to free it?
> > > > > 
> > > > > RTFS eval.c
> > > > 
> > > > Yes, indeed I started from there, the difference is that while in
> > > > eval.c the error string is set to a statically allocated string, I
> > > > would like in this case to return an error message with a variable
> > > > content, e.g.:
> > > > 
> > > > "The value for 'foo' was -1 which is not within 0 - inf\n";
> > > 
> > > "The value for '%s' was %f which is not within %f - %f\n"
> > > 
> > > still static const ...
> > 
> > Yes, but then you need to implement the logic to interpret the
> > parametric string outside av_set_number2(), which seems pretty nasty
> > to me.
> 
> 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.

> 
> 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.

Good night.
-- 
FFmpeg = Foolish Furious Multimedia Pitiful EntanGlement
-------------- next part --------------
A non-text attachment was scrubbed...
Name: implement-av-set-string2-00.patch
Type: text/x-diff
Size: 4881 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080518/c5ff45e7/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: enhance-opt-default-03.patch
Type: text/x-diff
Size: 1838 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080518/c5ff45e7/attachment-0001.patch>