[FFmpeg-devel] [PATCH] Remove libavcodec/opt.c:set_all_opt()

[Stefano Sabatini]

On date Sunday 2008-12-07 19:30:44 +0100, Stefano Sabatini encoded:
> Hi all,
> 
> currently av_set_string2(), if fed with the name of a named constant
> or flag, will look for all the non-constant options with the same unit
> and will set their value to the value of the named constant.
> 
> This permits for example to do tricks of the kind:
> 
> ffmpeg -mv4 foo -> ffmpeg -flags2 +mv4
> ffmpeg -zero bar -> ffmpeg -me_method zero
> 
> The problem with this is that this "feature" isn't reliable, since it
> only works with the *first* constant found, while different named
> constants may have the same name, so the user has to know the options
> array in order to predict the behaviour.
> 
> For example without knowing the options order "ffmpeg -auto 1" may mean:
> 1) ffmpeg -idct auto
> 2) ffmpeg -dct auto
> 3) ffmpeg -aa auto
> 
> Even worse, something like:
> ffmpeg -dct 1
> 
> may be interpreted both as:
> ffmpeg -dct 1
> or
> ffmpeg -cmp_func dct
> 
> (BTW the current code will broke if a named constant is defined
> *before* a parameter with the same name.)
> 
> I propose to remove this feature, this also makes the code smaller and
> simpler.
> 
> Regards.
[...]
> Index: libavcodec/opt.c
> ===================================================================
> --- libavcodec/opt.c	(revision 16030)
> +++ libavcodec/opt.c	(working copy)
> @@ -76,24 +76,6 @@
>      return o;
>  }
>  
> -static const AVOption *set_all_opt(void *v, const char *unit, double d){
> -    AVClass *c= *(AVClass**)v; //FIXME silly way of storing AVClass
> -    const AVOption *o= c->option;
> -    const AVOption *ret=NULL;
> -
> -    for(;o && o->name; o++){
> -        if(o->type != FF_OPT_TYPE_CONST && o->unit && !strcmp(o->unit, unit)){
> -            double tmp= d;
> -            if(o->type == FF_OPT_TYPE_FLAGS)
> -                tmp= av_get_int(v, o->name, NULL) | (int64_t)d;
> -
> -            av_set_number(v, o->name, tmp, 1, 1);
> -            ret= o;
> -        }
> -    }
> -    return ret;
> -}
> -
>  static const double const_values[]={
>      M_PI,
>      M_E,
> @@ -117,10 +99,7 @@
>  
>  const AVOption *av_set_string2(void *obj, const char *name, const char *val, int alloc){
>      const AVOption *o= av_find_opt(obj, name, NULL, 0, 0);
> -    if(o && o->offset==0 && o->type == FF_OPT_TYPE_CONST && o->unit){
> -        return set_all_opt(obj, o->unit, o->default_val);
> -    }
> -    if(!o || !val || o->offset<=0)
> +    if((o && o->offset<=0) || !o || !val)
>          return NULL;
>      if(o->type == FF_OPT_TYPE_BINARY){
>          uint8_t **dst = (uint8_t **)(((uint8_t*)obj) + o->offset);

Ping.

Regards.
-- 
FFmpeg = Faboulous and Fostering Minimalistic Powerful Ecumenical Gadget