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

@@ -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

libavfilter/avfilter.c

@@ -679,7 +679,6 @@ static const char *const filters_left_to_update[] = {
     "atempo",
     "buffer",
     "flite",
-    "hue",
     "mp",
     "pan",
     "scale",

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, \

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;
-
-    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)
+    AVExpr *old = NULL;
+
+    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;
+    }
+    av_expr_free(old);
+    return 0;
+}
+
+static av_cold int init(AVFilterContext *ctx, const char *args)
+{
+    HueContext *hue = ctx->priv;
+    int 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;
+#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");
 
     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 int init(AVFilterContext *ctx, const char *args)
-{
-    HueContext *hue = ctx->priv;
-
-    hue->class = &hue_class;
-    av_opt_set_defaults(hue);
-
-    hue->saturation    = SAT_DEFAULT_VAL;
-    hue->hue           = HUE_DEFAULT_VAL;
-    hue->hue_deg_pexpr = NULL;
-    hue->hue_pexpr     = NULL;
-    hue->flat_syntax   = 1;
-
-    return set_options(ctx, args);
-}
-
 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[] = {