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

[Stefano Sabatini]

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).

Regards.
-- 
FFmpeg = Faithless & Forgiving Minimal Pacific Enlightening Gargoyle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lavfi-add-parse-options.patch
Type: text/x-diff
Size: 6724 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090420/9d05043b/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: graphparser-use-get-token.patch
Type: text/x-diff
Size: 2323 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090420/9d05043b/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: graphparser-no-equal-escaping.patch
Type: text/x-diff
Size: 762 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090420/9d05043b/attachment-0002.patch>