[FFmpeg-devel] [PATCH] Make ffmpeg use parse_number_or_die instead of atoi

[Stefano Sabatini]

On date Thursday 2008-04-03 01:11:46 +0200, Michael Niedermayer encoded:
> On Sun, Mar 30, 2008 at 11:58:07PM +0200, Stefano Sabatini wrote:
> > On date Friday 2008-03-21 01:08:34 +0100, Stefano Sabatini encoded:
> > > On date Friday 2008-03-07 01:34:07 +0100, Stefano Sabatini encoded:
> > > > Hi,
> > > > 
> > > > as in $subject,
> > > > 
> > > > improves error management, diagnostic and reduces code size.
> > > > 
> > > > Note:
> > > > 
> > > > -    video_qscale = atof(arg);
> > > > +    video_qscale = parse_number_or_die("qscale", arg, OPT_FLOAT, -1.0/0.0, 1.0/0.0);
> > > >      if (video_qscale <= 0 ||
> > > >          video_qscale > 255) {
> > > >          fprintf(stderr, "qscale must be > 0.0 and <= 255\n");
> > > > 
> > > > I didn't do:
> > > > video_qscale = parse_number_or_die("qscale", arg, OPT_FLOAT, nextafter(0,1), 255)
> > > > 
> > > > because the resulting message for an error of the kind:
> > > > ffmpeg -qscale 0
> > > > 
> > > > would result as: 
> > > > "The value for qscale was 0 which is not within 0 - 255"
> > > > 
> > > > which is indeed quite confusing. Anyway the code for opt_qscale is
> > > > going to be removed soon with the libavilter inclusion.
> > > [...]
> > > 
> > > Two weeks interval passed... ping!
> > > 
> > > Please let me know if you're interested in this.
> > [...]
> > 
> > Hi, new ping (sorry for insisting, yes I know there are other
> > priorities in this period, just resending in the case it got missed).
> > 
> > If you prefer I can split this patch into many pieces.
> > 
> > Here it is an example of the new implemented behaviour:
> > ffmpeg -qscale foo
> > [...]
> > Expected number for qscale but found: foo
> > 
> > ffmpeg -qdiff -1
> > [...]
> > The value for qdiff was -1 which is not within 1.000000 - 31.000000
> > 
> > ffmpeg -strict foo
> > [...]
> > Expected number for strict but found: foo
> > 
> > ffmpeg -top -2
> > [...]
> > The value for top was -2 which is not within -1.000000 - 1.000000
> > 
> > ffmpeg -pass foo
> > Expected number for pass but found: foo
> > 
> > ffmpeg -pass -2
> > The value for pass was -2 which is not within 1.000000 - 2.000000
> > 
> > ffmpeg -v foo
> > Expected number for v but found: foo
> > 
> > ffmpeg -croptop -1
> > [...]
> > The value for croptop was -1 which is not within 0.000000 - 2147483647.000000
> > 
> > Next step is to pass context to the various opt_ function like in
> > r12559.
[...]
> 
> > Index: ffmpeg.c
> > ===================================================================
> > --- ffmpeg.c	(revision 12631)
> > +++ ffmpeg.c	(working copy)
> > @@ -2173,12 +2173,12 @@
> >  
> >  static void opt_me_threshold(const char *arg)
> >  {
> > -    me_threshold = atoi(arg);
> > +    me_threshold = parse_number_or_die("me_threshold", arg, OPT_INT64, INT_MIN, INT_MAX);
> >  }
> >  
> 
> >  static void opt_verbose(const char *arg)
> >  {
> > -    verbose = atoi(arg);
> > +    verbose = parse_number_or_die("v", arg, OPT_INT64, 0, INT_MAX);
> >      av_log_set_level(verbose);
> >  }
> >  
> 
> and negative ones ...

Yep, I missed that.
Is it OK to use the AV_LOG_QUIET and AV_LOG_DEBUG macros to specify constraints?
 
> anyway, this patch could be split

There is a total of 18 functions changed, opt_pad* and opt_crop* may
be grouped, but then there would be an amount of other 10 patches, I
think a single patch maybe is easier to review and revert in case of
problems, and is consistent with r12366.

> also many of the changes are to code which will be removed. Eithe because it
> should be handled through AVOption or through AVFilter

Yes I know, waiting for that the current patch still improves the
situation.

Best regards.
-- 
Stefano Sabatini
Linux user number 337176 (see http://counter.li.org)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: make-ffmpeg-use-parse-number-or-die-04.patch
Type: text/x-diff
Size: 5985 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080403/09d52ff4/attachment.patch>