[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-05-09 20:51:02 +0200, Stefano Sabatini encoded:
> On date Saturday 2010-05-08 19:09:08 +0200, Michael Niedermayer encoded:
> > On Sat, May 08, 2010 at 01:40:29PM +0200, Stefano Sabatini wrote:
> > > 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);
> > 
> > i have no faint idea why you are attempting to implement this approuch
> > there are better choices and i dont think il will approve this due to that
> > but if you still want to pursue this its:
> > av_log(p->log_ctx, p->log_level_offset+AV_LOG_ERROR, "undefined constant or missing (\n");
> 
> Oh that was silly of me, see patch attached.
> 
> Anyway I feel somehow uhnappy about the patch, I would have still
> preferred the log_on_error flag approach, using the offset looks a bit
> awkward to me.
> 
> Regards.
> -- 
> FFmpeg = Fostering and Fundamentalist Mere Philosofic Elastic Ghost

> From 0096d85e5deec5deb7114dc5165d59e17d9e8391 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> Date: Tue, 6 Apr 2010 00:13:04 +0200
> Subject: [PATCH 2/3] Change ff_parse_expr() and ff_parse_and_eval_expr() interface.
> 
> Make them print error messages using a log_ctx rather than set a
> constant error string in the Parser. A log_level_offset is used to
> change the log_level of the log_ctx, for example to silence eventual
> errors issued when evaluating the expression.
> 
> Allow the error message to be more expressive, as it is not anymore a
> generic const char * string.
[...]

Ping.
-- 
FFmpeg = Fanciful and Foolish Minimal Pure Erratic Generator