[MPlayer-dev-eng] [PATCH] -force-key-frames option

[Reimar Döffinger]

On Sun, Oct 17, 2010 at 02:35:11PM +0200, Nicolas George wrote:
> Le sextidi 26 vendémiaire, an CCXIX, Reimar Döffinger a écrit :
> > For the current use cause even that is overkill.
> > For that you just need to check that the following char is either 0 or ','.
> > You do not need to return a length or change the function in any other way,
> > and just need to add a strchr - in the current usage (if you want to fix them)
> > to check that there is no ',', in the new usages to advance to the next time.
> 
> I am not sure what your point really is.

I don't really like that a simple "string in, value out" becomes
a founction with 3 arguments and one return value.
Maybe there really is no truly better way, but I don't
really like it, taht is all.

> -static double parse_timestring(const char *str)
> +int parse_timestring(const char *str, double *rts, char endchar)

What is rts supposed to mean?
"time", "seconds", "result" maybe?

> -  int a, b;
> +  int a, b, end = 0;
>    double d;
> -  if (sscanf(str, "%d:%d:%lf", &a, &b, &d) == 3)
> -    return 3600*a + 60*b + d;
> -  else if (sscanf(str, "%d:%lf", &a, &d) == 2)
> -    return 60*a + d;
> -  else if (sscanf(str, "%lf", &d) == 1)
> -    return d;
> -  return -1e100;
> +  if (sscanf(str, "%d:%d:%lf%n", &a, &b, &d, &end) >= 3)
> +    *rts = 3600*a + 60*b + d;
> +  else if (sscanf(str, "%d:%lf%n", &a, &d, &end) >= 2)
> +    *rts = 60*a + d;
> +  else if (sscanf(str, "%lf%n", &d, &end) >= 1)
> +    *rts = d;
> +  if (str[end] && str[end] != endchar)
> +      end = 0;

Inconsistent indentation.
Also I'd find more explicit error case helpful, e.g.:
>  else if (sscanf(str, "%lf%n", &d, &end) >= 1)
>    *rts = d;
>  else {
>    *rts = <whatever>;
>    return 0;
>  }
>  if (str[end] && str[end] != endchar)
>    return 0;
>  return end;

Renaming end to len or so might be good, too.