[FFmpeg-devel] [PATCH] Implement options parsing for avfilter filters

[Stefano Sabatini]

On date Monday 2009-04-20 04:57:08 +0200, Michael Niedermayer encoded:
> On Mon, Apr 20, 2009 at 01:26:57AM +0200, Stefano Sabatini wrote:
[...]
> > Patch updated (other ones are only as reference/example/backup).
> [...]
> > +char *av_get_token(const char **buf, const char *term)
> > +{
> > +    char *out = av_malloc(strlen(*buf) + 1);
> > +    char *ret = out;
> > +    const char *p = *buf;
> > +    p += strspn(p, WHITESPACES);
> > +
> > +    do {
> > +        char c = *p++;
> > +        switch (c) {
> > +        case '\\':
> > +            *out++ = *p++;
> > +            break;
> > +        case '\'':
> > +            while(*p && *p != '\'')
> > +                *out++ = *p++;
> > +            if(*p)
> > +                p++;
> > +            break;
> > +        default:
> > +            if (!c || strspn((p-1), term))
> > +                *out++ = 0;
> > +            else
> > +                *out++ = c;
> > +        }
> > +    } while(out[-1]);
> > +
> > +    p--;
> > +    p += strspn(p, WHITESPACES);
> 
> is that supposed to remove trailing whitespaces?
> because i dont see how that could work

Should be fixed now.

> > +    *buf = p;
> > +
> > +    return ret;
> > +}
> > +
> > +/**
> > + * Stores the value in the field in ctx that is named like key.
> > + * ctx must be a AVClass context, storing is done using AVOptions.
> > + * Key and value are separated by the key_val_sep chars, pairs are
> > + * separated by the opt_sep chars.
> > + *
> > + * @param buf buffer to parse, buf will be updated to point to the
> > + * character just after the parsed string
> 
> > + * @return a non negative value if successfull, a negative value
> > + * otherwise
> 
> could you write this in a way that does not leave 0 ambigous

Strictly speaking that was not ambiguous, but I see your point
replaced by ">=0".
 
> > + */
> > +static int parse_key_value_pair(void *ctx, const char **buf,
> > +                                const char *key_val_sep, const char *opt_sep)
> > +{
> > +    char *key = av_get_token(buf, key_val_sep);
> > +    char *val = NULL;
> > +    const AVOption *o = NULL;
> > +    int ret;
> > +
> > +    if(**buf == '=') {
> 
> '=' ?
> 
> [...]
> > +int av_set_options_string(void *ctx, const char *opts,
> > +                          const char *key_val_sep, const char *opt_sep)
> > +{
> > +    char c;
> > +
> > +    do {
> > +        opts += strspn(opts, WHITESPACES);
> > +
> > +        if (parse_key_value_pair(ctx, &opts, key_val_sep, opt_sep) < 0)
> > +            return -1;
> > +
> > +        opts += strspn(opts, WHITESPACES);
> > +        c = *opts++;
> > +    } while (c == ':' || c);
> 
> ':'
> ?

Fixed.

> [...]
> > + * opts contains a list of key/value pairs separated. 
> 
> hmm that sounds a little strange

Fixed.
  
> > Key and value
> > + * are separated by the key_val_sep chars, pairs are separated by the
> > + * opt_sep chars.
> > + *
> > + * For each key/value pair found, stores the value in the field in ctx
> > + * that is named like the key. ctx must be an AVClass context, storing
> > + * is done using AVOptions.
> > + *
> 
> > + * @return 0 if successfull, a negative number otherwise
> 
> >=0 if succcessfull

Fixed. 

Attached patch (which is not for review, I want to test it more), adds
also a test program, and here it is the output:

--------------------------8<---------------------------------------
|| -> || + ||
|:| -> || + |:|
|    | -> || + ||
|foo     | -> |foo| + ||
|     foo| -> |foo| + ||
|      foo       | -> |foo| + ||
| foo   bar    :   blahblah| -> |foo   bar| + |:   blahblah|
|\f\o\o| -> |foo| + ||
|'foo : \ \  '   : blahblah| -> |foo : \ \| + |: blahblah|
|'\fo\o:': blahblah| -> |\fo\o:| + |: blahblah|
|\'fo\o\:':  foo'  :blahblah| -> |'foo::  foo| + |:blahblah|

|key1=val1:key2=val2   ::    key3=  valN | ->
Setting value 'val1' for key 'key1'
Setting value 'val2' for key 'key2'
Setting value 'valN' for key ':    key3'

|:| ->
No value found in the string '' for key ':'

|::| ->
No value found in the string '' for key '::'

|foo| ->
No value found in the string '' for key 'foo'

|foo=  :| ->
Setting value '' for key 'foo'
No value found in the string '' for key ''

|foo=     :bar=   :| ->
Setting value '' for key 'foo'
Setting value '' for key 'bar'
No value found in the string '' for key ''

|fooo::ooo::=====bar===| ->
Setting value '====bar===' for key 'fooo::ooo::'
--------------------------8<---------------------------------------

Note in particular in the last example that when the parser doesn't
expect the terminator char then it treats the terminator like part of
the key, I'm not sure this is the right behavior.

Also should we for example interpret ":::" as a unique separator or
parse it as: ":" + "::" where the last "::" is part of the key (the
last looks more correct to me).

Regards.
-- 
FFmpeg = Faithful and Fiendish Magnificient Pitiful Extreme Gangster
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lavfi-add-parse-options.patch
Type: text/x-diff
Size: 8294 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090422/d98b7454/attachment.patch>