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

[Stefano Sabatini]

On date Sunday 2010-04-25 02:18:19 +0200, Stefano Sabatini encoded:
> 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
[...] 
> > > > > Other ideas:
> > > > > // write the error message to the user-provided error buffer, allows
> > > > > // ad-hoc context-sensitive messages
> > > > > int av_parse_expr(..., const char *error, size_t error_size);
> > > > > 
> > > > > or maybe we could just provide more than one interface:
> > > > > av_parse_expr (..., void *log_ctx);
> > > > > av_parse_expr2(..., const char *error, size_t error_size);
> > > > > 
> > > > > Please tell me what you consider acceptable.
> > > > 
> > > > i dont like multiple interfaces, things are complex enough already
> > > 
> > > So what about the other solution:
> > > int av_parse_expr(..., const char *error, size_t error_size);
> > > ?
> > 
> > this would not allow debug and warnings to be printed
> > 
> > av_parse_expr(..., int log_level_offset)
> > would still be the clearly better choice
> 
> I'll go for this solution then.

This has currently different problems, when I do:

av_log(p->log_ctx, AV_LOG_ERROR, "undefined constant or missing (\n");

In order to change the log_level for this message I need to access the
global log level like this:

av_log_set_level(p->log_ctx, AV_LOG_ERROR + av_clip(p->log_level_offset >> 3, AV_LOG_QUIET, AV_LOG_DEBUG);
av_log(p->log_ctx, AV_LOG_ERROR, "undefined constant or missing (\n");
av_log_get_level(p->log_ctx, AV_LOG_ERROR - av_clip(p->log_level_offset >> 3, AV_LOG_QUIET, AV_LOG_DEBUG);

av_log_level being global may be accessed by some other threads during
the execution of this code, thus changing the global log level for
other threads during the execution of the code above.

My suggestion is again to use a simple flag log_error, which
would enable/disable error log printing:

AVExpr *ff_parse_expr(const char *s, const char * const *const_name,
               double (* const *func1)(void *, double), const char * const *func1_name,
               double (* const *func2)(void *, double, double), const char * const *func2_name,
               void *log_ctx, int log_on_error);

Then we may change this to something more flexible when we'll move
opt.c to lavu.

Regards.
-- 
FFmpeg = Foolish Fast Mystic Powerful Efficient Guide