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

Stefano Sabatini stefano.sabatini-lala
Tue May 18 00:56:53 CEST 2010


On date Monday 2010-05-17 00:53:16 +0200, Michael Niedermayer encoded:
> On Sun, May 16, 2010 at 02:42:00AM +0200, Stefano Sabatini wrote:
> > On date Friday 2010-05-14 00:22:56 +0200, Michael Niedermayer encoded:
> > > On Thu, May 13, 2010 at 12:39:05AM +0200, Stefano Sabatini wrote:
> > [...]
> > > > > 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
> > 
> > Updated.
> > 
> > Two questions:
> > Is it necessary to bump minor when adding an option to AVCodecContext?
> 
> its probably a good idea
> 
> 
> > 
> > Is OK to bump micro when changing the ff_parse_expr() and
> > ff_parse_and_eval_expr()? (see the thread "[RFC] Updating internl API
> > policy").
> > 
> > Regards.
> > -- 
> > FFmpeg = Fantastic Fantastic Miracolous Porno Evanescent Game
> 
> >  avcodec.h |    2 ++
> >  options.c |    3 ++-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> > 36819623adda80b556a3af3bc38843fadfd0f9e5  0002-Add-log_level_offset-to-AVCodecContext.patch
> > >From 32e45b3c8e98924373eb9dbb69881799cfe80393 Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> > Date: Sun, 16 May 2010 02:00:00 +0200
> > Subject: [PATCH 2/5] Add log_level_offset to AVCodecContext.
> 
> looks ok

Applied.

[...]
> > Subject: [PATCH 3/5] 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.
> > 
> > Allow the error message to be more expressive, as it is not anymore a
> > generic const char * string, and reference the provided input.
> > ---
> >  libavcodec/eval.c        |   20 +++++++++++---------
> >  libavcodec/eval.h        |    8 ++++----
> >  libavcodec/opt.c         |    8 ++++----
> >  libavcodec/ratecontrol.c |    6 +++---
> >  4 files changed, 22 insertions(+), 20 deletions(-)
[...]
> > --- a/libavcodec/opt.c
> > +++ b/libavcodec/opt.c
> > @@ -147,7 +147,6 @@ int av_set_string3(void *obj, const char *name, const char *val, int alloc, cons
> >              char buf[256];
> >              int cmd=0;
> >              double d;
> > -            const char *error = NULL;
> >  
> >              if(*val == '+' || *val == '-')
> >                  cmd= *(val++);
> > @@ -156,7 +155,9 @@ int av_set_string3(void *obj, const char *name, const char *val, int alloc, cons
> >                  buf[i]= val[i];
> >              buf[i]=0;
> >  
> > -            d = ff_parse_and_eval_expr(buf, const_values, const_names, NULL, NULL, NULL, NULL, NULL, &error);
> > +            av_set_int(obj, "log_level_offset", AV_LOG_DEBUG - AV_LOG_QUIET);
> > +            d = ff_parse_and_eval_expr(buf, const_names, const_values, NULL, NULL, NULL, NULL, NULL, obj);
> > +            av_set_int(obj, "log_level_offset", 0);
> >              if(isnan(d)) {
> >                  const AVOption *o_named= av_find_opt(obj, buf, o->unit, 0, 0);
> >                  if(o_named && o_named->type == FF_OPT_TYPE_CONST)
> 
> that could be a seperate patch

Not sure it's a good idea since without this it's causing a regression
(many error messages which weren't showed before). Also I plan to
remove that anyway.

Anyway split.
-- 
FFmpeg = Fundamentalist and Frightening Magic Puristic Erroneous Gargoyle



More information about the ffmpeg-devel mailing list