[FFmpeg-devel] [PATCH] lavfi/hue: apply major simplifications, and switch to AVOption-based system
Paul B Mahol
onemda at gmail.com
Thu Apr 11 11:01:55 CEST 2013
On 4/10/13, Stefano Sabatini <stefasab at gmail.com> wrote:
> This also drops support for "flat syntax" and "reinit" command.
> ---
> doc/filters.texi | 40 ++++++----------
> libavfilter/avfilter.c | 1 +
> libavfilter/vf_hue.c | 119
> ++++++++++++++++--------------------------------
> 3 files changed, 53 insertions(+), 107 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 4a40e93..766dcb1 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -3612,24 +3612,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.
> -
> - at 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 accept 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".
> +
> + at 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:
> + at 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
> @@ -3648,10 +3651,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
> @@ -3668,19 +3667,6 @@ hue=H=PI/2:s=1
> @end example
>
> @item
> -Same command without named options, hue must be expressed in degrees:
> - at example
> -hue=90:1
> - at end example
> -
> - at 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:
> - at example
> -hue=PI/2:1
> - at end example
> -
> - at item
> Rotate hue and make the saturation swing between 0
> and 2 over a period of 1 second:
> @example
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 916d661..18706fd 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -682,6 +682,7 @@ int avfilter_init_filter(AVFilterContext *filter, const
> char *args, void *opaque
> !strcmp(filter->filter->name, "histeq" ) ||
> !strcmp(filter->filter->name, "histogram" ) ||
> !strcmp(filter->filter->name, "hqdn3d" ) ||
> + !strcmp(filter->filter->name, "hue" ) ||
> !strcmp(filter->filter->name, "idet" ) ||
> !strcmp(filter->filter->name, "il" ) ||
> !strcmp(filter->filter->name, "kerndeint" ) ||
> diff --git a/libavfilter/vf_hue.c b/libavfilter/vf_hue.c
> index 478ce6e..2c63237 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,60 @@ 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)
> +enum EvalTarget { EVAL_HUE_DEG, EVAL_HUE_RAD, EVAL_SAT };
> +
> +static int set_expr(AVExpr **pexpr, const char *expr, void *log_ctx)
> +{
> + int ret;
> +
> + if (*pexpr)
> + av_expr_free(*pexpr);
> + *pexpr = NULL;
> + 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'\n", expr);
> + return ret;
> +}
> +
> +static av_cold int init(AVFilterContext *ctx, const char *args)
> {
> 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)
> - 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;
> +#define SET_EXPR(expr) \
> + if (hue->expr##_expr) { \
> + ret = set_expr(&hue->expr##_pexpr, hue->expr##_expr, ctx); \
> + if (ret < 0) \
> + return ret; \
> + }
> + SET_EXPR(saturation);
> + SET_EXPR(hue_deg);
> + SET_EXPR(hue);
>
> 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 +249,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 +277,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,8 +298,14 @@ 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);
> + HueContext *hue = ctx->priv;
> +
> + if (!strcmp(cmd, "h"))
> + return set_expr(&hue->hue_deg_pexpr, args, ctx);
> + else if (!strcmp(cmd, "H"))
> + return set_expr(&hue->hue_pexpr, args, ctx);
> + else if (!strcmp(cmd, "s"))
> + return set_expr(&hue->saturation_pexpr, args, ctx);
> else
> return AVERROR(ENOSYS);
I would like that this could be further simplified.
> }
> --
> 1.7.9.5
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
More information about the ffmpeg-devel
mailing list