[FFmpeg-devel] [PATCH] vf_hue: add named options support

Stefano Sabatini stefasab at gmail.com
Tue Aug 14 21:15:41 CEST 2012


On date Tuesday 2012-08-14 14:20:47 +0200, Jérémy Tran encoded:
> Old syntax has been kept for compatibility reasons.
> ---
>  doc/filters.texi     | 36 ++++++++++++++++++++++----
>  libavfilter/vf_hue.c | 72 ++++++++++++++++++++++++++++++++++++++--------------
>  2 files changed, 84 insertions(+), 24 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 763085c..4423da7 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -2191,12 +2191,38 @@ a float number which specifies chroma temporal strength, defaults to
>  
>  Modify the hue and/or the saturation of the input.
>  
> -This filter accepts the optional parameters: @var{hue}:@var{saturation}.
> +This filter accepts the optional parameters:

This filter accepts the following optional named options:

> - at var{hue} must be a float number that specifies the hue angle as a
> -number of degrees, and defaults to 0.0.
> - at var{saturation} must be a float number that specifies the saturation
> -in the [-10,10] range, and defaults to 1.0.
> + at table @option
> + at item h
> +a float number that specifies the hue angle as a
> +number of degrees, and defaults to 0.0
> + at item H

> +a float number that specifies the hue angle as a
> +number of radians, and defaults to 0.0
> + at item s
> +a float number that specifies the saturation
> +in the [-10,10] range, and defaults to 1.0
> + at end table
> +
> +The options can also be set using the syntax: @var{hue}:@var{saturation}
> +
> +In this case @var{hue} is expressed in degrees.
> +

> +Some examples follow:
> + at example
> +# hue equals to 90 degrees and saturation equals to 1.0
> +hue=h=90:s=1
> +
> +# same command but expressing the hue in radians
> +hue=H=PI/2:s=1
> +
> +# same command without named options, hue must be expressed in degrees
> +hue=90:1
> +
> +# this is wrong
> +hue=PI/2:1
> + at end example

This is better rendered with an @itemize environment.

>  
>  @section idet
>  
> diff --git a/libavfilter/vf_hue.c b/libavfilter/vf_hue.c
> index 16fa4f4..acddc73 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,7 +37,9 @@
>  #include "video.h"
>  
>  typedef struct {
> -    float    hue;
> +    const    AVClass *class;
> +    float    hue_deg; /* hue expressed in degrees */
> +    float    hue; /* hue expressed in radians */
>      float    saturation;
>      int      hsub;
>      int      vsub;
> @@ -42,37 +47,65 @@ typedef struct {
>      int32_t hue_cos;
>  } HueContext;
>  
> +#define OFFSET(x) offsetof(HueContext, x)

> +static const AVOption hue_options[] = {

> +    { "h", NULL, OFFSET(hue), AV_OPT_TYPE_FLOAT, { 0 }, -FLT_MAX, FLT_MAX, AV_OPT_FLAG_VIDEO_PARAM },
                           ^^^

I suppose this is hue_deg?

> +    { "H", NULL, OFFSET(hue), AV_OPT_TYPE_FLOAT, { 0 }, -FLT_MAX, FLT_MAX, AV_OPT_FLAG_VIDEO_PARAM },
> +    { "s", NULL, OFFSET(saturation), AV_OPT_TYPE_FLOAT, { 1 }, -10, 10, AV_OPT_FLAG_VIDEO_PARAM },
> +    { NULL }
> +};

Please specify the help text for the options.
h -> set the hue angle degrees
...

> +
> +AVFILTER_DEFINE_CLASS(hue);
> +
>  static av_cold int init(AVFilterContext *ctx, const char *args)
>  {
>      HueContext *hue = ctx->priv;
>      float h = 0, s = 1;
>      int n;
>      char c1 = 0, c2 = 0;
> -
> -    if (args) {
> -        n = sscanf(args, "%f%c%f%c", &h, &c1, &s, &c2);
> -        if (n != 0 && n != 1 && (n != 3 || c1 != ':')) {
> -            av_log(ctx, AV_LOG_ERROR,
> -                   "Invalid syntax for argument '%s': "
> -                   "must be in the form 'hue[:saturation]'\n", args);
> -            return AVERROR(EINVAL);
> +    char *equal;
> +
> +    hue->class = &hue_class;
> +
> +    /* named options syntax */
> +    if (equal = strchr(args, '=')) {
> +        av_opt_set_defaults(hue);
> +        av_set_options_string(hue, args, "=", ":");
> +    /* compatibility syntax */
> +    } else {

> +        if (args) {
> +            n = sscanf(args, "%f%c%f%c", &h, &c1, &s, &c2);
> +            if (n != 0 && n != 1 && (n != 3 || c1 != ':')) {
> +                av_log(ctx, AV_LOG_ERROR,
> +                       "Invalid syntax for argument '%s': "
> +                       "must be in the form 'hue[:saturation]'\n", args);
> +                return AVERROR(EINVAL);
> +            }
> +            if (s < -10 || s > 10) {
> +                av_log(ctx, AV_LOG_ERROR,
> +                       "Invalid value for saturation %0.1f: "
> +                       "must be included between range -10 and +10\n", s);
> +                return AVERROR(EINVAL);
> +            }

leave it non-indented if this will simplify the patch (send a
separated patch with the reindent change).

>          }
> +        hue->hue_deg = h;
> +        hue->saturation = s;
>      }
>  
> -    if (s < -10 || s > 10) {
> -        av_log(ctx, AV_LOG_ERROR,
> -               "Invalid value for saturation %0.1f: "
> -               "must be included between range -10 and +10\n", s);
> -        return AVERROR(EINVAL);
> -    }
> -
> -    /* Convert angle from degree to radian */
> -    hue->hue = h * M_PI / 180;
> -    hue->saturation = s;

> +    if (!hue->hue)

if hue->hue_deg?

> +        /* Convert angle from degrees to radians */
> +        hue->hue = hue->hue_deg * M_PI / 180;

Also since h/H conflict, maybe you should add a check for making sure
that only one is specified. A possible way:
you set hue and hue_deg by default to -FLT_MAX, meaning "undefined".

If either hue or hue_deg are different from -FLT_MAX, then means that
both have been defined and this is not ammissible.

If both are undefined, assume value 0 by default.

Unfortunately dealing with such situations is a bit awkward, since we
have no way to say which values have been "set" if not looking at
their values (we may consider to extend the options framework at some
point).

[...]
-- 
FFmpeg = Fantastic & Fantastic Monstrous Peaceless Ecumenical Gigant


More information about the ffmpeg-devel mailing list