From e4fd58f4725218db8ed38fe8cecbc776410059a1 Mon Sep 17 00:00:00 2001 From: Stefano Sabatini Date: Thu, 11 Apr 2013 01:12:33 +0200 Subject: [PATCH] lavfi/hue: apply major simplifications, and switch to AVOption-based system This also drops support for "flat syntax" and "reinit" command. "reinit" command is not very robust and complicates the logic more than necessary, since requires to reset all the options in the command. *This is a syntax break*. --- doc/filters.texi | 51 ++++++---------- libavfilter/avfilter.c | 1 - libavfilter/version.h | 2 +- libavfilter/vf_hue.c | 134 +++++++++++++++-------------------------- 4 files changed, 69 insertions(+), 119 deletions(-) diff --git a/doc/filters.texi b/doc/filters.texi index 34383083e2..ee44b5f632 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -3653,24 +3653,27 @@ a float number which specifies chroma temporal strength, defaults to Modify the hue and/or the saturation of the input. -This filter accepts the following optional named options: +This filter accepts the following options: @table @option @item h -Specify the hue angle as a number of degrees. It accepts a float -number or an expression, and defaults to 0.0. - -@item H -Specify the hue angle as a number of radians. It accepts a float -number or an expression, and defaults to 0.0. +Specify the hue angle as a number of degrees. It accepts an expression, +and defaults to "0". @item s Specify the saturation in the [-10,10] range. It accepts a float number and -defaults to 1.0. +defaults to "1". + +@item H +Specify the hue angle as a number of radians. It accepts a float +number or an expression, and defaults to "0". @end table -The @var{h}, @var{H} and @var{s} parameters are expressions containing the -following constants: +@option{h} and @option{H} are mutually exclusive, and can't be +specified at the same time. + +The @option{h}, @option{H} and @option{s} option values are +expressions containing the following constants: @table @option @item n @@ -3689,10 +3692,6 @@ timestamp expressed in seconds, NAN if the input timestamp is unknown time base of the input video @end table -The options can also be set using the syntax: @var{hue}:@var{saturation} - -In this case @var{hue} is expressed in degrees. - @subsection Examples @itemize @@ -3708,19 +3707,6 @@ Same command but expressing the hue in radians: hue=H=PI/2:s=1 @end example -@item -Same command without named options, hue must be expressed in degrees: -@example -hue=90:1 -@end example - -@item -Note that "h:s" syntax does not support expressions for the values of -h and s, so the following example will issue an error: -@example -hue=PI/2:1 -@end example - @item Rotate hue and make the saturation swing between 0 and 2 over a period of 1 second: @@ -3756,12 +3742,15 @@ hue="s=max(0\, min(1\, (START+DURATION-t)/DURATION))" This filter supports the following command: @table @option -@item reinit +@item s +@item h +@item H Modify the hue and/or the saturation of the input video. -The command accepts the same named options and syntax than when calling the -filter from the command-line. +The command accepts the same options and syntax of the corresponding +options. -If a parameter is omitted, it is kept at its current value. +If the specified expression is not valid, it is kept at its current +value. @end table @section idet diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c index 9655fcf0cb..3e2077d724 100644 --- a/libavfilter/avfilter.c +++ b/libavfilter/avfilter.c @@ -679,7 +679,6 @@ static const char *const filters_left_to_update[] = { "atempo", "buffer", "flite", - "hue", "mp", "pan", "scale", diff --git a/libavfilter/version.h b/libavfilter/version.h index e98ea3808b..9c5b99fe01 100644 --- a/libavfilter/version.h +++ b/libavfilter/version.h @@ -30,7 +30,7 @@ #define LIBAVFILTER_VERSION_MAJOR 3 #define LIBAVFILTER_VERSION_MINOR 52 -#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_hue.c b/libavfilter/vf_hue.c index 478ce6e141..cc191fe25c 100644 --- a/libavfilter/vf_hue.c +++ b/libavfilter/vf_hue.c @@ -36,12 +36,6 @@ #include "internal.h" #include "video.h" -#define HUE_DEFAULT_VAL 0 -#define SAT_DEFAULT_VAL 1 - -#define HUE_DEFAULT_VAL_STRING AV_STRINGIFY(HUE_DEFAULT_VAL) -#define SAT_DEFAULT_VAL_STRING AV_STRINGIFY(SAT_DEFAULT_VAL) - #define SAT_MIN_VAL -10 #define SAT_MAX_VAL 10 @@ -78,7 +72,6 @@ typedef struct { int vsub; int32_t hue_sin; int32_t hue_cos; - int flat_syntax; double var_values[VAR_NB]; } HueContext; @@ -87,9 +80,9 @@ typedef struct { static const AVOption hue_options[] = { { "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 = NULL }, .flags = FLAGS }, { "s", "set the saturation expression", OFFSET(saturation_expr), AV_OPT_TYPE_STRING, + { .str = "1" }, .flags = FLAGS }, + { "H", "set the hue angle radians expression", OFFSET(hue_expr), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = FLAGS }, { NULL } }; @@ -107,99 +100,62 @@ static inline void compute_sin_and_cos(HueContext *hue) hue->hue_cos = rint(cos(hue->hue) * (1 << 16) * hue->saturation); } -#define SET_EXPRESSION(attr, name) do { \ - if (hue->attr##_expr) { \ - if ((ret = av_expr_parse(&hue->attr##_pexpr, hue->attr##_expr, var_names, \ - NULL, NULL, NULL, NULL, 0, ctx)) < 0) { \ - av_log(ctx, AV_LOG_ERROR, \ - "Parsing failed for expression " #name "='%s'", \ - hue->attr##_expr); \ - hue->attr##_expr = old_##attr##_expr; \ - hue->attr##_pexpr = old_##attr##_pexpr; \ - return AVERROR(EINVAL); \ - } else if (old_##attr##_pexpr) { \ - av_freep(&old_##attr##_expr); \ - av_expr_free(old_##attr##_pexpr); \ - old_##attr##_pexpr = NULL; \ - } \ - } else { \ - hue->attr##_expr = old_##attr##_expr; \ - } \ -} while (0) - -static inline int set_options(AVFilterContext *ctx, const char *args) +static int set_expr(AVExpr **pexpr, const char *expr, const char *option, void *log_ctx) { - HueContext *hue = ctx->priv; int ret; - char *old_hue_expr, *old_hue_deg_expr, *old_saturation_expr; - AVExpr *old_hue_pexpr, *old_hue_deg_pexpr, *old_saturation_pexpr; - static const char *shorthand[] = { "h", "s", NULL }; - old_hue_expr = hue->hue_expr; - old_hue_deg_expr = hue->hue_deg_expr; - old_saturation_expr = hue->saturation_expr; + AVExpr *old = NULL; - old_hue_pexpr = hue->hue_pexpr; - old_hue_deg_pexpr = hue->hue_deg_pexpr; - old_saturation_pexpr = hue->saturation_pexpr; - - hue->hue_expr = NULL; - hue->hue_deg_expr = NULL; - hue->saturation_expr = NULL; - - if ((ret = av_opt_set_from_string(hue, args, shorthand, "=", ":")) < 0) + if (*pexpr) + old = *pexpr; + ret = av_expr_parse(pexpr, expr, var_names, + NULL, NULL, NULL, NULL, 0, log_ctx); + if (ret < 0) { + av_log(log_ctx, AV_LOG_ERROR, + "Error when evaluating the expression '%s' for %s\n", + expr, option); + *pexpr = old; return ret; - 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); } - - SET_EXPRESSION(hue_deg, h); - SET_EXPRESSION(hue, H); - SET_EXPRESSION(saturation, s); - - hue->flat_syntax = 0; - - av_log(ctx, AV_LOG_VERBOSE, - "H_expr:%s h_deg_expr:%s s_expr:%s\n", - hue->hue_expr, hue->hue_deg_expr, hue->saturation_expr); - - compute_sin_and_cos(hue); - + av_expr_free(old); return 0; } 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); + 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"); + return AVERROR(EINVAL); + } - hue->saturation = SAT_DEFAULT_VAL; - hue->hue = HUE_DEFAULT_VAL; - hue->hue_deg_pexpr = NULL; - hue->hue_pexpr = NULL; - hue->flat_syntax = 1; +#define SET_EXPR(expr, option) \ + if (hue->expr##_expr) do { \ + ret = set_expr(&hue->expr##_pexpr, hue->expr##_expr, option, ctx); \ + if (ret < 0) \ + return ret; \ + } while (0) + SET_EXPR(saturation, "s"); + SET_EXPR(hue_deg, "h"); + SET_EXPR(hue, "H"); - return set_options(ctx, args); + av_log(ctx, AV_LOG_VERBOSE, + "H_expr:%s h_deg_expr:%s s_expr:%s\n", + hue->hue_expr, hue->hue_deg_expr, hue->saturation_expr); + compute_sin_and_cos(hue); + + return 0; } static av_cold void uninit(AVFilterContext *ctx) { HueContext *hue = ctx->priv; - av_opt_free(hue); - - av_free(hue->hue_deg_expr); av_expr_free(hue->hue_deg_pexpr); - av_free(hue->hue_expr); av_expr_free(hue->hue_pexpr); - av_free(hue->saturation_expr); av_expr_free(hue->saturation_pexpr); } @@ -295,7 +251,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *inpic) av_frame_copy_props(outpic, inpic); } - if (!hue->flat_syntax) { + /* todo: reindent */ hue->var_values[VAR_T] = TS2T(inpic->pts, inlink->time_base); hue->var_values[VAR_PTS] = TS2D(inpic->pts); @@ -323,7 +279,6 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *inpic) hue->var_values[VAR_T], (int)hue->var_values[VAR_N]); compute_sin_and_cos(hue); - } hue->var_values[VAR_N] += 1; @@ -345,10 +300,17 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *inpic) static int process_command(AVFilterContext *ctx, const char *cmd, const char *args, char *res, int res_len, int flags) { - if (!strcmp(cmd, "reinit")) - return set_options(ctx, args); - else - return AVERROR(ENOSYS); + HueContext *hue = ctx->priv; + +#define SET_CMD(expr, option) \ + if (!strcmp(cmd, option)) do { \ + return set_expr(&hue->expr##_pexpr, args, cmd, ctx); \ + } while (0) + SET_CMD(hue_deg, "h"); + SET_CMD(hue, "H"); + SET_CMD(saturation, "s"); + + return AVERROR(ENOSYS); } static const AVFilterPad hue_inputs[] = {