[FFmpeg-devel] libavfilter/scale: Populate ow/oh when using 0 as w/h

Submitted by Kevin Mark on June 14, 2017, 5:03 a.m.

Details

Message ID 20170614050318.16835-1-kmark937@gmail.com
State Accepted
Commit 05feeeb813e9f71f69b6b3a7f33856c609237c06
Headers show

Commit Message

Kevin Mark June 14, 2017, 5:03 a.m.
The input width and height is known at parse time so there's no
reason ow/oh should not be usable when using 0 as the width or
height expression.

Previously in "scale=0:ow" ow would be set to "0" which works,
conveniently, as "scale=0:0" is perfectly valid input but this breaks
down when you do something like "scale=0:ow/4" which one could
reasonably expect to work as well, but does not as ow is 0 not the
real value.

This change handles the 0 case for w/h immediately so the ow/oh
variables work as expected. Consequently, the rest of the code does
not need to handle 0 input. w/h will always be > 0 or < 0.

The second explicit (int) cast ensures that ow/oh appear as integers
as a user might expect when dealing with pixel dimensions.

Signed-off-by: Kevin Mark <kmark937@gmail.com>
---
 libavfilter/scale.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

Comments

Ronald S. Bultje June 19, 2017, 6:29 p.m.
Hi,

On Wed, Jun 14, 2017 at 1:03 AM, Kevin Mark <kmark937@gmail.com> wrote:

> The input width and height is known at parse time so there's no
> reason ow/oh should not be usable when using 0 as the width or
> height expression.
>
> Previously in "scale=0:ow" ow would be set to "0" which works,
> conveniently, as "scale=0:0" is perfectly valid input but this breaks
> down when you do something like "scale=0:ow/4" which one could
> reasonably expect to work as well, but does not as ow is 0 not the
> real value.
>
> This change handles the 0 case for w/h immediately so the ow/oh
> variables work as expected. Consequently, the rest of the code does
> not need to handle 0 input. w/h will always be > 0 or < 0.
>
> The second explicit (int) cast ensures that ow/oh appear as integers
> as a user might expect when dealing with pixel dimensions.
>
> Signed-off-by: Kevin Mark <kmark937@gmail.com>
> ---
>  libavfilter/scale.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/libavfilter/scale.c b/libavfilter/scale.c
> index 03745ddcb8..eaee95fac6 100644
> --- a/libavfilter/scale.c
> +++ b/libavfilter/scale.c
> @@ -158,19 +158,19 @@ int ff_scale_eval_dimensions(void *log_ctx,
>      av_expr_parse_and_eval(&res, (expr = w_expr),
>                             names, var_values,
>                             NULL, NULL, NULL, NULL, NULL, 0, log_ctx);
> -    eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
> +    eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0
> ? inlink->w : (int) res;
>
>      if ((ret = av_expr_parse_and_eval(&res, (expr = h_expr),
>                                        names, var_values,
>                                        NULL, NULL, NULL, NULL, NULL, 0,
> log_ctx)) < 0)
>          goto fail;
> -    eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = res;
> +    eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = (int) res == 0
> ? inlink->h : (int) res;
>      /* evaluate again the width, as it may depend on the output height */
>      if ((ret = av_expr_parse_and_eval(&res, (expr = w_expr),
>                                        names, var_values,
>                                        NULL, NULL, NULL, NULL, NULL, 0,
> log_ctx)) < 0)
>          goto fail;
> -    eval_w = res;
> +    eval_w = (int) res == 0 ? inlink->w : (int) res;
>
>      w = eval_w;
>      h = eval_h;
> @@ -186,13 +186,10 @@ int ff_scale_eval_dimensions(void *log_ctx,
>          factor_h = -h;
>      }
>
> -    if (w < 0 && h < 0)
> -        eval_w = eval_h = 0;
> -
> -    if (!(w = eval_w))
> +    if (w < 0 && h < 0) {
>          w = inlink->w;
> -    if (!(h = eval_h))
>          h = inlink->h;
> +    }
>
>      /* Make sure that the result is divisible by the factor we determined
>       * earlier. If no factor was set, it is nothing will happen as the
> default
> --
> 2.13.1


Pushed.

Ronald

Patch hide | download patch | download mbox

diff --git a/libavfilter/scale.c b/libavfilter/scale.c
index 03745ddcb8..eaee95fac6 100644
--- a/libavfilter/scale.c
+++ b/libavfilter/scale.c
@@ -158,19 +158,19 @@  int ff_scale_eval_dimensions(void *log_ctx,
     av_expr_parse_and_eval(&res, (expr = w_expr),
                            names, var_values,
                            NULL, NULL, NULL, NULL, NULL, 0, log_ctx);
-    eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
+    eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0 ? inlink->w : (int) res;
 
     if ((ret = av_expr_parse_and_eval(&res, (expr = h_expr),
                                       names, var_values,
                                       NULL, NULL, NULL, NULL, NULL, 0, log_ctx)) < 0)
         goto fail;
-    eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = res;
+    eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = (int) res == 0 ? inlink->h : (int) res;
     /* evaluate again the width, as it may depend on the output height */
     if ((ret = av_expr_parse_and_eval(&res, (expr = w_expr),
                                       names, var_values,
                                       NULL, NULL, NULL, NULL, NULL, 0, log_ctx)) < 0)
         goto fail;
-    eval_w = res;
+    eval_w = (int) res == 0 ? inlink->w : (int) res;
 
     w = eval_w;
     h = eval_h;
@@ -186,13 +186,10 @@  int ff_scale_eval_dimensions(void *log_ctx,
         factor_h = -h;
     }
 
-    if (w < 0 && h < 0)
-        eval_w = eval_h = 0;
-
-    if (!(w = eval_w))
+    if (w < 0 && h < 0) {
         w = inlink->w;
-    if (!(h = eval_h))
         h = inlink->h;
+    }
 
     /* Make sure that the result is divisible by the factor we determined
      * earlier. If no factor was set, it is nothing will happen as the default