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

[Michael Niedermayer]

On Mon, Apr 19, 2010 at 12:52:58AM +0200, Stefano Sabatini wrote:
> On date Sunday 2010-04-18 00:07:25 +0200, Michael Niedermayer encoded:
> > On Mon, Apr 12, 2010 at 11:37:01PM +0200, Stefano Sabatini wrote:
> > > Make them print error messages on the log using a log_ctx rather than
> > > set a constant error string in the Parser.
> > > 
> > > Simplify the interface, and allow the error message to be more
> > > expressive, as it is not anymore a generic const char * string.
> > > ---
> > >  libavcodec/eval.c        |   18 +++++++++---------
> > >  libavcodec/eval.h        |    8 ++++----
> > >  libavcodec/opt.c         |    6 ++----
> > >  libavcodec/ratecontrol.c |    5 ++---
> > >  4 files changed, 17 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/libavcodec/eval.c b/libavcodec/eval.c
> > > index 0b2aed9..0149a55 100644
> > > --- a/libavcodec/eval.c
> > > +++ b/libavcodec/eval.c
> > > @@ -39,7 +39,7 @@ typedef struct Parser{
> > >      double (* const *func2)(void *, double a, double b); // NULL terminated
> > >      const char * const *func2_name;          // NULL terminated
> > >      void *opaque;
> > > -    const char **error;
> > > +    void *log_ctx;
> > >  #define VARS 10
> > >      double var[VARS];
> > >  } Parser;
> > > @@ -201,7 +201,7 @@ static AVExpr * parse_primary(Parser *p) {
> > >  
> > >      p->s= strchr(p->s, '(');
> > >      if(p->s==NULL){
> > > -        *p->error = "undefined constant or missing (";
> > > +        av_log(p->log_ctx, AV_LOG_ERROR, "undefined constant or missing (\n");
> > 
> > this patch (not this change specifically) causes problems
> > one no longer can evaluate an expression without spamming the user
> > thats all fine if what is evaluated is a user supplied string, for other
> > cases though it is not appropriate.
> > If av_log() is used then we need a way to put a limit on the error level
> > an error for evaluation may or may not be an error for the calling code.
> > maybe we can just add an 8 entry array to AVClass and use that to
> > remap the error levels. With that eval would in this case need to
> > be run on a custom object with an AVClass that silences it.
> > Thats a quite generic solution (that likely will come in handy
> > for other code outside evel) but maybe its not the simplest solution
> > 
> > the next obvious alternative is to leave the returning of error messages
> > as it is.
> 
> I see the problem but maybe the remap solution is overkill, for
> example that would require a mapping and a reverse mapping in the
> current code in lavc which uses it.

where do you see a need to reverse map?


> 
> 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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- 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/20100419/af5d7004/attachment.pgp>