On Sun, 22 May 2011, Justin Ruggles wrote:
> On 05/22/2011 01:00 PM, Martin Storsjö wrote:
>
> > On Sun, 22 May 2011, Justin Ruggles wrote:
> >
> >> On 05/20/2011 07:59 AM, Martin Storsjö wrote:
> >>
> >>> ---
> >>> libavutil/opt.c | 3 ++-
> >>> 1 files changed, 2 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/libavutil/opt.c b/libavutil/opt.c
> >>> index 57e3248..9ff0a9a 100644
> >>> --- a/libavutil/opt.c
> >>> +++ b/libavutil/opt.c
> >>> @@ -262,7 +262,7 @@ static int av_get_number(void *obj, const char *name,
> >>> const AVOption **o_out, do
> >>> {
> >>> const AVOption *o= av_find_opt(obj, name, NULL, 0, 0);
> >>> void *dst;
> >>> - if (!o || o->offset<=0)
> >>> + if (!o || (o->offset<=0 && o->type != FF_OPT_TYPE_CONST))
> >>> goto error;
> >>>
> >>> dst= ((uint8_t*)obj) + o->offset;
> >>> @@ -278,6 +278,7 @@ static int av_get_number(void *obj, const char *name,
> >>> const AVOption **o_out, do
> >>> case FF_OPT_TYPE_RATIONAL: *intnum= ((AVRational*)dst)->num;
> >>> *den = ((AVRational*)dst)->den;
> >>> return 0;
> >>> + case FF_OPT_TYPE_CONST: *intnum= o->default_val.dbl;return 0;
> >>> }
> >>> error:
> >>> *den=*intnum=0;
> >>
> >>
> >> Does this fix something? If not, does it make some user-level task easier?
> >
> > It makes it possible to check whether a particular flag is set, without
> > knowing the exact value of the flag (which after this patch could be
> > queried with this function). You can set such flags with e.g.
> > av_set_string3(ctx, "flags", "flag1+flag2") - but given ctx, you can't
> > query for what constant "flag1" corresponds to, making it impossible to
> > opaquely inspect whether that flag is set in the flags field.
>
>
> That makes sense.
>
> One thing I'm worried about is duplicate names. The constant options
> don't have to be unique. See "auto" in the libavcodec options for
> example. The av_find_opt() call in av_get_number() doesn't specify the
> unit parameter, which would distinguish between duplicate names.
Good point. If there's a risk for that case, I guess it's better to use
av_find_opt straight away instead. (I didn't think of that option
initially, but that would work without any modifications.)
In any case, do you think this patch is worth applying?
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
