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

Michael Niedermayer michaelni
Mon Apr 20 04:57:08 CEST 2009


On Mon, Apr 20, 2009 at 01:26:57AM +0200, Stefano Sabatini wrote:
> On date Sunday 2009-04-19 23:54:57 +0200, Michael Niedermayer encoded:
> > On Sun, Apr 19, 2009 at 11:41:29PM +0200, Stefano Sabatini wrote:
> > > On date Sunday 2009-04-19 20:24:13 +0200, Michael Niedermayer encoded:
> > > > On Sun, Apr 19, 2009 at 06:37:13PM +0200, Stefano Sabatini wrote:
> > > [...]
> > > > > Mmh, do you mean for key=val pairs?
> > > > > 
> > > > > What about to use the '?' to separate filter from params?
> > > > > 
> > > > > filter?key1=val1:key2=val2:...:keyN=valN
> > > > > 
> > > > > The '=' for key=val pairs seems the more natural choice.
> > > > 
> > > > '=' is not a symbol that needs escaping in the parameter string
> > > > 
> > > > only terminating symbols and \ ' need escaping
> > > > 
> > > > scale=key1=val1:key2=val2
> > > > is not ambigous
> > > > 
> > > > in that sense i dont think much escaping should be needed
> > > 
> > > Yet maybe such a syntax:
> > > filter?key1=val1:key2=val2:...:keyN=valN
> > 
> > i like to repeat, the = does not need escaping and the syntax is
> > filter=key1=val1:key2...
> 
> Yes right, see the attached patch (not for review, I'll repost it when
> this will be applied). 
> 
> [...]
> 
> > > [...]
> > > > > +/**
> > > > > + * Parses the options in opts.
> > > > > + *
> > > > > + * opts contains a list of "key=value" pairs separated by the ":"
> > > > > + * chars. The '=' and ':' chars can be escaped using the '\' char.
> > > > > + *
> > > > > + * For each key/value pair found, stores the value in the field in ctx
> > > > > + * that is named like key. ctx must be an AVClass context, storing is
> > > > > + * done using AVOptions.
> > > > > + *
> > > > > + * @return 0 if successfull, a negative number otherwise
> > > > > + */
> > > > > +int avfilter_parse_options(void *ctx, const char *opts);
> > > > 
> > > > no, av_ not avfilter, this code is not a filter or in any way
> > > > related to avfilter beyond just being using by avfilters at first
> > > 
> > > This suggests a better name for the files, parseutils.[hc], but then I
> > > wonder lavfi is not the better location for them.
> > > 
> > > And since they also require opt.[hc] then the better place looks lavc.
> > 
> > ive no objection to putting them in lavc, but this requires them to be
> > public API and this means it will take more time to approve because
> > changing public API later is painfull
> 
> Well we can keep parseutils.h, then if we want to promote the
> functions av_get_string() may be moved to libavutil/avstring.h, and
> av_set_options_string() should be placed in libavcodec/opt.h.
> 
> 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


> +    *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


> + */
> +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);

':'
?

[...]
> + * opts contains a list of key/value pairs separated. 

hmm that sounds a little strange



> 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


[...]
> @@ -97,7 +57,7 @@
>      char *name;
>      (*buf)++;
>  
> -    name = consume_string(buf);
> +    name = av_get_token(buf, TERM);
>  
>      if(!name[0]) {
>          av_log(log_ctx, AV_LOG_ERROR,
> @@ -162,12 +122,12 @@
>                                       int index, AVClass *log_ctx)
>  {
>      char *opts = NULL;
> -    char *name = consume_string(buf);
> +    char *name = av_get_token(buf, TERM);
>      AVFilterContext *ret;
>  
>      if(**buf == '=') {
>          (*buf)++;
> -        opts = consume_string(buf);
> +        opts = av_get_token(buf, TERM);
>      }
>  
>      ret = create_filter(graph, index, name, opts, log_ctx);

iam pretty sure TERM is not always correct

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090420/8d0cad68/attachment.pgp>



More information about the ffmpeg-devel mailing list