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

[Michael Niedermayer]

On Tue, May 18, 2010 at 10:03:41PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2010-05-18 21:29:56 +0200, Michael Niedermayer encoded:
> > On Tue, May 18, 2010 at 08:38:23PM +0200, Stefano Sabatini wrote:
> > > On date Tuesday 2010-05-18 17:27:37 +0200, Michael Niedermayer encoded:
> > > > On Tue, May 18, 2010 at 12:56:53AM +0200, Stefano Sabatini wrote:
> > > > [...]
> > > > >  opt.c |    2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > > 11b07e9a34d65b8e88f47b5fd2d9af456258620b  0002-Silence-logging-when-evaluating-the-expression-for-b.patch
> > > > > >From c18ac9ee11ca03ab36e5acf2b72ecfc537caaced Mon Sep 17 00:00:00 2001
> > > > > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> > > > > Date: Tue, 18 May 2010 00:31:01 +0200
> > > > > Subject: [PATCH 2/4] Silence logging when evaluating the expression for buf in
> > > > >  av_set_string3().
> > > > > 
> > > > > ---
> > > > >  libavcodec/opt.c |    2 ++
> > > > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > > > > 
> > > > > diff --git a/libavcodec/opt.c b/libavcodec/opt.c
> > > > > index 5b56bc8..844c0ec 100644
> > > > > --- a/libavcodec/opt.c
> > > > > +++ b/libavcodec/opt.c
> > > > > @@ -155,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;
> > > > >  
> > > > > +            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);
> > > > 
> > > > not ok, obj is not owned by this code. Other code might via a seperate
> > > > thread print via obj at the same time.
> > > 
> > > So please suggest what to do.
> > 
> > the problem appears to me that eval prints using an outside context and
> > not its own context.
> > I do not know if changing this would lead to other problems or how much
> > work that would be
> >
> > one way to fix this i see would be to seperate context allocation
> > from the rest.
> 
> I don't think it is a good idea to have a specific context for eval if
> that's what you mean. 

i meant that eval should pass its own context to av_log not the parent context


> Also please let's try not to waste too much
> time/brain power on this, the feature you're requesting is something
> which is currently required just once in the code, and I'm convinced
> that use should be avoided even in that case.

if you want eval to be moved to public api then it has to be working
and while none of the cosmetics, parameter reordering and that prevented
it from becoming public.
This does. eval must integrate cleanly into code using it and that may or
may not want to show error messages to the user depending on how it is used.
This problem is not specific to eval nor this specific situation other util
code too can have error conditions and can want to print a human readable
description of the condition. Its the same as with eval an error of such
util may be fatal to the user application or it may be minor debug info
and the app would just try another variant.
Its like fopen() failing can if the file in question is essential be
the end of an application or it can be just the app probing a few directories
where failing before the very last means nothing

if you have another idea thats welcome but not solving it is not

> 
> > We could also (thats independant) ff_parse_and_eval_expr() to take a
> > log_level_offset argument

this too requires eval to have its own av_log() context

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- 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/20100519/fc76a0bf/attachment.pgp>