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

[Michael Niedermayer]

On Thu, May 13, 2010 at 12:39:05AM +0200, Stefano Sabatini wrote:
> 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.

what the patch does is not in line with the recent AVClass changes

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100514/ec74d602/attachment.pgp>