[FFmpeg-devel] [PATCH 2/2] vf_hue: add named options syntax for the filter parameters

[Stefano Sabatini]

On date Sunday 2012-08-12 13:26:18 +0200, Jérémy Tran encoded:
> The old syntax has been kept.
> It is now also possible to express the hue in radian.
> 
> Signed-off-by: Jérémy Tran <tran.jeremy.av at gmail.com>
> ---
>  doc/filters.texi     |  2 ++
>  libavfilter/vf_hue.c | 80 +++++++++++++++++++++++++++++++++++++++-------------
>  2 files changed, 63 insertions(+), 19 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 71f6670..97bd9d2 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -2198,6 +2198,8 @@ ffmpeg -i in.avi -vf hue=100:1
>  
>  # same command but using named options
>  ffmpeg -i in.avi -vf hue=h=100:s=1
> +# it is possible to express the hue in radian
> +ffmpeg -i in.avi -vf hue=hrad=PI/2:s=1
>  @end example
>  
>  @table @option
> diff --git a/libavfilter/vf_hue.c b/libavfilter/vf_hue.c
> index 8523e66..f62c300 100644
> --- a/libavfilter/vf_hue.c
> +++ b/libavfilter/vf_hue.c
> @@ -25,7 +25,10 @@
>   * Ported from MPlayer libmpcodecs/vf_hue.c
>   */
>  
> +#include <float.h>
> +#include "libavutil/eval.h"
>  #include "libavutil/imgutils.h"
> +#include "libavutil/opt.h"
>  #include "libavutil/pixdesc.h"
>  
>  #include "avfilter.h"
> @@ -34,42 +37,69 @@
>  #include "video.h"
>  
>  typedef struct {
> -    float    hue;
> -    float    saturation;
> -    int      hsub;
> -    int      vsub;
> -    uint32_t hue_sin;
> -    uint32_t hue_cos;
> +    const AVClass *class;
> +    float         h;
> +    char*         hrad; /* expr in radian */
> +    float         s;

why the rename (h->hue, s->saturation?)

> +    int32_t       hsub;
> +    int32_t       vsub;
> +    int32_t       hue_sin;
> +    int32_t       hue_cos;

unrelated


>  } HueContext;
>  
> +#define OFFSET(x) offsetof(HueContext, x)
> +static const AVOption hue_options[] = {
> +    { "h", NULL, OFFSET(h), AV_OPT_TYPE_FLOAT, { 0 }, -FLT_MAX, FLT_MAX, AV_OPT_FLAG_VIDEO_PARAM },
> +    { "hrad", NULL, OFFSET(hrad), AV_OPT_TYPE_STRING, { 0 }, .flags = AV_OPT_FLAG_VIDEO_PARAM },
> +    { "s", NULL, OFFSET(s), AV_OPT_TYPE_FLOAT, { 1 }, 0, 2, AV_OPT_FLAG_VIDEO_PARAM },
> +    { NULL }
> +};

Also please add documentation to filters.texi.

> +AVFILTER_DEFINE_CLASS(hue);
> +
>  static av_cold int init(AVFilterContext *ctx, const char *args)
>  {
>      HueContext *hue = ctx->priv;
> +    char *equal;
> +    char c1, c2;
>      float h = 0, s = 1;
>      int32_t ret;
> -    char c1 = 0, c2 = 0;
>  
> -    if (args) {
> +    hue->class = &hue_class;
> +
> +    // named options syntax
> +    if (equal = strchr(args, '=')) {
> +        av_opt_set_defaults(hue);
> +        ret = av_set_options_string(hue, args, "=", ":");

> +        if (ret < 0) {

> +            av_log(ctx, AV_LOG_ERROR,
> +                   "Error parsing the options string: %s\n", args);

No need for this.

> +            av_opt_free(hue);

This should be moved in uninit(), or the filter will leak.

> +            return ret;
> +        }
> +    // compatibility syntax
> +    } else {
>          ret = sscanf(args, "%f%c%f%c", &h, &c1, &s, &c2);
>          if (!ret || c1 != ':' || c2) {
>              av_log(ctx, AV_LOG_ERROR,
>                     "Error parsing the options string: %s\n", args);
>              return AVERROR(EINVAL);
>          }
> +
> +        hue->h = h;
> +        hue->s = s;
>      }
>  
> -    if (s < 0 || s > 2) {

> +    if (hue->s < 0 || hue->s > 2)
>          av_log(ctx, AV_LOG_ERROR,
> -               "Invalid value for s:%0.1f : must be included in the range 0-2\n",
> -               s);
> -        return AVERROR(EINVAL);
> +               "Invalid value for s:%1.0f : must be included in the range 0-2\n", s);

unrelated cosemtics

> +    else {
> +        /* Convert angle from degree to radian */
> +        hue->h *= M_PI / 180;
> +        return 0;
>      }
>  
> -    /* Convert angle from degree to radian */
> -    hue->hue = h * M_PI / 180;
> -    hue->saturation = s;
> -
> -    return 0;
> +    return AVERROR(EINVAL);
>  }
>  
>  static int query_formats(AVFilterContext *ctx)
> @@ -91,16 +121,28 @@ static int config_props(AVFilterLink *inlink)
>  {
>      HueContext *hue = inlink->dst->priv;
>      const AVPixFmtDescriptor *desc = &av_pix_fmt_descriptors[inlink->format];

> +    double expr_res;
> +    char *expr = hue->hrad;
> +    const char *var_names[] = { "PI", NULL };
> +    double var_values[] = { M_PI, 0 };
>  
>      hue->hsub = desc->log2_chroma_w;
>      hue->vsub = desc->log2_chroma_h;
> +
> +    if (expr) {
> +        av_expr_parse_and_eval(&expr_res, expr,
> +                               var_names, var_values,
> +                               NULL, NULL, NULL, NULL, NULL, 0, hue);
> +        hue->h = expr_res;
> +    }

Note that this should not be required. opt supports expressions (with
PI and other constants) out of the box.
-- 
FFmpeg = Free and Faithful Mega Perfectionist Ecumenical Gospel