[FFmpeg-devel] [PATCH 1/2] lavfi/gradfun: support named options.

[Stefano Sabatini]

On date Tuesday 2012-12-18 22:24:06 +0100, Clément Bœsch encoded:
> This breaks usage for out-of-range values.
> ---
>  doc/filters.texi         | 12 +++++++-----
>  libavfilter/gradfun.h    |  2 ++
>  libavfilter/version.h    |  2 +-
>  libavfilter/vf_gradfun.c | 31 +++++++++++++++++++++++--------
>  4 files changed, 33 insertions(+), 14 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 2f50d65..6e9ebc7 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -2665,19 +2665,21 @@ This filter is designed for playback only.  Do not use it prior to
>  lossy compression, because compression tends to lose the dither and
>  bring back the bands.
>  
> -The filter takes two optional parameters, separated by ':':
> - at var{strength}:@var{radius}
> +The filter accepts a list of options in the form of @var{key}=@var{value} pairs
> +separated by ":". A description of the accepted options follows.
>  
>  @var{strength} is the maximum amount by which the filter will change
>  any one pixel. Also the threshold for detecting nearly flat
>  regions. Acceptable values range from .51 to 64, default value is
> -1.2, out-of-range values will be clipped to the valid range.
> +1.2.

For consistency please create @ŧable option and a separate @item for
each option.

>  
>  @var{radius} is the neighborhood to fit the gradient to. A larger
>  radius makes for smoother gradients, but also prevents the filter from
>  modifying the pixels near detailed regions. Acceptable values are
> -8-32, default value is 16, out-of-range values will be clipped to the
> -valid range.
> +8-32, default value is 16.
> +
> +Alternatively, the options can be specified as a flat string:
> + at var{strength}[:@var{radius}]
>  
>  @example
>  # default parameters
> diff --git a/libavfilter/gradfun.h b/libavfilter/gradfun.h
> index 939b129..801dddd 100644
> --- a/libavfilter/gradfun.h
> +++ b/libavfilter/gradfun.h
> @@ -26,6 +26,8 @@
>  
>  /// Holds instance-specific information for gradfun.
>  typedef struct GradFunContext {
> +    const AVClass *class;
> +    double strength;    ///< user specified strength, used to define thresh
>      int thresh;    ///< threshold for gradient algorithm
>      int radius;    ///< blur radius
>      int chroma_w;  ///< width of the chroma planes
> diff --git a/libavfilter/version.h b/libavfilter/version.h
> index 26b6645..ee6525f 100644
> --- a/libavfilter/version.h
> +++ b/libavfilter/version.h
> @@ -30,7 +30,7 @@
>  
>  #define LIBAVFILTER_VERSION_MAJOR  3
>  #define LIBAVFILTER_VERSION_MINOR  28
> -#define LIBAVFILTER_VERSION_MICRO 101
> +#define LIBAVFILTER_VERSION_MICRO 102
>  
>  #define LIBAVFILTER_VERSION_INT AV_VERSION_INT(LIBAVFILTER_VERSION_MAJOR, \
>                                                 LIBAVFILTER_VERSION_MINOR, \
> diff --git a/libavfilter/vf_gradfun.c b/libavfilter/vf_gradfun.c
> index 6319bf7..2e92fda 100644
> --- a/libavfilter/vf_gradfun.c
> +++ b/libavfilter/vf_gradfun.c
> @@ -36,12 +36,24 @@
>  #include "libavutil/common.h"
>  #include "libavutil/cpu.h"
>  #include "libavutil/pixdesc.h"
> +#include "libavutil/opt.h"
>  #include "avfilter.h"
>  #include "formats.h"
>  #include "gradfun.h"
>  #include "internal.h"
>  #include "video.h"
>  
> +#define OFFSET(x) offsetof(GradFunContext, x)
> +#define F AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
> +
> +static const AVOption gradfun_options[] = {

> +    { "strength", "set the maximum amount by which the filter will change any one pixel", OFFSET(strength), AV_OPT_TYPE_DOUBLE, {.dbl = 1.2}, 0.51, 64, F },

unrelated, but this help message is confusing/confused

> +    { "radius",   "set the neighborhood to fit the gradient to",                          OFFSET(radius),   AV_OPT_TYPE_INT,    {.i64 =  16},    4, 32, F },
> +    { NULL }
> +};
> +
> +AVFILTER_DEFINE_CLASS(gradfun);
> +
>  DECLARE_ALIGNED(16, static const uint16_t, dither)[8][8] = {
>      {0x00,0x60,0x18,0x78,0x06,0x66,0x1E,0x7E},
>      {0x40,0x20,0x58,0x38,0x46,0x26,0x5E,0x3E},
> @@ -121,16 +133,18 @@ static void filter(GradFunContext *ctx, uint8_t *dst, const uint8_t *src, int wi
>  
>  static av_cold int init(AVFilterContext *ctx, const char *args)
>  {
> +    int ret;
>      GradFunContext *gf = ctx->priv;
> -    float thresh = 1.2;
> -    int radius = 16;
> +    static const char *shorthand[] = { "strength", "radius", NULL };
> +
> +    gf->class = &gradfun_class;
> +    av_opt_set_defaults(gf);
>  
> -    if (args)
> -        sscanf(args, "%f:%d", &thresh, &radius);
> +    if ((ret = av_opt_set_from_string(gf, args, shorthand, "=", ":")) < 0)
> +        return ret;
>  
> -    thresh = av_clipf(thresh, 0.51, 64);
> -    gf->thresh = (1 << 15) / thresh;
> -    gf->radius = av_clip((radius + 1) & ~1, 4, 32);
> +    gf->thresh = (1 << 15) / gf->strength;
> +    gf->radius = av_clip((gf->radius + 1) & ~1, 4, 32);
>  
>      gf->blur_line = ff_gradfun_blur_line_c;
>      gf->filter_line = ff_gradfun_filter_line_c;
> @@ -138,7 +152,7 @@ static av_cold int init(AVFilterContext *ctx, const char *args)
>      if (ARCH_X86)
>          ff_gradfun_init_x86(gf);
>  
> -    av_log(ctx, AV_LOG_VERBOSE, "threshold:%.2f radius:%d\n", thresh, gf->radius);
> +    av_log(ctx, AV_LOG_VERBOSE, "threshold:%.2f radius:%d\n", gf->strength, gf->radius);
>  
>      return 0;
>  }
> @@ -252,4 +266,5 @@ AVFilter avfilter_vf_gradfun = {
>  
>      .inputs    = avfilter_vf_gradfun_inputs,
>      .outputs   = avfilter_vf_gradfun_outputs,
> +    .priv_class    = &gradfun_class,
>  };

LGTM otherwise, thanks.
-- 
FFmpeg = Fierce and Foolish Muttering Picky Ecletic God