[Ffmpeg-devel] [PATCH] Use ff_eval for AVOption parsing

[Michael Niedermayer]

Hi

On Sat, Sep 23, 2006 at 12:33:36PM +0200, Panagiotis Issaris wrote:
> Hi,
> 
> The attached patch changes the AVOption parsing code to use ff_eval
> instead of av_strtod. As ff_eval uses av_strtod, the postfix ('k',
> 'M', ...) functionality is not lost.
> 
> It does this, by moving av_strtod to eval.c as this is the only place
> where it is used now. Furthermore, one parser log message has been
> decreased in severness as it kept complaining about missing '(' in
> "bitexact", "fastint", etc. which are all parsed by eval now, and aren't
> errors. Parsing failures were/are indicated by NAN (the infrastructure
> was partly there, but they returned 0 not NAN). For the moment I've
> only added the QP2LAMBDA option to the eval's constants, so that
> ffmpeg -i src.avi -lmax 10*QP2LAMBDA dst.avi
> would work. After I sent a patch to migrate lmax from OptionDef to
> AVOptions that is.
> 
> One more thing is that I had to add four "if isnan returns" in the code,
> because parsing would fail (segfault in fact) if parsing continued when
> using "max", "default" or "min" as options:
> ffmpeg -i src.avi -b max dst.avi
> 
> To be able to add these escapes, I introduced a temporary variable in
> two out of four cases.
> 
> I am not sure I fully like this patch myself:
> 1. because of it is quiet intrusive imho, although splitting the patch
> would result in broken code for most parts of it (except for the
> escapes)
> 2. because of the added escapes, more precisely, because I do not fully
> (in detail) understand the segfaults. Initially I thought it was because
> of the continued operations on a double containing NAN, but it seemed
> impossible that such would result in a _segfault_. I would think it
> would just always result in NAN. Then I thought it might be because the
> parser increases p->s++, and in some strange way there would be a
> dereference somewhere causing the segfault. Anyway, I did not find the
> exact reason for the segfault, but I am sending it anyway as I'm off for
> lunch and shopping and hoped for some feedback on this patch.

iam against changes which by some mysterious reason fix a segfault ...
maybe gcc misscompiles something? does -O0 or gcc 2.95 fix that segfault?
what does valgrind say about it? and if all fails adding lots of printf/
av_log to see exactly whats happening before the segfault seems to be
the obvious next step
also look at the warnings gcc outputs when compiling eval.c & opt.c

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

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is