[FFmpeg-devel] [PATCH] lavfi/hue: add dynamic expression support

Stefano Sabatini stefasab at gmail.com
Sun Sep 9 12:29:58 CEST 2012


On date Saturday 2012-09-08 10:58:17 +0200, Jérémy Tran encoded:
> ---
>  doc/filters.texi     |  45 +++++++++++
>  libavfilter/vf_hue.c | 216 +++++++++++++++++++++++++++++++++++++++------------
>  2 files changed, 212 insertions(+), 49 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 685a3e2..0dcae5d 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -2318,6 +2318,26 @@ Specify the saturation in the [-10,10] range. It accepts a float number and
>  defaults to 1.0.
>  @end table
>  
> +The @var{h}, @var{H} and @var{s} parameters are expressions containing the
> +following constants:
> +
> + at table @option
> + at item t
> +Timestamp expressed in seconds, NAN if the input timestamp is unknown.
> +
> + at item n
> +Frame count of the input frame.
> +
> + at item tb
> +Time base of the input video.
> +
> + at item pts
> +Presentation timestamp of the input frame expressed in time base units.
> +

> + at item r
> +Frame rate of the input video.

What if frame rate is not defined?

> + at end table

Overall table nit:
incomplete sentences like "Time base of the input video." does not
need to be capitalized/dot-terminated, so:
"Time base of the input video." => "time base of the input video"

(and yes docs are not very consistent from this POV)

> +
>  The options can also be set using the syntax: @var{hue}:@var{saturation}
>  
>  In this case @var{hue} is expressed in degrees.
> @@ -2348,6 +2368,31 @@ h and s, so the following example will issue an error:
>  @example
>  hue=PI/2:1
>  @end example
> +
> + at item
> +Rotate hue over a period of 1 second and make the saturation swing between 0
> +and 2:

Nit: factorize "over a period of 1 second" since the period is the
same for both functions:

Rotate hue and make the saturation swing between 0 and 2 over a period
of 1 second.

> + at example
> +hue="H=2*PI*t: s=sin(2*PI*t)+1"
> + at end example
> +
> + at item
> +Apply a 3 seconds saturation fade-in:
> + at example
> +hue=s=min(t/3,1)
> + at end example
> +
> + at item
> +Apply a 3 seconds saturation fade-out starting at 5 seconds:
> + at example
> +hue="s=max(0, min(1, (8-t)/3))"
> + at end example
> +
> + at item
> +The general fade-out equation is:
> + at example
> +hue="s=max(0, min(1, (START+DURATION)/DURATION))"

Nit++: maybe we could show the general equation, then a specific
application.

> + at end example
>  @end itemize
>  
>  @subsection Commands
> diff --git a/libavfilter/vf_hue.c b/libavfilter/vf_hue.c
> index 1ce231e..1741446 100644
> --- a/libavfilter/vf_hue.c
> +++ b/libavfilter/vf_hue.c
> @@ -26,6 +26,7 @@
>   */
>  
>  #include <float.h>
> +#include "libavutil/eval.h"
>  #include "libavutil/imgutils.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/pixdesc.h"
> @@ -38,52 +39,140 @@
>  #define HUE_DEFAULT_VAL 0
>  #define SAT_DEFAULT_VAL 1
>  
> +#define STR(s) #s
> +#define STRINGIFY(s) STR(s)
> +#define HUE_DEFAULT_VAL_STRING STRINGIFY(HUE_DEFAULT_VAL)
> +#define SAT_DEFAULT_VAL_STRING STRINGIFY(SAT_DEFAULT_VAL)

Slightly overkill, you could simply do:

#define HUE_DEFAULT_VAL         0
#define HUE_DEFAULT_VAL_STRING "0"
#define SAT_DEFAULT_VAL         1
#define SAT_DEFAULT_VAL_STRING "1"

> +
> +#define SAT_MIN_VAL -10
> +#define SAT_MAX_VAL 10
> +

> +#define CONVERT_HUE_FROM_DEGREES_TO_RADIANS(hue) \
> +    hue->hue = hue->hue_deg * M_PI / 180

Pointless wrapper?

> +

> +static const char *const var_names[] = {
> +    "t",   // timestamp expressed in seconds
> +    "n",   // frame count
> +    "tb",  // timebase
> +    "pts", // presentation timestamp expressed in AV_TIME_BASE units
> +    "r",   // frame rate
> +    NULL
> +};
> +
> +enum var_name {
> +    VAR_T,
> +    VAR_N,
> +    VAR_TB,
> +    VAR_PTS,
> +    VAR_R,
> +    VAR_NB
> +};

Nit: lexycal order, usually helps to spot bugs when there are many
fields

> +
>  typedef struct {
>      const    AVClass *class;
>      float    hue_deg; /* hue expressed in degrees */
>      float    hue; /* hue expressed in radians */
> +    char     *hue_deg_expr;
> +    char     *hue_expr;
> +    AVExpr   *hue_deg_pexpr;
> +    AVExpr   *hue_pexpr;
>      float    saturation;
> +    char     *saturation_expr;
> +    AVExpr   *saturation_pexpr;
>      int      hsub;
>      int      vsub;
>      int32_t hue_sin;
>      int32_t hue_cos;
> +    int      flat_syntax;
> +    double   var_values[VAR_NB];
>  } HueContext;
>  
>  #define OFFSET(x) offsetof(HueContext, x)
>  #define FLAGS AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
>  static const AVOption hue_options[] = {
> -    { "h", "set the hue angle degrees", OFFSET(hue_deg), AV_OPT_TYPE_FLOAT,
> -      { .dbl = -FLT_MAX }, -FLT_MAX, FLT_MAX, FLAGS },
> -    { "H", "set the hue angle radians", OFFSET(hue), AV_OPT_TYPE_FLOAT,
> -      { .dbl = -FLT_MAX }, -FLT_MAX, FLT_MAX, FLAGS },
> -    { "s", "set the saturation value", OFFSET(saturation), AV_OPT_TYPE_FLOAT,
> -      { .dbl = SAT_DEFAULT_VAL }, -10, 10, FLAGS },
> +    { "h", "set the hue angle degrees expression", OFFSET(hue_deg_expr), AV_OPT_TYPE_STRING,
> +      { .str = NULL }, .flags = FLAGS },
> +    { "H", "set the hue angle radians expression", OFFSET(hue_expr), AV_OPT_TYPE_STRING,
> +      { .str = HUE_DEFAULT_VAL_STRING }, .flags = FLAGS },
> +    { "s", "set the saturation expression", OFFSET(saturation_expr), AV_OPT_TYPE_STRING,
> +      { .str = SAT_DEFAULT_VAL_STRING }, .flags = FLAGS },
>      { NULL }
>  };
>  
>  AVFILTER_DEFINE_CLASS(hue);
>  
> +static inline void compute_sin_and_cos(HueContext *hue)
> +{
> +    /*
> +     * Scale the value to the norm of the resulting (U,V) vector, that is
> +     * the saturation.
> +     * This will be useful in the process_chrominance function.
> +     */
> +    hue->hue_sin = rint(sin(hue->hue) * (1 << 16) * hue->saturation);
> +    hue->hue_cos = rint(cos(hue->hue) * (1 << 16) * hue->saturation);
> +}
> +
>  static inline int set_options(AVFilterContext *ctx, const char *args)
>  {
>      HueContext *hue = ctx->priv;
>      int n, ret;
>      char c1 = 0, c2 = 0;
> -    char *equal;
> +    char *old_hue_expr, *old_hue_deg_expr;
>  
>      if (args) {
>          /* named options syntax */
> -        if (equal = strchr(args, '=')) {
> -            hue->hue     = -FLT_MAX;
> -            hue->hue_deg = -FLT_MAX;
> +        if (strchr(args, '=')) {
> +            old_hue_expr     = hue->hue_expr;
> +            old_hue_deg_expr = hue->hue_deg_expr;
> +            hue->hue_expr     = NULL;
> +            hue->hue_deg_expr = NULL;
>  
>              if ((ret = av_set_options_string(hue, args, "=", ":")) < 0)
>                  return ret;
> -            if (hue->hue != -FLT_MAX && hue->hue_deg != -FLT_MAX) {
> +            if (hue->hue_expr && hue->hue_deg_expr) {
>                  av_log(ctx, AV_LOG_ERROR,
>                         "H and h options are incompatible and cannot be specified "
>                         "at the same time\n");
> +                hue->hue_expr     = old_hue_expr;
> +                hue->hue_deg_expr = old_hue_deg_expr;
> +
> +                return AVERROR(EINVAL);
> +            }
> +
> +            if (!hue->hue_expr && !hue->hue_deg_expr) {
> +                hue->hue_expr     = old_hue_expr;
> +                hue->hue_deg_expr = old_hue_deg_expr;
> +            }
> +
> +            if (hue->hue_expr || hue->hue_deg_expr) {
> +                if (hue->hue_deg_expr) {

> +                    if ((ret = av_expr_parse(&hue->hue_deg_pexpr, hue->hue_deg_expr, var_names,
> +                                             NULL, NULL, NULL, NULL, 0, ctx)) < 0) {
> +                        av_log(ctx, AV_LOG_ERROR,
> +                               "Parsing failed evaluating expression h='%s'\n",
> +                               hue->hue_deg_expr);
> +                        return AVERROR(EINVAL);
> +                    }

Note: this could be macroified.

> +                    CONVERT_HUE_FROM_DEGREES_TO_RADIANS(hue);
> +                } else if (hue->hue_expr &&
> +                           (ret = av_expr_parse(&hue->hue_pexpr, hue->hue_expr, var_names,
> +                                                NULL, NULL, NULL, NULL, 0, ctx)) < 0) {
> +                    av_log(ctx, AV_LOG_ERROR,
> +                           "Parsing failed evaluating expression H='%s'\n",
> +                           hue->hue_expr);
> +                    return AVERROR(EINVAL);
> +                }
> +            }
> +            if (hue->saturation_expr &&
> +                (ret = av_expr_parse(&hue->saturation_pexpr, hue->saturation_expr, var_names,
> +                                     NULL, NULL, NULL, NULL, 0, ctx) < 0)) {
> +                av_log(ctx, AV_LOG_ERROR,
> +                       "Parsing failed evaluating expression s='%s'\n",
> +                       hue->saturation_expr);
>                  return AVERROR(EINVAL);
>              }

Right now in case of failure the function doesn't ripristinate the old
expressions and parsed expressions values. It may be safer to restore
them.

Also if I'm not wrong this is leaking, since the old values of AVExpr
are not freed before being overwritten.

> +
> +            hue->flat_syntax = 0;
>          /* compatibility h:s syntax */
>          } else {
>              n = sscanf(args, "%f%c%f%c", &hue->hue_deg, &c1, &hue->saturation, &c2);
> @@ -94,13 +183,18 @@ static inline int set_options(AVFilterContext *ctx, const char *args)
>                  return AVERROR(EINVAL);
>              }
>  
> -            if (hue->saturation < -10 || hue->saturation > 10) {
> +            if (hue->saturation < SAT_MIN_VAL || hue->saturation > SAT_MAX_VAL) {
>                  av_log(ctx, AV_LOG_ERROR,
>                         "Invalid value for saturation %0.1f: "
> -                       "must be included between range -10 and +10\n", hue->saturation);
> +                       "must be included between range %d and +%d\n",
> +                       hue->saturation, SAT_MIN_VAL, SAT_MAX_VAL);
>                  return AVERROR(EINVAL);
>              }
> +
> +            CONVERT_HUE_FROM_DEGREES_TO_RADIANS(hue);
> +            hue->flat_syntax = 1;
>          }
> +        compute_sin_and_cos(hue);
>      }

Adding logs after the options are set would greatly help debugging.

>      return 0;
> @@ -109,25 +203,13 @@ static inline int set_options(AVFilterContext *ctx, const char *args)
>  static av_cold int init(AVFilterContext *ctx, const char *args)
>  {
>      HueContext *hue = ctx->priv;
> -    int ret;
>  
>      hue->class = &hue_class;
>      av_opt_set_defaults(hue);
> +    hue->saturation = SAT_DEFAULT_VAL;
> +    hue->hue        = HUE_DEFAULT_VAL;
>  
> -    if ((ret = set_options(ctx, args)) < 0)
> -        return ret;
> -
> -    if (hue->saturation == -FLT_MAX)
> -        hue->hue = SAT_DEFAULT_VAL;
> -    if (hue->hue == -FLT_MAX)
> -        hue->hue = HUE_DEFAULT_VAL;
> -    if (hue->hue_deg != -FLT_MAX)
> -        /* Convert angle from degrees to radians */
> -        hue->hue = hue->hue_deg * M_PI / 180;
> -
> -    av_log(ctx, AV_LOG_VERBOSE, "hue:%f*PI hue_deg:%f saturation:%f\n",
> -           hue->hue/M_PI, hue->hue*180/M_PI, hue->saturation);
> -    return 0;

> +    return set_options(ctx, args);

Does it work if args = 0? In particular is flat_syntax set?

>  }
>  
>  static av_cold void uninit(AVFilterContext *ctx)
> @@ -159,13 +241,10 @@ static int config_props(AVFilterLink *inlink)
>  
>      hue->hsub = desc->log2_chroma_w;
>      hue->vsub = desc->log2_chroma_h;
> -    /*
> -     * Scale the value to the norm of the resulting (U,V) vector, that is
> -     * the saturation.
> -     * This will be useful in the process_chrominance function.
> -     */
> -    hue->hue_sin = rint(sin(hue->hue) * (1 << 16) * hue->saturation);
> -    hue->hue_cos = rint(cos(hue->hue) * (1 << 16) * hue->saturation);
> +
> +    hue->var_values[VAR_N]  = 0;
> +    hue->var_values[VAR_TB] = av_q2d(inlink->time_base);

> +    hue->var_values[VAR_R]  = av_q2d(inlink->frame_rate);

This could be undefined, in that case better to set NaN rather than Inf.

>  
>      return 0;
>  }
> @@ -209,6 +288,57 @@ static void process_chrominance(uint8_t *udst, uint8_t *vdst, const int dst_line
>      }
>  }
>  
> +static int start_frame(AVFilterLink *inlink, AVFilterBufferRef *inpic)
> +{
> +    HueContext *hue = inlink->dst->priv;
> +    AVFilterLink *outlink = inlink->dst->outputs[0];
> +    AVFilterBufferRef *buf_out;
> +
> +    outlink->out_buf = ff_get_video_buffer(outlink, AV_PERM_WRITE, outlink->w, outlink->h);
> +    if (!outlink->out_buf)
> +        return AVERROR(ENOMEM);
> +
> +    avfilter_copy_buffer_ref_props(outlink->out_buf, inpic);
> +    outlink->out_buf->video->w = outlink->w;
> +    outlink->out_buf->video->h = outlink->h;
> +    buf_out = avfilter_ref_buffer(outlink->out_buf, ~0);
> +    if (!buf_out)
> +        return AVERROR(ENOMEM);
> +
> +    if (!hue->flat_syntax) {
> +        hue->var_values[VAR_T] = inpic->pts == AV_NOPTS_VALUE ?
> +           NAN : inpic->pts * av_q2d(inlink->time_base);
> +        hue->var_values[VAR_N] += 1;
> +        hue->var_values[VAR_PTS] = inpic->pts;
> +
> +        if (hue->saturation_expr) {
> +            hue->saturation = av_expr_eval(hue->saturation_pexpr, hue->var_values, NULL);
> +
> +            if (hue->saturation < SAT_MIN_VAL || hue->saturation > SAT_MAX_VAL) {

> +                hue->saturation = av_clip(hue->saturation, -10, 10);
                                                              ^^^  ^^
SAT_MIN/MAX_VAL

[...]
-- 
FFmpeg = Foolish & Fundamentalist Monstrous Philosofic Elastic Gospel


More information about the ffmpeg-devel mailing list