[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