diff mbox series

[FFmpeg-devel,2/8] lavfi/vf_vpp_qsv: handle NULL pointer when evaluating an expression

Message ID 20230109071210.1829699-2-haihao.xiang@intel.com
State Accepted
Commit 129530e91707b6451a28580bca147031996993af
Headers show
Series [FFmpeg-devel,1/8] lavfi/vf_vpp_qsv: add "a", "dar" and "sar" variables | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Xiang, Haihao Jan. 9, 2023, 7:12 a.m. UTC 
From: Haihao Xiang <haihao.xiang@intel.com>

This patch provides default value if the expression is NULL.

This is in preparation for reusing the code for other QSV filters.

Signed-off-by: Haihao Xiang <haihao.xiang@intel.com>
---
 libavfilter/vf_vpp_qsv.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

Comments

Andreas Rheinhardt Jan. 12, 2023, 4:01 a.m. UTC | #1 
Xiang, Haihao:
> From: Haihao Xiang <haihao.xiang@intel.com>
> 
> This patch provides default value if the expression is NULL.
> 
> This is in preparation for reusing the code for other QSV filters.
> 
> Signed-off-by: Haihao Xiang <haihao.xiang@intel.com>
> ---
>  libavfilter/vf_vpp_qsv.c | 36 ++++++++++++++++++++----------------
>  1 file changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> index 8d4227f841..3a0a395732 100644
> --- a/libavfilter/vf_vpp_qsv.c
> +++ b/libavfilter/vf_vpp_qsv.c
> @@ -172,14 +172,19 @@ enum var_name {
>  static int eval_expr(AVFilterContext *ctx)
>  {
>  #define PASS_EXPR(e, s) {\
> -    ret = av_expr_parse(&e, s, var_names, NULL, NULL, NULL, NULL, 0, ctx); \
> -    if (ret < 0) {\
> -        av_log(ctx, AV_LOG_ERROR, "Error when passing '%s'.\n", s);\
> -        goto release;\
> +    if (s) {\
> +        ret = av_expr_parse(&e, s, var_names, NULL, NULL, NULL, NULL, 0, ctx); \
> +        if (ret < 0) {                                                  \
> +            av_log(ctx, AV_LOG_ERROR, "Error when passing '%s'.\n", s); \
> +            goto release;                                               \
> +        }                                                               \
>      }\
>  }
> -#define CALC_EXPR(e, v, i) {\
> -    i = v = av_expr_eval(e, var_values, NULL); \
> +#define CALC_EXPR(e, v, i, d) {\
> +    if (e)\
> +        i = v = av_expr_eval(e, var_values, NULL);      \
> +    else\
> +        i = v = d;\
>  }
>      VPPContext *vpp = ctx->priv;
>      double  var_values[VAR_VARS_NB] = { NAN };
> @@ -209,30 +214,29 @@ static int eval_expr(AVFilterContext *ctx)
>      var_values[VAR_DAR] = var_values[VAR_A] * var_values[VAR_SAR];
>  
>      /* crop params */
> -    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w);
> -    CALC_EXPR(ch_expr, var_values[VAR_CH], vpp->crop_h);
> +    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w, var_values[VAR_IW]);
> +    CALC_EXPR(ch_expr, var_values[VAR_CH], vpp->crop_h, var_values[VAR_IH]);
>  
>      /* calc again in case cw is relative to ch */
> -    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w);
> +    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w, var_values[VAR_IW]);
>  
>      CALC_EXPR(w_expr,
>              var_values[VAR_OUT_W] = var_values[VAR_OW] = var_values[VAR_W],
> -            vpp->out_width);
> +            vpp->out_width, var_values[VAR_CW]);
>      CALC_EXPR(h_expr,
>              var_values[VAR_OUT_H] = var_values[VAR_OH] = var_values[VAR_H],
> -            vpp->out_height);
> +            vpp->out_height, var_values[VAR_CH]);
>  
>      /* calc again in case ow is relative to oh */
>      CALC_EXPR(w_expr,
>              var_values[VAR_OUT_W] = var_values[VAR_OW] = var_values[VAR_W],
> -            vpp->out_width);
> +            vpp->out_width, var_values[VAR_CW]);
>  
> -
> -    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x);
> -    CALC_EXPR(cy_expr, var_values[VAR_CY], vpp->crop_y);
> +    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x, (var_values[VAR_IW] - var_values[VAR_OW]) / 2);
> +    CALC_EXPR(cy_expr, var_values[VAR_CY], vpp->crop_y, (var_values[VAR_IH] - var_values[VAR_OH]) / 2);
>  
>      /* calc again in case cx is relative to cy */
> -    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x);
> +    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x, (var_values[VAR_IW] - var_values[VAR_OW]) / 2);
>  
>      if ((vpp->crop_w != var_values[VAR_IW]) || (vpp->crop_h != var_values[VAR_IH]))
>          vpp->use_crop = 1;

Does this add runtime checks to cases where there are not necessary?

- Andreas
Xiang, Haihao Jan. 12, 2023, 6:20 a.m. UTC | #2 
On Do, 2023-01-12 at 05:01 +0100, Andreas Rheinhardt wrote:
> Xiang, Haihao:
> > From: Haihao Xiang <haihao.xiang@intel.com>
> > 
> > This patch provides default value if the expression is NULL.
> > 
> > This is in preparation for reusing the code for other QSV filters.
> > 
> > Signed-off-by: Haihao Xiang <haihao.xiang@intel.com>
> > ---
> >  libavfilter/vf_vpp_qsv.c | 36 ++++++++++++++++++++----------------
> >  1 file changed, 20 insertions(+), 16 deletions(-)
> > 
> > diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> > index 8d4227f841..3a0a395732 100644
> > --- a/libavfilter/vf_vpp_qsv.c
> > +++ b/libavfilter/vf_vpp_qsv.c
> > @@ -172,14 +172,19 @@ enum var_name {
> >  static int eval_expr(AVFilterContext *ctx)
> >  {
> >  #define PASS_EXPR(e, s) {\
> > -    ret = av_expr_parse(&e, s, var_names, NULL, NULL, NULL, NULL, 0, ctx);
> > \
> > -    if (ret < 0) {\
> > -        av_log(ctx, AV_LOG_ERROR, "Error when passing '%s'.\n", s);\
> > -        goto release;\
> > +    if (s) {\
> > +        ret = av_expr_parse(&e, s, var_names, NULL, NULL, NULL, NULL, 0,
> > ctx); \
> > +        if (ret < 0) {                                                  \
> > +            av_log(ctx, AV_LOG_ERROR, "Error when passing '%s'.\n", s); \
> > +            goto release;                                               \
> > +        }                                                               \
> >      }\
> >  }
> > -#define CALC_EXPR(e, v, i) {\
> > -    i = v = av_expr_eval(e, var_values, NULL); \
> > +#define CALC_EXPR(e, v, i, d) {\
> > +    if (e)\
> > +        i = v = av_expr_eval(e, var_values, NULL);      \
> > +    else\
> > +        i = v = d;\
> >  }
> >      VPPContext *vpp = ctx->priv;
> >      double  var_values[VAR_VARS_NB] = { NAN };
> > @@ -209,30 +214,29 @@ static int eval_expr(AVFilterContext *ctx)
> >      var_values[VAR_DAR] = var_values[VAR_A] * var_values[VAR_SAR];
> >  
> >      /* crop params */
> > -    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w);
> > -    CALC_EXPR(ch_expr, var_values[VAR_CH], vpp->crop_h);
> > +    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w,
> > var_values[VAR_IW]);
> > +    CALC_EXPR(ch_expr, var_values[VAR_CH], vpp->crop_h,
> > var_values[VAR_IH]);
> >  
> >      /* calc again in case cw is relative to ch */
> > -    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w);
> > +    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w,
> > var_values[VAR_IW]);
> >  
> >      CALC_EXPR(w_expr,
> >              var_values[VAR_OUT_W] = var_values[VAR_OW] = var_values[VAR_W],
> > -            vpp->out_width);
> > +            vpp->out_width, var_values[VAR_CW]);
> >      CALC_EXPR(h_expr,
> >              var_values[VAR_OUT_H] = var_values[VAR_OH] = var_values[VAR_H],
> > -            vpp->out_height);
> > +            vpp->out_height, var_values[VAR_CH]);
> >  
> >      /* calc again in case ow is relative to oh */
> >      CALC_EXPR(w_expr,
> >              var_values[VAR_OUT_W] = var_values[VAR_OW] = var_values[VAR_W],
> > -            vpp->out_width);
> > +            vpp->out_width, var_values[VAR_CW]);
> >  
> > -
> > -    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x);
> > -    CALC_EXPR(cy_expr, var_values[VAR_CY], vpp->crop_y);
> > +    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x, (var_values[VAR_IW]
> > - var_values[VAR_OW]) / 2);
> > +    CALC_EXPR(cy_expr, var_values[VAR_CY], vpp->crop_y, (var_values[VAR_IH]
> > - var_values[VAR_OH]) / 2);
> >  
> >      /* calc again in case cx is relative to cy */
> > -    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x);
> > +    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x, (var_values[VAR_IW]
> > - var_values[VAR_OW]) / 2);
> >  
> >      if ((vpp->crop_w != var_values[VAR_IW]) || (vpp->crop_h !=
> > var_values[VAR_IH]))
> >          vpp->use_crop = 1;
> 
> Does this add runtime checks to cases where there are not necessary?

No. It is irrelated to runtime check. It is used to make sure each variable have
a right default value even if the corresponding expression is NULL.

E.g. vpp->cw is NULL when we re-use functions and data structures in
vf_vpp_qsv.c and qsvvpp.c for deinterlace_qsv filter.

Thanks
Haihao


> 
> - Andreas
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Xiang, Haihao Jan. 16, 2023, 4:34 a.m. UTC | #3 
On Do, 2023-01-12 at 06:20 +0000, Xiang, Haihao wrote:
> On Do, 2023-01-12 at 05:01 +0100, Andreas Rheinhardt wrote:
> > Xiang, Haihao:
> > > From: Haihao Xiang <haihao.xiang@intel.com>
> > > 
> > > This patch provides default value if the expression is NULL.
> > > 
> > > This is in preparation for reusing the code for other QSV filters.
> > > 
> > > Signed-off-by: Haihao Xiang <haihao.xiang@intel.com>
> > > ---
> > >  libavfilter/vf_vpp_qsv.c | 36 ++++++++++++++++++++----------------
> > >  1 file changed, 20 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> > > index 8d4227f841..3a0a395732 100644
> > > --- a/libavfilter/vf_vpp_qsv.c
> > > +++ b/libavfilter/vf_vpp_qsv.c
> > > @@ -172,14 +172,19 @@ enum var_name {
> > >  static int eval_expr(AVFilterContext *ctx)
> > >  {
> > >  #define PASS_EXPR(e, s) {\
> > > -    ret = av_expr_parse(&e, s, var_names, NULL, NULL, NULL, NULL, 0,
> > > ctx);
> > > \
> > > -    if (ret < 0) {\
> > > -        av_log(ctx, AV_LOG_ERROR, "Error when passing '%s'.\n", s);\
> > > -        goto release;\
> > > +    if (s) {\
> > > +        ret = av_expr_parse(&e, s, var_names, NULL, NULL, NULL, NULL, 0,
> > > ctx); \
> > > +        if (ret < 0) {                                                  \
> > > +            av_log(ctx, AV_LOG_ERROR, "Error when passing '%s'.\n", s); \
> > > +            goto release;                                               \
> > > +        }                                                               \
> > >      }\
> > >  }
> > > -#define CALC_EXPR(e, v, i) {\
> > > -    i = v = av_expr_eval(e, var_values, NULL); \
> > > +#define CALC_EXPR(e, v, i, d) {\
> > > +    if (e)\
> > > +        i = v = av_expr_eval(e, var_values, NULL);      \
> > > +    else\
> > > +        i = v = d;\
> > >  }
> > >      VPPContext *vpp = ctx->priv;
> > >      double  var_values[VAR_VARS_NB] = { NAN };
> > > @@ -209,30 +214,29 @@ static int eval_expr(AVFilterContext *ctx)
> > >      var_values[VAR_DAR] = var_values[VAR_A] * var_values[VAR_SAR];
> > >  
> > >      /* crop params */
> > > -    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w);
> > > -    CALC_EXPR(ch_expr, var_values[VAR_CH], vpp->crop_h);
> > > +    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w,
> > > var_values[VAR_IW]);
> > > +    CALC_EXPR(ch_expr, var_values[VAR_CH], vpp->crop_h,
> > > var_values[VAR_IH]);
> > >  
> > >      /* calc again in case cw is relative to ch */
> > > -    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w);
> > > +    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w,
> > > var_values[VAR_IW]);
> > >  
> > >      CALC_EXPR(w_expr,
> > >              var_values[VAR_OUT_W] = var_values[VAR_OW] =
> > > var_values[VAR_W],
> > > -            vpp->out_width);
> > > +            vpp->out_width, var_values[VAR_CW]);
> > >      CALC_EXPR(h_expr,
> > >              var_values[VAR_OUT_H] = var_values[VAR_OH] =
> > > var_values[VAR_H],
> > > -            vpp->out_height);
> > > +            vpp->out_height, var_values[VAR_CH]);
> > >  
> > >      /* calc again in case ow is relative to oh */
> > >      CALC_EXPR(w_expr,
> > >              var_values[VAR_OUT_W] = var_values[VAR_OW] =
> > > var_values[VAR_W],
> > > -            vpp->out_width);
> > > +            vpp->out_width, var_values[VAR_CW]);
> > >  
> > > -
> > > -    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x);
> > > -    CALC_EXPR(cy_expr, var_values[VAR_CY], vpp->crop_y);
> > > +    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x,
> > > (var_values[VAR_IW]
> > > - var_values[VAR_OW]) / 2);
> > > +    CALC_EXPR(cy_expr, var_values[VAR_CY], vpp->crop_y,
> > > (var_values[VAR_IH]
> > > - var_values[VAR_OH]) / 2);
> > >  
> > >      /* calc again in case cx is relative to cy */
> > > -    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x);
> > > +    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x,
> > > (var_values[VAR_IW]
> > > - var_values[VAR_OW]) / 2);
> > >  
> > >      if ((vpp->crop_w != var_values[VAR_IW]) || (vpp->crop_h !=
> > > var_values[VAR_IH]))
> > >          vpp->use_crop = 1;
> > 
> > Does this add runtime checks to cases where there are not necessary?
> 
> No. It is irrelated to runtime check. It is used to make sure each variable
> have
> a right default value even if the corresponding expression is NULL.
> 
> E.g. vpp->cw is NULL when we re-use functions and data structures in
> vf_vpp_qsv.c and qsvvpp.c for deinterlace_qsv filter.
> 

Will apply this patchset if no more comment.

-Haihao
diff mbox series

Patch

diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
index 8d4227f841..3a0a395732 100644
--- a/libavfilter/vf_vpp_qsv.c
+++ b/libavfilter/vf_vpp_qsv.c
@@ -172,14 +172,19 @@  enum var_name {
 static int eval_expr(AVFilterContext *ctx)
 {
 #define PASS_EXPR(e, s) {\
-    ret = av_expr_parse(&e, s, var_names, NULL, NULL, NULL, NULL, 0, ctx); \
-    if (ret < 0) {\
-        av_log(ctx, AV_LOG_ERROR, "Error when passing '%s'.\n", s);\
-        goto release;\
+    if (s) {\
+        ret = av_expr_parse(&e, s, var_names, NULL, NULL, NULL, NULL, 0, ctx); \
+        if (ret < 0) {                                                  \
+            av_log(ctx, AV_LOG_ERROR, "Error when passing '%s'.\n", s); \
+            goto release;                                               \
+        }                                                               \
     }\
 }
-#define CALC_EXPR(e, v, i) {\
-    i = v = av_expr_eval(e, var_values, NULL); \
+#define CALC_EXPR(e, v, i, d) {\
+    if (e)\
+        i = v = av_expr_eval(e, var_values, NULL);      \
+    else\
+        i = v = d;\
 }
     VPPContext *vpp = ctx->priv;
     double  var_values[VAR_VARS_NB] = { NAN };
@@ -209,30 +214,29 @@  static int eval_expr(AVFilterContext *ctx)
     var_values[VAR_DAR] = var_values[VAR_A] * var_values[VAR_SAR];
 
     /* crop params */
-    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w);
-    CALC_EXPR(ch_expr, var_values[VAR_CH], vpp->crop_h);
+    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w, var_values[VAR_IW]);
+    CALC_EXPR(ch_expr, var_values[VAR_CH], vpp->crop_h, var_values[VAR_IH]);
 
     /* calc again in case cw is relative to ch */
-    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w);
+    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w, var_values[VAR_IW]);
 
     CALC_EXPR(w_expr,
             var_values[VAR_OUT_W] = var_values[VAR_OW] = var_values[VAR_W],
-            vpp->out_width);
+            vpp->out_width, var_values[VAR_CW]);
     CALC_EXPR(h_expr,
             var_values[VAR_OUT_H] = var_values[VAR_OH] = var_values[VAR_H],
-            vpp->out_height);
+            vpp->out_height, var_values[VAR_CH]);
 
     /* calc again in case ow is relative to oh */
     CALC_EXPR(w_expr,
             var_values[VAR_OUT_W] = var_values[VAR_OW] = var_values[VAR_W],
-            vpp->out_width);
+            vpp->out_width, var_values[VAR_CW]);
 
-
-    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x);
-    CALC_EXPR(cy_expr, var_values[VAR_CY], vpp->crop_y);
+    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x, (var_values[VAR_IW] - var_values[VAR_OW]) / 2);
+    CALC_EXPR(cy_expr, var_values[VAR_CY], vpp->crop_y, (var_values[VAR_IH] - var_values[VAR_OH]) / 2);
 
     /* calc again in case cx is relative to cy */
-    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x);
+    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x, (var_values[VAR_IW] - var_values[VAR_OW]) / 2);
 
     if ((vpp->crop_w != var_values[VAR_IW]) || (vpp->crop_h != var_values[VAR_IH]))
         vpp->use_crop = 1;