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

[Michael Niedermayer]

Hi

On Sun, Sep 24, 2006 at 01:50:54PM +0200, Panagiotis Issaris wrote:
> Hi,
> 
> On za, 2006-09-23 at 12:33 +0200, Panagiotis Issaris wrote:
> > 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.
> > [...]
> 
> Okay, after tracing ff_eval for too many hours, the bug in eval.c is
> fixed now, so no "if ... returns" are necessary anymore.
> 
> So, this is an updated version of the older patch, with these checks
> removed. Regression tests succeed.
> 
> These all work now:
> ffmpeg -i src.avi -b 100k     dst.avi
> ffmpeg -i src.avi -b 8*9k     dst.avi
> ffmpeg -i src.avi -b default  dst.avi
> ffmpeg -i src.avi -b QP2LAMDBA*10k   dst.avi 
> 
> The last one is not useful for -b, but will be for the next patches in
> my queue (the ones for migrating lmax and others).
> 
> And this fails cleanly (no segfaulting or anything):
> ffmpeg -i src.avi -b takis    dst.avi
> 
>  eval.c |   62 +++++++++++++++++++++++++++++++++++++++++++--
>  opt.c  |   89 +++++++++++++----------------------------------------------------
>  2 files changed, 78 insertions(+), 73 deletions(-)

looks ok

[...]
-- 
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