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

[Clément Bœsch]

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

>  #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

> +static const AVOption options[] = {
> +    { "tb", "set timebase expression", OFFSET(tb_expr), AV_OPT_TYPE_STRING, {.str="intb"}, .flags=FLAGS },
> +    { NULL },

nit++: extra comma

> +};
> +
> +static av_cold int init(AVFilterContext *ctx, const char *args, const AVClass *class)
>  {
>      SetTBContext *settb = ctx->priv;
> -    av_strlcpy(settb->tb_expr, "intb", sizeof(settb->tb_expr));
> +    static const char *shorthand[] = { "tb", NULL };
> +    int ret;
> +
> +    settb->class = class;
> +    av_opt_set_defaults(settb);
>  
> -    if (args)
> -        sscanf(args, "%255[^:]", settb->tb_expr);
> +    if ((ret = av_opt_set_from_string(settb, args, shorthand, "=", ":")) < 0)
> +        return ret;
>  
>      return 0;
>  }
>  
> +static av_cold void uninit(AVFilterContext *ctx)
> +{
> +    SetTBContext *settb = ctx->priv;
> +    av_opt_free(settb);
> +}
> +
>  static int config_output_props(AVFilterLink *outlink)
>  {
>      AVFilterContext *ctx = outlink->src;
> @@ -120,6 +140,15 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>  }
>  
>  #if CONFIG_SETTB_FILTER
> +
> +#define settb_options options
> +AVFILTER_DEFINE_CLASS(settb);
> +
> +static av_cold int settb_init(AVFilterContext *ctx, const char *args)
> +{
> +    return init(ctx, args, &settb_class);
> +}
> +
>  static const AVFilterPad avfilter_vf_settb_inputs[] = {
>      {
>          .name             = "default",
> @@ -142,16 +171,27 @@ static const AVFilterPad avfilter_vf_settb_outputs[] = {
>  AVFilter avfilter_vf_settb = {
>      .name      = "settb",
>      .description = NULL_IF_CONFIG_SMALL("Set timebase for the video output link."),
> -    .init      = init,
> +    .init      = settb_init,
> +    .uninit    = uninit,
>  
>      .priv_size = sizeof(SetTBContext),
>  
>      .inputs    = avfilter_vf_settb_inputs,
>      .outputs   = avfilter_vf_settb_outputs,
> +    .priv_class = &settb_class,
>  };
>  #endif
>  
>  #if CONFIG_ASETTB_FILTER
> +
> +#define asettb_options options
> +AVFILTER_DEFINE_CLASS(asettb);
> +
> +static av_cold int asettb_init(AVFilterContext *ctx, const char *args)
> +{
> +    return init(ctx, args, &asettb_class);
> +}
> +
>  static const AVFilterPad avfilter_af_asettb_inputs[] = {
>      {
>          .name             = "default",
> @@ -174,10 +214,12 @@ static const AVFilterPad avfilter_af_asettb_outputs[] = {
>  AVFilter avfilter_af_asettb = {
>      .name      = "asettb",
>      .description = NULL_IF_CONFIG_SMALL("Set timebase for the audio output link."),
> -    .init      = init,
> +    .init      = asettb_init,
> +    .uninit    = uninit,
>  
>      .priv_size = sizeof(SetTBContext),
>      .inputs    = avfilter_af_asettb_inputs,
>      .outputs   = avfilter_af_asettb_outputs,
> +    .priv_class = &asettb_class,
>  };
>  #endif

LGTM otherwise.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130316/c21db910/attachment.asc>