[FFmpeg-devel] [PATCH 2/4] avutil/opt: check offset / constness correctness

Stefano Sabatini stefasab at gmail.com
Tue Apr 16 00:25:42 CEST 2013


On date Monday 2013-04-15 23:58:49 +0200, Michael Niedermayer encoded:
> On Mon, Apr 15, 2013 at 09:49:59PM +0200, Stefano Sabatini wrote:
> > On date Saturday 2013-04-13 20:37:13 +0200, Michael Niedermayer encoded:
> > > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > > ---
> > >  libavutil/opt.c |    7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/libavutil/opt.c b/libavutil/opt.c
> > > index b4d1d67..23f57b5 100644
> > > --- a/libavutil/opt.c
> > > +++ b/libavutil/opt.c
> > > @@ -66,7 +66,7 @@ const AVOption *av_opt_next(void *obj, const AVOption *last)
> > >      AVClass *class = *(AVClass**)obj;
> > >  #if defined(HAVE_AV_CONFIG_H) && defined(ASSERT_LEVEL) && ASSERT_LEVEL >= 2
> > >      AVOption *o = last && last[1].name ? last + 1 : class->option;
> > > -    if(o) {
> > 
> > > +    if(o && (o->name || o->help || o->offset || o->min || o->max || o->unit || o->default_val.i64)) {
> > 
> > nit: if_(o
> > 
> > why these additional checks?
> 
> it checks more cases with the asserts, is that usefull? no idea

yes but what's the point of checking o->min, o->max, o->help???

> 
> 
> > 
> > >          int faulty = 0;
> > >          if (o->type == AV_OPT_TYPE_DOUBLE || o->type == AV_OPT_TYPE_FLOAT) {
> > >              faulty |= (o->min > o->default_val.dbl || o->default_val.dbl > o->max);
> > > @@ -80,6 +80,11 @@ const AVOption *av_opt_next(void *obj, const AVOption *last)
> > >              av_log(obj, AV_LOG_FATAL, "Invalid min/default/max combination in %s\n", o->name);
> > >              av_assert2(0);
> > >          }
> > > +        if (   (o->offset && o->type == AV_OPT_TYPE_CONST && o->offset != last->offset)
> > 
> > what's o->offset != last->offset good for?
> 

> There are constants that use their parent fields offset, this code is

Do you have an example to share?

Otherwise check only for o->offset && o->type == AV_OPT_TYPE_CONST
could be useful.

> checking for them that way. Yes the code is messy/ugly ...
-- 
FFmpeg = Furious & Formidable Mega Pacific Elected Gorilla


More information about the ffmpeg-devel mailing list