[Ffmpeg-devel] [PATCH] Add kibi, mibi and gibi support

[Panagiotis Issaris]

Hi,

I had already written the attached patch before your si_prefixes commit.
I'll read the related code and see how it fits in with this.

I'm sending the attached patch anyways because the current code
contained (at least) two bugs.

On za, 2006-09-16 at 17:24 +0200, Michael Niedermayer wrote: 
> [...]
> > The problem was that av_strtod() was not correctly interpreting strings
> > which contained no number. If the first call to strtod() indicated that
> > there was no number, it should have returned immediately. But it didn't
> > which caused it to interpret the first 'i' of "-dct int" as some kind of
> > number. This was also due to the previous bug, which allowed using "i"
> > without an earlier "k", "M" or "G" which makes no sense at all. Only
> > fixing this behavior would ofcourse be wrong, as any other flag value,
> > starting with a B would cause yet another option interpretation problem.
>[...] 
> >  static double av_strtod(const char *name, char **tail) {
> >      double d;
> > +    int p = 0;
> >      d= strtod(name, tail);
> > +    if (*tail<=name)
> > +        return d;
> > +
> >      if(*tail>name && (**tail=='k')) {
> > -        d*=1000;
> > +        p = 1;
> >          (*tail)++;
> >      }
> >      else if(*tail && (**tail=='M')) {
> 
> why the difference between *tail>name and *tail in the 2 if() ?
> it alsi seems that with the if (*tail<=name) return d; the *tail checks
> are unneeded? or other way around the *tail if changed to *tail>name
> would make the if (*tail<=name) return d unneeded ?
Yes, you are right. The "if (*tail<=name) return d;" was added in the
third patch, as I noticed that it would be best to return immediately
from the function if parsing failed (by strtod). Otherwise every other
possible start of a postfix (such as 'B' in this case), would need to
check for this possible failure. Not that there would be many other
possible starts of such a postfix, but still, I think one check is
better then two if they are equivalent.

The bug appeared when one tried to use the "dct" parameter with value
"int". avctx->dct_algo would be set to zero instead of 2 because of a
failure in the av_strtod() function.

Furthermore, the function did not mimic strtod correctly, as in the
latter, it is allowed to pass in NULL for the second parameter.

I've attached a patch which fixes both issue.


> > -        d*=1000000;
> > +        p = 2;
> >          (*tail)++;
> >      }
> >      else if(*tail && (**tail=='G')) {
> > -        d*=1000000000;
> > +        p = 3;
> >          (*tail)++;
> >      }
> > +    if (p>0)
> 
> the (*tail)++ can be moved here, that would remove 2 of them
The attached patch also cleans up the redundant checks.

> [...]


 opt.c |   48 +++++++++++++++++++++++++++++++-----------------
 1 files changed, 31 insertions(+), 17 deletions(-)


With friendly regards,
Takis
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pi-20060917T112338-ffmpeg-av_strtod_fixes.diff
Type: text/x-patch
Size: 1703 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20060917/032c1fda/attachment.bin>