[FFmpeg-devel] [PATCH 1/5] Change eval internal functions, ff_parse_expr() and ff_parse_and_eval_expr() interface.

[Stefano Sabatini]

On date Wednesday 2010-04-28 22:37:22 +0200, Michael Niedermayer encoded:
> On Sun, Apr 25, 2010 at 02:18:19AM +0200, Stefano Sabatini wrote:
> > On date Wednesday 2010-04-21 23:36:05 +0200, Michael Niedermayer encoded:
> > > On Wed, Apr 21, 2010 at 02:22:58AM +0200, Stefano Sabatini wrote:
> > > > On date Tuesday 2010-04-20 02:37:55 +0200, Michael Niedermayer encoded:
> > > > > On Tue, Apr 20, 2010 at 01:18:01AM +0200, Stefano Sabatini wrote:
> > > > [...]
> > > > > > > > A possible solution would be to add a simple flag log_error to
> > > > > > > > ff_parse_expr()/ff_parse_and_eval_expr().
> > > > > > > 
> > > > > > > iam against this, its unreasonable unflexible, even a offset to the
> > > > > > > log level would be better
> > > > > > 
> > > > > > Problem with the map is that we don't have just 8 values for log, it
> > > > > > is an int ranging from -8 to 48, that's why I can't grasp the 8 entry
> > > > > > array map idea, please elaborate.
> > > > > 
> > > > > right, sorry, i meant something like:
> > > > > level += map[av_clip(level>>3,...)]
> > > > 
> > > > If I understood it correctly this would mean to set an ad-hoc log
> > > > level for each log context, right?
> > > 
> > > it would allow to compensate the log level per context.
> > > without it or a equialent system av_log() can not be used in generic utility
> > > code ever. This also would mean all uses in libavutil would have to be
> > > removed
> > > the global log level, that is for example "print errors" breaks down once
> > > you go down into a function of some generic util, its error might not be
> > > an error for the main application at all or it might be it all depends on
> > > what the failure of such util means in each specific instance to the main
> > > application.
> > 
> > My idea is to implement something of the kind:
> > 
> > static inline int av_log_get_level_offset(void *log_ctx)
> > {
> >     int *log_level = log_ctx ? log_ctx+sizeof(AVClass *) : NULL;
> >     
> >     return log_level ? *log_level : 0;
> > }
> > 
> > static inline int av_log_set_level_offset(void *log_ctx) { ... };
> > 
> > But this would require to put ant int log_level_offset in the contexts
> > just after the class pointers:
> 
> You are confused
> we can add fields to structures since ages (at the end) this doesnt break
> ABI.
> We can access them through AVOptions in a generic way 
> (thats what AVOptions are there for)

The idea crossed my mind, but I considered it a little expensive
efficiency-wise, having to make av_get_int() for every av_log() is
going to be a little slow, av_log() is supposed to be fast, anyway I
won't object if you like this approach.

Also in this case we need some way to tell to AVOptions which are the
"reserved" options, an idea could be to prefix with "_" the options
which have a special meaning, e.g. we could have _log_level_offset.
 
> The only thing you need to do is move AVOptions from lavc to lavu

opt.c depends on eval.c, so this will be done after moving eval API to
lavu.

Regards.
-- 
FFmpeg = Frenzy Fostering Magnificient Puristic Elastic God