[FFmpeg-devel] [PATCH] lavfi/settb: add support for named options

[Stefano Sabatini]

On date Saturday 2013-03-16 00:42:12 +0100, Clément Bœsch encoded:
> On Sat, Mar 16, 2013 at 12:30:35AM +0100, Stefano Sabatini wrote:
> > TODO: bump micro
> > ---
> >  doc/filters.texi      |   16 +++++++++++---
> >  libavfilter/f_settb.c |   56 ++++++++++++++++++++++++++++++++++++++++++-------
> >  2 files changed, 62 insertions(+), 10 deletions(-)
> > 
> > diff --git a/doc/filters.texi b/doc/filters.texi
> > index 108718b..df8e0fe 100644
> > --- a/doc/filters.texi
> > +++ b/doc/filters.texi
> > @@ -6579,12 +6579,22 @@ ffmpeg -nostats -i input.mp3 -filter_complex ebur128 -f null -
> >  Set the timebase to use for the output frames timestamps.
> >  It is mainly useful for testing timebase configuration.
> >  
> > -It accepts in input an arithmetic expression representing a rational.
> > -The expression can contain the constants "AVTB" (the
> > -default timebase), "intb" (the input timebase) and "sr" (the sample rate,
> > +The filter accepts parameters as a list of @var{key}=@var{value}
> > +pairs, separated by ":". If the key of the first options is omitted,
> > +the arguments are interpreted according to the syntax @option{tb}.
> 
> The @option{tb} alone looks a bit weird for a "syntax". Maybe you could
> reword that sentence.
> 
> > +
> > +A description of the accepted options follows.
> > +
> > + at table @option
> > + at item tb
> > +Set arithmetic expression representing a rational.
> > +
> > +The expression can contain the constants "AVTB" (the default
> > +timebase), "intb" (the input timebase) and "sr" (the sample rate,
> >  audio only).
> >  
> >  The default value for the input is "intb".
> > + at end table
> >  
> >  @subsection Examples
> >  
> > diff --git a/libavfilter/f_settb.c b/libavfilter/f_settb.c
> > index 436491e..afefae7 100644
> > --- a/libavfilter/f_settb.c
> > +++ b/libavfilter/f_settb.c
> > @@ -29,6 +29,7 @@
> >  #include "libavutil/avstring.h"
> >  #include "libavutil/eval.h"
> >  #include "libavutil/internal.h"
> > +#include "libavutil/opt.h"
> 
> nit++: can be moved one line below for perfection

It would be boring if perfect, but fixed.
 
> >  #include "libavutil/mathematics.h"
> >  #include "libavutil/rational.h"
> >  #include "avfilter.h"
> > @@ -51,21 +52,40 @@ enum var_name {
> >  };
> >  
> >  typedef struct {
> > -    char tb_expr[256];
> > +    const AVClass *class;
> > +    char *tb_expr;
> >      double var_values[VAR_VARS_NB];
> >  } SetTBContext;
> >  
> > -static av_cold int init(AVFilterContext *ctx, const char *args)
> > +#define OFFSET(x) offsetof(SetTBContext, x)
> 
> > +#define FLAGS AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
> > +
> 
> This is not true for asettb

Reworked with a clever hack.
 
> > +static const AVOption options[] = {
> > +    { "tb", "set timebase expression", OFFSET(tb_expr), AV_OPT_TYPE_STRING, {.str="intb"}, .flags=FLAGS },
> > +    { NULL },
> 
> nit++: extra comma
[...] 
> LGTM otherwise.

Will push it tomorrow, TFTR.
-- 
FFmpeg = Free Faithful Magnificient Pure Entertaining Gigant
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-lavfi-settb-add-support-for-named-options.patch
Type: text/x-diff
Size: 5008 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130316/ac989c82/attachment.bin>