[FFmpeg-devel] [PATCH] -fs parameter human friendly

[Stefano Sabatini]

On date Wednesday 2008-08-20 01:43:17 +0200, Michael Niedermayer encoded:
> On Mon, Aug 11, 2008 at 11:43:53PM +0200, Stefano Sabatini wrote:
[...]

Michael:
> i wonder if it would make sense to just use the whole expression evaluator
> instead of av_strtod().

I think this could be a very good idea, anyway maybe is still a good
idea to export av_strtod(), skip review of the following patch
otherwise.

> > Index: libavcodec/eval.h
> > ===================================================================
> > --- libavcodec/eval.h	(revision 14690)
> > +++ libavcodec/eval.h	(working copy)
> > @@ -84,4 +84,18 @@
> >  double ff_parse_eval(AVEvalExpr * e, double *const_value, void *opaque);
> >  void ff_eval_free(AVEvalExpr * e);
> >  
> > +/**
> > + * Parses the value in \p name and returns its value as a double.
> > + *
> > + * @param name a string representing a number, may contain the the
> 
> the the?

Fixed.
  
> > + * 'k', 'M', 'G', 'ki', 'Mi', 'Gi' and 'B' postfixes. This allows
> > + * using f.e. kB, MiB, G and B as a postfix.
> 
> <list> postfixes. This allows using f.e <list> postfix. 
> what is this supposed to mean?

Reworded, now it should be more clear.
 
> > + * Values are expressed in
> > + * bits, unless the 'B' postfix is used, in this case the values are
> > + * assumed to be in bytes.
> 
> what?

As above.

> > + * @param tail if non-NULL put at this address the pointer to the
> > + * character just after the part of the string successfully parsed
> 
> parse error

Check the new explanation, I took it from a comment in the
implementation, I don't think parse error is very expressive.

> > + * @return the value of the parsed string as a numer of bits
> 
> numer!? bits?

Fixed the typo, as for the return value, yes it is expressed in
bits.

> > + */
> > +double av_strtod(const char *name, char **tail);
> > +
> >  #endif /* FFMPEG_EVAL_H */
> > Index: libavcodec/eval.c
> > ===================================================================
> > --- libavcodec/eval.c	(revision 14690)
> > +++ libavcodec/eval.c	(working copy)
> > @@ -82,11 +82,7 @@
> >      ['Y'-'E']=  24,
> >  };
> >  
> > -/** strtod() function extended with 'k', 'M', 'G', 'ki', 'Mi', 'Gi' and 'B'
> > - * postfixes.  This allows using f.e. kB, MiB, G and B as a postfix. This
> > - * function assumes that the unit of numbers is bits not bytes.
> > - */
> > -static double av_strtod(const char *name, char **tail) {
> 
> why didnt you just copy this?  :(((((((((((((((((
> 
> 
> > +double av_strtod(const char *name, char **tail) {
> >      double d;
> >      char *next;
> >      d = strtod(name, &next);
> 
> > Index: cmdutils.c
> > ===================================================================
> > --- cmdutils.c	(revision 14695)
> > +++ cmdutils.c	(working copy)
> > @@ -29,6 +29,7 @@
> >  #include "libavfilter/avfilter.h"
> >  #include "libavdevice/avdevice.h"
> >  #include "libavutil/avstring.h"
> > +#include "libavcodec/eval.h"
> >  #include "cmdutils.h"
> >  #include "version.h"
> >  #ifdef CONFIG_NETWORK
> > @@ -42,7 +43,7 @@
> >  {
> >      char *tail;
> >      const char *error;
> > -    double d = strtod(numstr, &tail);
> > +    double d = av_strtod(numstr, &tail);
> >      if (*tail)
> >          error= "Expected number for %s but found: %s\n";
> >      else if (d < min || d > max)
> 
> i wonder if it would make sense to just use the whole expression evaluator
> instead of av_strtod().
> Actually i think this would be a great idea

I'll explore that possibility tomorrow.

Good night.
-- 
FFmpeg = Friendly and Funny Meaningless Portable Egregious Guru
-------------- next part --------------
A non-text attachment was scrubbed...
Name: export-av-strtod-01.patch
Type: text/x-diff
Size: 1536 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080820/05881e44/attachment.patch>