[FFmpeg-devel] [PATCH] lavfi/hue: fix h/H commands in case h and H are not specified as initial parameters

[Clément Bœsch]

On Sun, Apr 21, 2013 at 05:33:33PM +0200, Stefano Sabatini wrote:
> In filter_frame(), check for the existence of the corresponding parsed
> expression rather than on the expression, which may or may not be set,
> and release the alternative parsed expression when one of H/h is set.
> ---
>  libavfilter/vf_hue.c |   31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/libavfilter/vf_hue.c b/libavfilter/vf_hue.c
> index 9b2ecd4..b91e9c1 100644
> --- a/libavfilter/vf_hue.c
> +++ b/libavfilter/vf_hue.c
> @@ -255,7 +255,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *inpic)
>      hue->var_values[VAR_T]   = TS2T(inpic->pts, inlink->time_base);
>      hue->var_values[VAR_PTS] = TS2D(inpic->pts);
>  
> -    if (hue->saturation_expr) {
> +    if (hue->saturation_pexpr) {
>          hue->saturation = av_expr_eval(hue->saturation_pexpr, hue->var_values, NULL);
>  
>          if (hue->saturation < SAT_MIN_VAL || hue->saturation > SAT_MAX_VAL) {
> @@ -266,10 +266,10 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *inpic)
>          }
>      }
>  
> -    if (hue->hue_deg_expr) {
> +    if (hue->hue_deg_pexpr) {
>          hue->hue_deg = av_expr_eval(hue->hue_deg_pexpr, hue->var_values, NULL);
>          hue->hue = hue->hue_deg * M_PI / 180;
> -    } else if (hue->hue_expr) {
> +    } else if (hue->hue_pexpr) {
>          hue->hue = av_expr_eval(hue->hue_pexpr, hue->var_values, NULL);
>          hue->hue_deg = hue->hue * 180 / M_PI;
>      }
> @@ -307,16 +307,25 @@ static int process_command(AVFilterContext *ctx, const char *cmd, const char *ar
>                             char *res, int res_len, int flags)
>  {
>      HueContext *hue = ctx->priv;
> +    int ret =  AVERROR(ENOSYS);
>  
> -#define SET_CMD(expr, option) do {                                 \
> -    if (!strcmp(cmd, option))                                      \
> -        return set_expr(&hue->expr##_pexpr, args, cmd, ctx);       \
> -} while (0)
> -    SET_CMD(hue_deg,    "h");
> -    SET_CMD(hue,        "H");
> -    SET_CMD(saturation, "s");
> +    if (!strcmp(cmd, "h")) {
> +        ret = set_expr(&hue->hue_deg_pexpr, args, cmd, ctx);
> +        if (ret >= 0) {
> +            av_expr_free(hue->hue_pexpr);
> +            hue->hue_pexpr = NULL;
> +        }
> +    } else if (!strcmp(cmd, "H")) {
> +        ret = set_expr(&hue->hue_pexpr, args, cmd, ctx);
> +        if (ret >= 0) {
> +            av_expr_free(hue->hue_deg_pexpr);
> +            hue->hue_deg_pexpr = NULL;
> +        }
> +    } else if (!strcmp(cmd, "s")) {
> +        ret = set_expr(&hue->saturation_pexpr, args, cmd, ctx);
> +    }
>  
> -    return AVERROR(ENOSYS);
> +    return ret;
>  }
>  

LGTM

Note: you could also av_freep() the opposite expression strings in the h/H
command case to avoid inconsistent context states and the first diff
chunks of that patch.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130422/84b338ad/attachment.asc>