[FFmpeg-devel] [PATCH] Fix opt_default()
Stefano Sabatini
stefano.sabatini-lala
Tue Dec 16 00:31:02 CET 2008
On date Monday 2008-12-15 02:41:25 +0100, Michael Niedermayer encoded:
> On Mon, Dec 15, 2008 at 12:08:49AM +0100, Stefano Sabatini wrote:
> > On date Sunday 2008-12-07 18:49:21 +0100, Michael Niedermayer encoded:
> > > On Sun, Dec 07, 2008 at 05:10:11PM +0100, Stefano Sabatini wrote:
> > > > On date Sunday 2008-12-07 14:35:58 +0100, Michael Niedermayer encoded:
> > > > > On Sun, Dec 07, 2008 at 02:45:16AM +0100, Stefano Sabatini wrote:
> > [...]
> > > > > > As for the index in the options array, I found its use quite awkward,
> > > > > > but anyway if you like it I can implement this:
> > > > > >
> > > > > > int av_set_string3(void *obj, const char *name, const char *val);
> > > > >
> > > > > iam not insisting on this, what i insist on is that we will not have
> > > > > 20 return -1 and parse an error message (or check if its NULL) to find
> > > > > out which kind of error it was.
> > > >
> > > > I continue to prefer:
> > > >
> > > > 1) int av_set_string3(void *obj, const char *name, const char *val, const AVOption **o_out);
> > > > (return an error code or 0 if the option has been set, put in o_out
> > > > the option eventually found)
> > > >
> > > > over:
> > > >
> > > > 2) int av_set_string3(void *obj, const char *name, const char *val);
> > > > (return an error code or the index in the obj->options array if the
> > > > option is found and the value is valid)
> > > >
> > > > but I won't insist if you prefer the other one, please just say which
> > > > you prefer.
> > > >
> > > > Advantage of 1) over 2) is that you know the option which has been
> > > > found even in case of error, so no need to iterate through options to
> > > > find it again, if you don't care about the option found you can simply
> > > > set o_out to NULL.
> > >
> > > i dont mind 1.
> >
> > Patchset attached.
> >
> > Regards.
> > --
> > FFmpeg = Fiendish and Fantastic Magic Power Energized Glue
>
> > Index: ffmpeg/libavcodec/opt.c
> > ===================================================================
> > --- ffmpeg.orig/libavcodec/opt.c 2008-12-06 01:06:53.000000000 +0100
> > +++ ffmpeg/libavcodec/opt.c 2008-12-06 01:13:34.000000000 +0100
> > @@ -54,7 +54,7 @@
> > return NULL;
> >
> > if(o->max*den < num*intnum || o->min*den > num*intnum) {
> > - av_log(NULL, AV_LOG_ERROR, "Value %lf for parameter '%s' out of range.\n", num, name);
> > + av_log(NULL, AV_LOG_ERROR, "Value %lf for parameter '%s' out of range\n", num, name);
> > return NULL;
> > }
> >
>
> ok
Applied.
> > Index: ffmpeg/libavcodec/opt.c
> > ===================================================================
> > --- ffmpeg.orig/libavcodec/opt.c 2008-12-14 22:42:21.000000000 +0100
> > +++ ffmpeg/libavcodec/opt.c 2008-12-14 23:57:53.000000000 +0100
> > @@ -47,15 +47,17 @@
> > else return (*(AVClass**)obj)->option;
> > }
> >
> > -static const AVOption *av_set_number(void *obj, const char *name, double num, int den, int64_t intnum){
> > +static int av_set_number2(void *obj, const char *name, double num, int den, int64_t intnum, const AVOption **o_out){
> > const AVOption *o= av_find_opt(obj, name, NULL, 0, 0);
> > void *dst;
> > + if(o_out)
> > + *o_out= o;
> > if(!o || o->offset<=0)
> > - return NULL;
> > + return AVERROR(ENOENT);
> >
> > if(o->max*den < num*intnum || o->min*den > num*intnum) {
> > av_log(NULL, AV_LOG_ERROR, "Value %lf for parameter '%s' out of range\n", num, name);
> > - return NULL;
> > + return AVERROR(ERANGE);
> > }
> >
> > dst= ((uint8_t*)obj) + o->offset;
> > @@ -71,9 +73,17 @@
> > else *(AVRational*)dst= av_d2q(num*intnum/den, 1<<24);
> > break;
> > default:
> > - return NULL;
> > + return AVERROR(EINVAL);
> > }
> > - return o;
> > + return 0;
> > +}
> > +
> > +static const AVOption *av_set_number(void *obj, const char *name, double num, int den, int64_t intnum){
> > + const AVOption *o = NULL;
> > + if (av_set_number2(obj, name, num, den, intnum, &o) < 0)
> > + return NULL;
> > + else
> > + return o;
> > }
> >
> > static const double const_values[]={
>
> ok
Applied.
> > Index: ffmpeg/libavcodec/opt.h
> > ===================================================================
> > --- ffmpeg.orig/libavcodec/opt.h 2008-12-14 23:17:27.000000000 +0100
> > +++ ffmpeg/libavcodec/opt.h 2008-12-14 23:21:18.000000000 +0100
>
> > @@ -105,6 +105,14 @@
> > attribute_deprecated const AVOption *av_set_string(void *obj, const char *name, const char *val);
> >
> > /**
> > + * @return a pointer to the AVOption corresponding to the field set or
> > + * NULL if no matching AVOption exists, or if the value \p val is not
> > + * valid
> > + * @see av_set_string3()
> > + */
> > +const AVOption *av_set_string2(void *obj, const char *name, const char *val, int alloc);
> > +
>
> this should be under #ifdefs to remove it with the next major bump
So I think it should be deprecated and ifdeffed at the same time. See
the attached patches (unfortunately there is still a warning which I
think we cannot avoid).
> > +/**
> > * Sets the field of obj with the given name to value.
> > *
> > * @param[in] obj A struct whose first element is a pointer to an
> > @@ -120,14 +128,15 @@
> > * scalars or named flags separated by '+' or '-'. Prefixing a flag
> > * with '+' causes it to be set without affecting the other flags;
> > * similarly, '-' unsets a flag.
> > - * @return a pointer to the AVOption corresponding to the field set or
> > - * NULL if no matching AVOption exists, or if the value \p val is not
> > - * valid
>
> > + * @param[in,out] o_out if non-NULL put here a pointer to the AVOption
> > + * found
>
> id say this is just [out]
Fixed.
Regards.
--
FFmpeg = Fast and Frenzy Martial Purposeless Educated Gargoyle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: implement-av-set-string3.patch
Type: text/x-diff
Size: 4616 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081216/31c90197/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-opt-default.patch
Type: text/x-diff
Size: 1881 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081216/31c90197/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: use-av-set-string3.patch
Type: text/x-diff
Size: 1320 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081216/31c90197/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: replace-o2.patch
Type: text/x-diff
Size: 551 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081216/31c90197/attachment-0003.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ifdef-av-set-string12.patch
Type: text/x-diff
Size: 1508 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081216/31c90197/attachment-0004.patch>
More information about the ffmpeg-devel
mailing list