[FFmpeg-devel] [PATCH 1/8] lavfi/vf_vpp_qsv: add "a", "dar" and "sar" variables

[FFmpeg-devel] [PATCH 1/8] lavfi/vf_vpp_qsv: add "a", "dar" and "sar" variables

[Xiang, Haihao]

From: Haihao Xiang <haihao.xiang@intel.com>

Also fix the naming style in enum var_name.

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 | 49 ++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
index 317ae06c12..8d4227f841 100644
--- a/libavfilter/vf_vpp_qsv.c
+++ b/libavfilter/vf_vpp_qsv.c
@@ -150,18 +150,22 @@ static const char *const var_names[] = {
     "ch",
     "cx",
     "cy",
+    "a", "dar",
+    "sar",
     NULL
 };
 
 enum var_name {
-    VAR_iW, VAR_IN_W,
-    VAR_iH, VAR_IN_H,
-    VAR_oW, VAR_OUT_W, VAR_W,
-    VAR_oH, VAR_OUT_H, VAR_H,
-    CW,
-    CH,
-    CX,
-    CY,
+    VAR_IW, VAR_IN_W,
+    VAR_IH, VAR_IN_H,
+    VAR_OW, VAR_OUT_W, VAR_W,
+    VAR_OH, VAR_OUT_H, VAR_H,
+    VAR_CW,
+    VAR_CH,
+    VAR_CX,
+    VAR_CY,
+    VAR_A, VAR_DAR,
+    VAR_SAR,
     VAR_VARS_NB
 };
 
@@ -193,39 +197,44 @@ static int eval_expr(AVFilterContext *ctx)
     PASS_EXPR(cx_expr, vpp->cx);
     PASS_EXPR(cy_expr, vpp->cy);
 
-    var_values[VAR_iW] =
+    var_values[VAR_IW] =
     var_values[VAR_IN_W] = ctx->inputs[0]->w;
 
-    var_values[VAR_iH] =
+    var_values[VAR_IH] =
     var_values[VAR_IN_H] = ctx->inputs[0]->h;
 
+    var_values[VAR_A] = (double)var_values[VAR_IN_W] / var_values[VAR_IN_H];
+    var_values[VAR_SAR] = ctx->inputs[0]->sample_aspect_ratio.num ?
+        (double)ctx->inputs[0]->sample_aspect_ratio.num / ctx->inputs[0]->sample_aspect_ratio.den : 1;
+    var_values[VAR_DAR] = var_values[VAR_A] * var_values[VAR_SAR];
+
     /* crop params */
-    CALC_EXPR(cw_expr, var_values[CW], vpp->crop_w);
-    CALC_EXPR(ch_expr, var_values[CH], vpp->crop_h);
+    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w);
+    CALC_EXPR(ch_expr, var_values[VAR_CH], vpp->crop_h);
 
     /* calc again in case cw is relative to ch */
-    CALC_EXPR(cw_expr, var_values[CW], vpp->crop_w);
+    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w);
 
     CALC_EXPR(w_expr,
-            var_values[VAR_OUT_W] = var_values[VAR_oW] = var_values[VAR_W],
+            var_values[VAR_OUT_W] = var_values[VAR_OW] = var_values[VAR_W],
             vpp->out_width);
     CALC_EXPR(h_expr,
-            var_values[VAR_OUT_H] = var_values[VAR_oH] = var_values[VAR_H],
+            var_values[VAR_OUT_H] = var_values[VAR_OH] = var_values[VAR_H],
             vpp->out_height);
 
     /* 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],
+            var_values[VAR_OUT_W] = var_values[VAR_OW] = var_values[VAR_W],
             vpp->out_width);
 
 
-    CALC_EXPR(cx_expr, var_values[CX], vpp->crop_x);
-    CALC_EXPR(cy_expr, var_values[CY], vpp->crop_y);
+    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x);
+    CALC_EXPR(cy_expr, var_values[VAR_CY], vpp->crop_y);
 
     /* calc again in case cx is relative to cy */
-    CALC_EXPR(cx_expr, var_values[CX], vpp->crop_x);
+    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x);
 
-    if ((vpp->crop_w != var_values[VAR_iW]) || (vpp->crop_h != var_values[VAR_iH]))
+    if ((vpp->crop_w != var_values[VAR_IW]) || (vpp->crop_h != var_values[VAR_IH]))
         vpp->use_crop = 1;
 
 release:
-- 
2.25.1

_______________________________________________
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".

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

[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;
-- 
2.25.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 3/8] lavfi/vf_vpp_qsv: allow special values for the output video dimensions

[Xiang, Haihao]

From: Haihao Xiang <haihao.xiang@intel.com>

Special values are:
0 = original width/height
-1 = keep original aspect

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 | 47 ++++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
index 3a0a395732..cf11cd7fdc 100644
--- a/libavfilter/vf_vpp_qsv.c
+++ b/libavfilter/vf_vpp_qsv.c
@@ -59,6 +59,11 @@ typedef struct VPPContext{
     mfxExtVPPMirroring mirroring_conf;
     mfxExtVPPScaling scale_conf;
 
+    /**
+     * New dimensions. Special values are:
+     *   0 = original width/height
+     *  -1 = keep original aspect
+     */
     int out_width;
     int out_height;
     /**
@@ -127,10 +132,10 @@ static const AVOption options[] = {
     { "cx",   "set the x crop area expression",       OFFSET(cx), AV_OPT_TYPE_STRING, { .str = "(in_w-out_w)/2" }, 0, 0, FLAGS },
     { "cy",   "set the y crop area expression",       OFFSET(cy), AV_OPT_TYPE_STRING, { .str = "(in_h-out_h)/2" }, 0, 0, FLAGS },
 
-    { "w",      "Output video width",  OFFSET(ow), AV_OPT_TYPE_STRING, { .str="cw" }, 0, 255, .flags = FLAGS },
-    { "width",  "Output video width",  OFFSET(ow), AV_OPT_TYPE_STRING, { .str="cw" }, 0, 255, .flags = FLAGS },
-    { "h",      "Output video height", OFFSET(oh), AV_OPT_TYPE_STRING, { .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
-    { "height", "Output video height", OFFSET(oh), AV_OPT_TYPE_STRING, { .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
+    { "w",      "Output video width(0=input video width, -1=keep input video aspect)",  OFFSET(ow), AV_OPT_TYPE_STRING, { .str="cw" }, 0, 255, .flags = FLAGS },
+    { "width",  "Output video width(0=input video width, -1=keep input video aspect)",  OFFSET(ow), AV_OPT_TYPE_STRING, { .str="cw" }, 0, 255, .flags = FLAGS },
+    { "h",      "Output video height(0=input video height, -1=keep input video aspect)", OFFSET(oh), AV_OPT_TYPE_STRING, { .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
+    { "height", "Output video height(0=input video height, -1=keep input video aspect)", OFFSET(oh), AV_OPT_TYPE_STRING, { .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
     { "format", "Output pixel format", OFFSET(output_format_str), AV_OPT_TYPE_STRING, { .str = "same" }, .flags = FLAGS },
     { "async_depth", "Internal parallelization depth, the higher the value the higher the latency.", OFFSET(async_depth), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, .flags = FLAGS },
     { "scale_mode", "scale & format conversion mode: 0=auto, 1=low power, 2=high quality", OFFSET(scale_mode), AV_OPT_TYPE_INT, { .i64 = MFX_SCALING_MODE_DEFAULT }, MFX_SCALING_MODE_DEFAULT, MFX_SCALING_MODE_QUALITY, .flags = FLAGS, "scale mode" },
@@ -276,6 +281,7 @@ static int config_input(AVFilterLink *inlink)
     AVFilterContext *ctx = inlink->dst;
     VPPContext      *vpp = ctx->priv;
     int              ret;
+    int64_t          ow, oh;
 
     if (vpp->framerate.den == 0 || vpp->framerate.num == 0)
         vpp->framerate = inlink->frame_rate;
@@ -289,11 +295,38 @@ static int config_input(AVFilterLink *inlink)
         return ret;
     }
 
-    if (vpp->out_height == 0 || vpp->out_width == 0) {
-        vpp->out_width  = inlink->w;
-        vpp->out_height = inlink->h;
+    ow = vpp->out_width;
+    oh = vpp->out_height;
+
+    /* sanity check params */
+    if (ow <  -1 || oh <  -1) {
+        av_log(ctx, AV_LOG_ERROR, "Size values less than -1 are not acceptable.\n");
+        return AVERROR(EINVAL);
     }
 
+    if (ow == -1 && oh == -1)
+        vpp->out_width = vpp->out_height = 0;
+
+    if (!(ow = vpp->out_width))
+        ow = inlink->w;
+
+    if (!(oh = vpp->out_height))
+        oh = inlink->h;
+
+    if (ow == -1)
+        ow = av_rescale(oh, inlink->w, inlink->h);
+
+    if (oh == -1)
+        oh = av_rescale(ow, inlink->h, inlink->w);
+
+    if (ow > INT_MAX || oh > INT_MAX ||
+        (oh * inlink->w) > INT_MAX  ||
+        (ow * inlink->h) > INT_MAX)
+        av_log(ctx, AV_LOG_ERROR, "Rescaled value for width or height is too big.\n");
+
+    vpp->out_width = ow;
+    vpp->out_height = oh;
+
     if (vpp->use_crop) {
         vpp->crop_x = FFMAX(vpp->crop_x, 0);
         vpp->crop_y = FFMAX(vpp->crop_y, 0);
-- 
2.25.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 4/8] lavfi/vf_vpp_qsv: add vpp_preinit callback

[Xiang, Haihao]

From: Haihao Xiang <haihao.xiang@intel.com>

Set the expected default value for options in this callback, hence we
have the right values even if these options are not included in the
option arrray.

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 | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
index cf11cd7fdc..2a7b06fa33 100644
--- a/libavfilter/vf_vpp_qsv.c
+++ b/libavfilter/vf_vpp_qsv.c
@@ -259,6 +259,19 @@ release:
     return ret;
 }
 
+static av_cold int vpp_preinit(AVFilterContext *ctx)
+{
+    VPPContext  *vpp  = ctx->priv;
+    /* For AV_OPT_TYPE_STRING options, NULL is handled in other way so
+     * we needn't set default value here
+     */
+    vpp->saturation = 1.0;
+    vpp->contrast = 1.0;
+    vpp->transpose = -1;
+
+    return 0;
+}
+
 static av_cold int vpp_init(AVFilterContext *ctx)
 {
     VPPContext  *vpp  = ctx->priv;
@@ -683,6 +696,7 @@ const AVFilter ff_vf_vpp_qsv = {
     .name          = "vpp_qsv",
     .description   = NULL_IF_CONFIG_SMALL("Quick Sync Video VPP."),
     .priv_size     = sizeof(VPPContext),
+    .preinit       = vpp_preinit,
     .init          = vpp_init,
     .uninit        = vpp_uninit,
     FILTER_INPUTS(vpp_inputs),
-- 
2.25.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 5/8] lavfi/vf_vpp_qsv: add has_passthrough flag in VPPContext

[Xiang, Haihao]

From: Haihao Xiang <haihao.xiang@intel.com>

QSV filters may set this flag in preinit callback to turn on / off pass
through mode

This is in preparation for reusing the code for other QSV filters. E.g.
scale_qsv filter doesn't support pass through mode.

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

diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
index 2a7b06fa33..b04307b644 100644
--- a/libavfilter/vf_vpp_qsv.c
+++ b/libavfilter/vf_vpp_qsv.c
@@ -102,6 +102,7 @@ typedef struct VPPContext{
 
     int async_depth;
     int eof;
+    int has_passthrough;        /* apply pass through mode if possible */
 } VPPContext;
 
 static const AVOption options[] = {
@@ -269,6 +270,8 @@ static av_cold int vpp_preinit(AVFilterContext *ctx)
     vpp->contrast = 1.0;
     vpp->transpose = -1;
 
+    vpp->has_passthrough = 1;
+
     return 0;
 }
 
@@ -552,7 +555,8 @@ static int config_output(AVFilterLink *outlink)
 
     if (vpp->use_frc || vpp->use_crop || vpp->deinterlace || vpp->denoise ||
         vpp->detail || vpp->procamp || vpp->rotate || vpp->hflip ||
-        inlink->w != outlink->w || inlink->h != outlink->h || in_format != vpp->out_format)
+        inlink->w != outlink->w || inlink->h != outlink->h || in_format != vpp->out_format ||
+        !vpp->has_passthrough)
         return ff_qsvvpp_create(ctx, &vpp->qsv, &param);
     else {
         av_log(ctx, AV_LOG_VERBOSE, "qsv vpp pass through mode.\n");
-- 
2.25.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 6/8] lavfi/vf_vpp_qsv: check output format string against NULL pointer

[Xiang, Haihao]

From: Haihao Xiang <haihao.xiang@intel.com>

This is in preparation for reusing the code for other QSV filters. E.g.
deinterlacing_qsv may have an option array without format option

Signed-off-by: Haihao Xiang <haihao.xiang@intel.com>
---
 libavfilter/vf_vpp_qsv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
index b04307b644..4aad856a2e 100644
--- a/libavfilter/vf_vpp_qsv.c
+++ b/libavfilter/vf_vpp_qsv.c
@@ -279,7 +279,7 @@ static av_cold int vpp_init(AVFilterContext *ctx)
 {
     VPPContext  *vpp  = ctx->priv;
 
-    if (!strcmp(vpp->output_format_str, "same")) {
+    if (!vpp->output_format_str || !strcmp(vpp->output_format_str, "same")) {
         vpp->out_format = AV_PIX_FMT_NONE;
     } else {
         vpp->out_format = av_get_pix_fmt(vpp->output_format_str);
-- 
2.25.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 7/8] lavfi/qsvvpp: set output frame durations

[Xiang, Haihao]

From: Haihao Xiang <haihao.xiang@intel.com>

Signed-off-by: Haihao Xiang <haihao.xiang@intel.com>
---
 libavfilter/qsvvpp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c
index 064b105a17..f074bf9978 100644
--- a/libavfilter/qsvvpp.c
+++ b/libavfilter/qsvvpp.c
@@ -503,6 +503,11 @@ static QSVFrame *query_frame(QSVVPPContext *s, AVFilterLink *outlink)
             return NULL;
     }
 
+    if (outlink->frame_rate.num && outlink->frame_rate.den)
+        out_frame->frame->duration = av_rescale_q(1, av_inv_q(outlink->frame_rate), outlink->time_base);
+    else
+        out_frame->frame->duration = 0;
+
     out_frame->frame->width  = outlink->w;
     out_frame->frame->height = outlink->h;
     out_frame->surface.Info = s->vpp_param.vpp.Out;
-- 
2.25.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 8/8] lavfi/qsv: use QSVVPPContext as base context in vf_vpp_qsv/vf_overlay_qsv

[Xiang, Haihao]

From: Haihao Xiang <haihao.xiang@intel.com>

The same members between QSVVPPContext and VPPContext are removed from
VPPContext, and async_depth is moved from QSVVPPParam to QSVVPPContext
so that all QSV filters using QSVVPPContext may support async depth.

In addition, we may use QSVVPPContext as base context in other QSV
filters in the future so that we may re-use functions defined in
qsvvpp.c for other QSV filters.

This commit shouldn't change the functionality of vpp_qsv / overlay_qsv.

Signed-off-by: Haihao Xiang <haihao.xiang@intel.com>
---
 libavfilter/qsvvpp.c         | 25 ++++++++-----------------
 libavfilter/qsvvpp.h         |  8 ++++----
 libavfilter/vf_overlay_qsv.c | 11 +++++------
 libavfilter/vf_vpp_qsv.c     | 35 ++++++++++++++---------------------
 4 files changed, 31 insertions(+), 48 deletions(-)

diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c
index f074bf9978..7e64944f2c 100644
--- a/libavfilter/qsvvpp.c
+++ b/libavfilter/qsvvpp.c
@@ -687,15 +687,11 @@ static int init_vpp_session(AVFilterContext *avctx, QSVVPPContext *s)
     return 0;
 }
 
-int ff_qsvvpp_create(AVFilterContext *avctx, QSVVPPContext **vpp, QSVVPPParam *param)
+int ff_qsvvpp_init(AVFilterContext *avctx, QSVVPPParam *param)
 {
     int i;
     int ret;
-    QSVVPPContext *s;
-
-    s = av_mallocz(sizeof(*s));
-    if (!s)
-        return AVERROR(ENOMEM);
+    QSVVPPContext *s = avctx->priv;
 
     s->filter_frame  = param->filter_frame;
     if (!s->filter_frame)
@@ -767,14 +763,13 @@ int ff_qsvvpp_create(AVFilterContext *avctx, QSVVPPContext **vpp, QSVVPPParam *p
     s->got_frame = 0;
 
     /** keep fifo size at least 1. Even when async_depth is 0, fifo is used. */
-    s->async_fifo  = av_fifo_alloc2(param->async_depth + 1, sizeof(QSVAsyncFrame), 0);
-    s->async_depth = param->async_depth;
+    s->async_fifo  = av_fifo_alloc2(s->async_depth + 1, sizeof(QSVAsyncFrame), 0);
     if (!s->async_fifo) {
         ret = AVERROR(ENOMEM);
         goto failed;
     }
 
-    s->vpp_param.AsyncDepth = param->async_depth;
+    s->vpp_param.AsyncDepth = s->async_depth;
 
     if (IS_SYSTEM_MEMORY(s->in_mem_mode))
         s->vpp_param.IOPattern |= MFX_IOPATTERN_IN_SYSTEM_MEMORY;
@@ -805,25 +800,22 @@ int ff_qsvvpp_create(AVFilterContext *avctx, QSVVPPContext **vpp, QSVVPPParam *p
     } else if (ret > 0)
         ff_qsvvpp_print_warning(avctx, ret, "Warning When creating qsvvpp");
 
-    *vpp = s;
     return 0;
 
 failed:
-    ff_qsvvpp_free(&s);
+    ff_qsvvpp_close(avctx);
 
     return ret;
 }
 
-int ff_qsvvpp_free(QSVVPPContext **vpp)
+int ff_qsvvpp_close(AVFilterContext *avctx)
 {
-    QSVVPPContext *s = *vpp;
-
-    if (!s)
-        return 0;
+    QSVVPPContext *s = avctx->priv;
 
     if (s->session) {
         MFXVideoVPP_Close(s->session);
         MFXClose(s->session);
+        s->session = NULL;
     }
 
     /* release all the resources */
@@ -836,7 +828,6 @@ int ff_qsvvpp_free(QSVVPPContext **vpp)
 #endif
     av_freep(&s->frame_infos);
     av_fifo_freep2(&s->async_fifo);
-    av_freep(vpp);
 
     return 0;
 }
diff --git a/libavfilter/qsvvpp.h b/libavfilter/qsvvpp.h
index 6f7c9bfc15..3b32193744 100644
--- a/libavfilter/qsvvpp.h
+++ b/libavfilter/qsvvpp.h
@@ -53,6 +53,8 @@ typedef struct QSVFrame {
 } QSVFrame;
 
 typedef struct QSVVPPContext {
+    const AVClass      *class;
+
     mfxSession          session;
     int (*filter_frame) (AVFilterLink *outlink, AVFrame *frame); /**< callback */
     enum AVPixelFormat  out_sw_format;   /**< Real output format */
@@ -102,15 +104,13 @@ typedef struct QSVVPPParam {
     /* Crop information for each input, if needed */
     int num_crop;
     QSVVPPCrop *crop;
-
-   int async_depth;
 } QSVVPPParam;
 
 /* create and initialize the QSV session */
-int ff_qsvvpp_create(AVFilterContext *avctx, QSVVPPContext **vpp, QSVVPPParam *param);
+int ff_qsvvpp_init(AVFilterContext *avctx, QSVVPPParam *param);
 
 /* release the resources (eg.surfaces) */
-int ff_qsvvpp_free(QSVVPPContext **vpp);
+int ff_qsvvpp_close(AVFilterContext *avctx);
 
 /* vpp filter frame and call the cb if needed */
 int ff_qsvvpp_filter_frame(QSVVPPContext *vpp, AVFilterLink *inlink, AVFrame *frame);
diff --git a/libavfilter/vf_overlay_qsv.c b/libavfilter/vf_overlay_qsv.c
index 1a2c1b1e96..5bec7dd414 100644
--- a/libavfilter/vf_overlay_qsv.c
+++ b/libavfilter/vf_overlay_qsv.c
@@ -57,10 +57,9 @@ enum var_name {
 };
 
 typedef struct QSVOverlayContext {
-    const AVClass      *class;
+    QSVVPPContext      qsv;
 
     FFFrameSync fs;
-    QSVVPPContext      *qsv;
     QSVVPPParam        qsv_param;
     mfxExtVPPComposite comp_conf;
     double             var_values[VAR_VARS_NB];
@@ -230,14 +229,14 @@ static int config_overlay_input(AVFilterLink *inlink)
 static int process_frame(FFFrameSync *fs)
 {
     AVFilterContext  *ctx = fs->parent;
-    QSVOverlayContext  *s = fs->opaque;
+    QSVVPPContext    *qsv = fs->opaque;
     AVFrame        *frame = NULL;
     int               ret = 0, i;
 
     for (i = 0; i < ctx->nb_inputs; i++) {
         ret = ff_framesync_get_frame(fs, i, &frame, 0);
         if (ret == 0)
-            ret = ff_qsvvpp_filter_frame(s->qsv, ctx->inputs[i], frame);
+            ret = ff_qsvvpp_filter_frame(qsv, ctx->inputs[i], frame);
         if (ret < 0 && ret != AVERROR(EAGAIN))
             break;
     }
@@ -301,7 +300,7 @@ static int config_output(AVFilterLink *outlink)
     if (ret < 0)
         return ret;
 
-    return ff_qsvvpp_create(ctx, &vpp->qsv, &vpp->qsv_param);
+    return ff_qsvvpp_init(ctx, &vpp->qsv_param);
 }
 
 /*
@@ -350,7 +349,7 @@ static av_cold void overlay_qsv_uninit(AVFilterContext *ctx)
 {
     QSVOverlayContext *vpp = ctx->priv;
 
-    ff_qsvvpp_free(&vpp->qsv);
+    ff_qsvvpp_close(ctx);
     ff_framesync_uninit(&vpp->fs);
     av_freep(&vpp->comp_conf.InputStream);
     av_freep(&vpp->qsv_param.ext_buf);
diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
index 4aad856a2e..4a1af7146a 100644
--- a/libavfilter/vf_vpp_qsv.c
+++ b/libavfilter/vf_vpp_qsv.c
@@ -45,9 +45,7 @@
 #define ENH_FILTERS_COUNT (8)
 
 typedef struct VPPContext{
-    const AVClass *class;
-
-    QSVVPPContext *qsv;
+    QSVVPPContext qsv;
 
     /* Video Enhancement Algorithms */
     mfxExtVPPDeinterlacing  deinterlace_conf;
@@ -100,8 +98,6 @@ typedef struct VPPContext{
     char *ow, *oh;
     char *output_format_str;
 
-    int async_depth;
-    int eof;
     int has_passthrough;        /* apply pass through mode if possible */
 } VPPContext;
 
@@ -138,7 +134,7 @@ static const AVOption options[] = {
     { "h",      "Output video height(0=input video height, -1=keep input video aspect)", OFFSET(oh), AV_OPT_TYPE_STRING, { .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
     { "height", "Output video height(0=input video height, -1=keep input video aspect)", OFFSET(oh), AV_OPT_TYPE_STRING, { .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
     { "format", "Output pixel format", OFFSET(output_format_str), AV_OPT_TYPE_STRING, { .str = "same" }, .flags = FLAGS },
-    { "async_depth", "Internal parallelization depth, the higher the value the higher the latency.", OFFSET(async_depth), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, .flags = FLAGS },
+    { "async_depth", "Internal parallelization depth, the higher the value the higher the latency.", OFFSET(qsv.async_depth), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, .flags = FLAGS },
     { "scale_mode", "scale & format conversion mode: 0=auto, 1=low power, 2=high quality", OFFSET(scale_mode), AV_OPT_TYPE_INT, { .i64 = MFX_SCALING_MODE_DEFAULT }, MFX_SCALING_MODE_DEFAULT, MFX_SCALING_MODE_QUALITY, .flags = FLAGS, "scale mode" },
     { "auto",      "auto mode",             0,    AV_OPT_TYPE_CONST,  { .i64 = MFX_SCALING_MODE_DEFAULT},  INT_MIN, INT_MAX, FLAGS, "scale mode"},
     { "low_power", "low power mode",        0,    AV_OPT_TYPE_CONST,  { .i64 = MFX_SCALING_MODE_LOWPOWER}, INT_MIN, INT_MAX, FLAGS, "scale mode"},
@@ -401,7 +397,6 @@ static int config_output(AVFilterLink *outlink)
     param.filter_frame  = NULL;
     param.num_ext_buf   = 0;
     param.ext_buf       = ext_buf;
-    param.async_depth   = vpp->async_depth;
 
     if (get_mfx_version(ctx, &mfx_version) != MFX_ERR_NONE) {
         av_log(ctx, AV_LOG_ERROR, "Failed to query mfx version.\n");
@@ -557,8 +552,9 @@ static int config_output(AVFilterLink *outlink)
         vpp->detail || vpp->procamp || vpp->rotate || vpp->hflip ||
         inlink->w != outlink->w || inlink->h != outlink->h || in_format != vpp->out_format ||
         !vpp->has_passthrough)
-        return ff_qsvvpp_create(ctx, &vpp->qsv, &param);
+        return ff_qsvvpp_init(ctx, &param);
     else {
+        /* No MFX session is created in this case */
         av_log(ctx, AV_LOG_VERBOSE, "qsv vpp pass through mode.\n");
         if (inlink->hw_frames_ctx)
             outlink->hw_frames_ctx = av_buffer_ref(inlink->hw_frames_ctx);
@@ -571,29 +567,27 @@ static int activate(AVFilterContext *ctx)
 {
     AVFilterLink *inlink = ctx->inputs[0];
     AVFilterLink *outlink = ctx->outputs[0];
-    VPPContext *s =ctx->priv;
-    QSVVPPContext *qsv = s->qsv;
+    QSVVPPContext *qsv = ctx->priv;
     AVFrame *in = NULL;
     int ret, status = 0;
     int64_t pts = AV_NOPTS_VALUE;
 
     FF_FILTER_FORWARD_STATUS_BACK(outlink, inlink);
 
-    if (!s->eof) {
+    if (!qsv->eof) {
         ret = ff_inlink_consume_frame(inlink, &in);
         if (ret < 0)
             return ret;
 
         if (ff_inlink_acknowledge_status(inlink, &status, &pts)) {
             if (status == AVERROR_EOF) {
-                s->eof = 1;
+                qsv->eof = 1;
             }
         }
     }
 
-    if (qsv) {
-        if (in || s->eof) {
-            qsv->eof = s->eof;
+    if (qsv->session) {
+        if (in || qsv->eof) {
             ret = ff_qsvvpp_filter_frame(qsv, inlink, in);
             av_frame_free(&in);
             if (ret == AVERROR(EAGAIN))
@@ -601,7 +595,7 @@ static int activate(AVFilterContext *ctx)
             else if (ret < 0)
                 return ret;
 
-            if (s->eof)
+            if (qsv->eof)
                 goto eof;
 
             if (qsv->got_frame) {
@@ -610,6 +604,7 @@ static int activate(AVFilterContext *ctx)
             }
         }
     } else {
+        /* No MFX session is created in pass-through mode */
         if (in) {
             if (in->pts != AV_NOPTS_VALUE)
                 in->pts = av_rescale_q(in->pts, inlink->time_base, outlink->time_base);
@@ -618,7 +613,7 @@ static int activate(AVFilterContext *ctx)
             if (ret < 0)
                 return ret;
 
-            if (s->eof)
+            if (qsv->eof)
                 goto eof;
 
             return 0;
@@ -626,7 +621,7 @@ static int activate(AVFilterContext *ctx)
     }
 
 not_ready:
-    if (s->eof)
+    if (qsv->eof)
         goto eof;
 
     FF_FILTER_FORWARD_WANTED(outlink, inlink);
@@ -667,9 +662,7 @@ static int query_formats(AVFilterContext *ctx)
 
 static av_cold void vpp_uninit(AVFilterContext *ctx)
 {
-    VPPContext *vpp = ctx->priv;
-
-    ff_qsvvpp_free(&vpp->qsv);
+    ff_qsvvpp_close(ctx);
 }
 
 static const AVClass vpp_class = {
-- 
2.25.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/8] lavfi/vf_vpp_qsv: add "a", "dar" and "sar" variables

[Xiang, Haihao]

On Ma, 2023-01-09 at 15:12 +0800, Xiang, Haihao wrote:
> From: Haihao Xiang <haihao.xiang@intel.com>
> 
> Also fix the naming style in enum var_name.
> 
> 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 | 49 ++++++++++++++++++++++++----------------
>  1 file changed, 29 insertions(+), 20 deletions(-)
> 
> diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> index 317ae06c12..8d4227f841 100644
> --- a/libavfilter/vf_vpp_qsv.c
> +++ b/libavfilter/vf_vpp_qsv.c
> @@ -150,18 +150,22 @@ static const char *const var_names[] = {
>      "ch",
>      "cx",
>      "cy",
> +    "a", "dar",
> +    "sar",
>      NULL
>  };
>  
>  enum var_name {
> -    VAR_iW, VAR_IN_W,
> -    VAR_iH, VAR_IN_H,
> -    VAR_oW, VAR_OUT_W, VAR_W,
> -    VAR_oH, VAR_OUT_H, VAR_H,
> -    CW,
> -    CH,
> -    CX,
> -    CY,
> +    VAR_IW, VAR_IN_W,
> +    VAR_IH, VAR_IN_H,
> +    VAR_OW, VAR_OUT_W, VAR_W,
> +    VAR_OH, VAR_OUT_H, VAR_H,
> +    VAR_CW,
> +    VAR_CH,
> +    VAR_CX,
> +    VAR_CY,
> +    VAR_A, VAR_DAR,
> +    VAR_SAR,
>      VAR_VARS_NB
>  };
>  
> @@ -193,39 +197,44 @@ static int eval_expr(AVFilterContext *ctx)
>      PASS_EXPR(cx_expr, vpp->cx);
>      PASS_EXPR(cy_expr, vpp->cy);
>  
> -    var_values[VAR_iW] =
> +    var_values[VAR_IW] =
>      var_values[VAR_IN_W] = ctx->inputs[0]->w;
>  
> -    var_values[VAR_iH] =
> +    var_values[VAR_IH] =
>      var_values[VAR_IN_H] = ctx->inputs[0]->h;
>  
> +    var_values[VAR_A] = (double)var_values[VAR_IN_W] / var_values[VAR_IN_H];
> +    var_values[VAR_SAR] = ctx->inputs[0]->sample_aspect_ratio.num ?
> +        (double)ctx->inputs[0]->sample_aspect_ratio.num / ctx->inputs[0]-
> >sample_aspect_ratio.den : 1;
> +    var_values[VAR_DAR] = var_values[VAR_A] * var_values[VAR_SAR];
> +
>      /* crop params */
> -    CALC_EXPR(cw_expr, var_values[CW], vpp->crop_w);
> -    CALC_EXPR(ch_expr, var_values[CH], vpp->crop_h);
> +    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w);
> +    CALC_EXPR(ch_expr, var_values[VAR_CH], vpp->crop_h);
>  
>      /* calc again in case cw is relative to ch */
> -    CALC_EXPR(cw_expr, var_values[CW], vpp->crop_w);
> +    CALC_EXPR(cw_expr, var_values[VAR_CW], vpp->crop_w);
>  
>      CALC_EXPR(w_expr,
> -            var_values[VAR_OUT_W] = var_values[VAR_oW] = var_values[VAR_W],
> +            var_values[VAR_OUT_W] = var_values[VAR_OW] = var_values[VAR_W],
>              vpp->out_width);
>      CALC_EXPR(h_expr,
> -            var_values[VAR_OUT_H] = var_values[VAR_oH] = var_values[VAR_H],
> +            var_values[VAR_OUT_H] = var_values[VAR_OH] = var_values[VAR_H],
>              vpp->out_height);
>  
>      /* 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],
> +            var_values[VAR_OUT_W] = var_values[VAR_OW] = var_values[VAR_W],
>              vpp->out_width);
>  
>  
> -    CALC_EXPR(cx_expr, var_values[CX], vpp->crop_x);
> -    CALC_EXPR(cy_expr, var_values[CY], vpp->crop_y);
> +    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x);
> +    CALC_EXPR(cy_expr, var_values[VAR_CY], vpp->crop_y);
>  
>      /* calc again in case cx is relative to cy */
> -    CALC_EXPR(cx_expr, var_values[CX], vpp->crop_x);
> +    CALC_EXPR(cx_expr, var_values[VAR_CX], vpp->crop_x);
>  
> -    if ((vpp->crop_w != var_values[VAR_iW]) || (vpp->crop_h !=
> var_values[VAR_iH]))
> +    if ((vpp->crop_w != var_values[VAR_IW]) || (vpp->crop_h !=
> var_values[VAR_IH]))
>          vpp->use_crop = 1;
>  
>  release:

Will apply,

-Haihao

_______________________________________________
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".

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

[Andreas Rheinhardt]

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

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 4/8] lavfi/vf_vpp_qsv: add vpp_preinit callback

[Andreas Rheinhardt]

Xiang, Haihao:
> From: Haihao Xiang <haihao.xiang@intel.com>
> 
> Set the expected default value for options in this callback, hence we
> have the right values even if these options are not included in the
> option arrray.
> 
> 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 | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> index cf11cd7fdc..2a7b06fa33 100644
> --- a/libavfilter/vf_vpp_qsv.c
> +++ b/libavfilter/vf_vpp_qsv.c
> @@ -259,6 +259,19 @@ release:
>      return ret;
>  }
>  
> +static av_cold int vpp_preinit(AVFilterContext *ctx)
> +{
> +    VPPContext  *vpp  = ctx->priv;
> +    /* For AV_OPT_TYPE_STRING options, NULL is handled in other way so
> +     * we needn't set default value here
> +     */
> +    vpp->saturation = 1.0;
> +    vpp->contrast = 1.0;
> +    vpp->transpose = -1;
> +
> +    return 0;
> +}
> +
>  static av_cold int vpp_init(AVFilterContext *ctx)
>  {
>      VPPContext  *vpp  = ctx->priv;
> @@ -683,6 +696,7 @@ const AVFilter ff_vf_vpp_qsv = {
>      .name          = "vpp_qsv",
>      .description   = NULL_IF_CONFIG_SMALL("Quick Sync Video VPP."),
>      .priv_size     = sizeof(VPPContext),
> +    .preinit       = vpp_preinit,
>      .init          = vpp_init,
>      .uninit        = vpp_uninit,
>      FILTER_INPUTS(vpp_inputs),

I do not get the point of this at all: None of these options are of type
AV_OPT_TYPE_STRING. You are merely setting the default values that
av_opt_set_defaults() would set lateron anyway.

I also fail to see how this would facilitate reusing this code for other
QSV filters.

- 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".

Re: [FFmpeg-devel] [PATCH 5/8] lavfi/vf_vpp_qsv: add has_passthrough flag in VPPContext

[Andreas Rheinhardt]

Xiang, Haihao:
> From: Haihao Xiang <haihao.xiang@intel.com>
> 
> QSV filters may set this flag in preinit callback to turn on / off pass
> through mode
> 
> This is in preparation for reusing the code for other QSV filters. E.g.
> scale_qsv filter doesn't support pass through mode.
> 
> Signed-off-by: Haihao Xiang <haihao.xiang@intel.com>
> ---
>  libavfilter/vf_vpp_qsv.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> index 2a7b06fa33..b04307b644 100644
> --- a/libavfilter/vf_vpp_qsv.c
> +++ b/libavfilter/vf_vpp_qsv.c
> @@ -102,6 +102,7 @@ typedef struct VPPContext{
>  
>      int async_depth;
>      int eof;
> +    int has_passthrough;        /* apply pass through mode if possible */
>  } VPPContext;
>  
>  static const AVOption options[] = {
> @@ -269,6 +270,8 @@ static av_cold int vpp_preinit(AVFilterContext *ctx)
>      vpp->contrast = 1.0;
>      vpp->transpose = -1;
>  
> +    vpp->has_passthrough = 1;
> +
>      return 0;
>  }
>  
> @@ -552,7 +555,8 @@ static int config_output(AVFilterLink *outlink)
>  
>      if (vpp->use_frc || vpp->use_crop || vpp->deinterlace || vpp->denoise ||
>          vpp->detail || vpp->procamp || vpp->rotate || vpp->hflip ||
> -        inlink->w != outlink->w || inlink->h != outlink->h || in_format != vpp->out_format)
> +        inlink->w != outlink->w || inlink->h != outlink->h || in_format != vpp->out_format ||
> +        !vpp->has_passthrough)
>          return ff_qsvvpp_create(ctx, &vpp->qsv, &param);
>      else {
>          av_log(ctx, AV_LOG_VERBOSE, "qsv vpp pass through mode.\n");

I don't get how this patch either. In the current patchset,
has_passthrough is always 1 (after the preinit callback) for the vpp_qsv
filter. The code here is only used by said filter and no other filter at
all, so has_passthrough is always 1 in config_output(), i.e. this whole
change is pointless.

- 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".

Re: [FFmpeg-devel] [PATCH 4/8] lavfi/vf_vpp_qsv: add vpp_preinit callback

[Xiang, Haihao]

On Do, 2023-01-12 at 05:11 +0100, Andreas Rheinhardt wrote:
> Xiang, Haihao:
> > From: Haihao Xiang <haihao.xiang@intel.com>
> > 
> > Set the expected default value for options in this callback, hence we
> > have the right values even if these options are not included in the
> > option arrray.
> > 
> > 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 | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> > index cf11cd7fdc..2a7b06fa33 100644
> > --- a/libavfilter/vf_vpp_qsv.c
> > +++ b/libavfilter/vf_vpp_qsv.c
> > @@ -259,6 +259,19 @@ release:
> >      return ret;
> >  }
> >  
> > +static av_cold int vpp_preinit(AVFilterContext *ctx)
> > +{
> > +    VPPContext  *vpp  = ctx->priv;
> > +    /* For AV_OPT_TYPE_STRING options, NULL is handled in other way so
> > +     * we needn't set default value here
> > +     */
> > +    vpp->saturation= 1.0;
> > +    vpp->contrast = 1.0;
> > +    vpp->transpose = -1;
> > +
> > +    return 0;
> > +}
> > +
> >  static av_cold int vpp_init(AVFilterContext *ctx)
> >  {
> >      VPPContext  *vpp  = ctx->priv;
> > @@ -683,6 +696,7 @@ const AVFilter ff_vf_vpp_qsv = {
> >      .name          = "vpp_qsv",
> >      .description   = NULL_IF_CONFIG_SMALL("Quick Sync Video VPP."),
> >      .priv_size     = sizeof(VPPContext),
> > +    .preinit       = vpp_preinit,
> >      .init          = vpp_init,
> >      .uninit        = vpp_uninit,
> >      FILTER_INPUTS(vpp_inputs),
> 
> I do not get the point of this at all: None of these options are of type
> AV_OPT_TYPE_STRING. You are merely setting the default values that
> av_opt_set_defaults() would set lateron anyway.

> 
> I also fail to see how this would facilitate reusing this code for other
> QSV filters.

Actually this is a part of an old patchset, see 
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4467, I want to use use
functions and data structures defined in vf_vpp_qsv.c and qsvvpp.c to implement
scale_qsv and deinterlace_qsv filters, hence we may remove vf_scale_qsv.c and
vf_deinterlace_qsv.c 

E.g.
saturation option is used in vpp_qsv filter but not in other qsv filters, so
av_opt_set_defaults() works for saturation in vpp_qsv filter but not for other
filters, we need to set a default value to vpp->saturation for other filters,
otherwise vpp->saturation is 0 for other other filters. 
 
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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 4/8] lavfi/vf_vpp_qsv: add vpp_preinit callback

[Andreas Rheinhardt]

Xiang, Haihao:
> On Do, 2023-01-12 at 05:11 +0100, Andreas Rheinhardt wrote:
>> Xiang, Haihao:
>>> From: Haihao Xiang <haihao.xiang@intel.com>
>>>
>>> Set the expected default value for options in this callback, hence we
>>> have the right values even if these options are not included in the
>>> option arrray.
>>>
>>> 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 | 14 ++++++++++++++
>>>  1 file changed, 14 insertions(+)
>>>
>>> diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
>>> index cf11cd7fdc..2a7b06fa33 100644
>>> --- a/libavfilter/vf_vpp_qsv.c
>>> +++ b/libavfilter/vf_vpp_qsv.c
>>> @@ -259,6 +259,19 @@ release:
>>>      return ret;
>>>  }
>>>  
>>> +static av_cold int vpp_preinit(AVFilterContext *ctx)
>>> +{
>>> +    VPPContext  *vpp  = ctx->priv;
>>> +    /* For AV_OPT_TYPE_STRING options, NULL is handled in other way so
>>> +     * we needn't set default value here
>>> +     */
>>> +    vpp->saturation= 1.0;
>>> +    vpp->contrast = 1.0;
>>> +    vpp->transpose = -1;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>  static av_cold int vpp_init(AVFilterContext *ctx)
>>>  {
>>>      VPPContext  *vpp  = ctx->priv;
>>> @@ -683,6 +696,7 @@ const AVFilter ff_vf_vpp_qsv = {
>>>      .name          = "vpp_qsv",
>>>      .description   = NULL_IF_CONFIG_SMALL("Quick Sync Video VPP."),
>>>      .priv_size     = sizeof(VPPContext),
>>> +    .preinit       = vpp_preinit,
>>>      .init          = vpp_init,
>>>      .uninit        = vpp_uninit,
>>>      FILTER_INPUTS(vpp_inputs),
>>
>> I do not get the point of this at all: None of these options are of type
>> AV_OPT_TYPE_STRING. You are merely setting the default values that
>> av_opt_set_defaults() would set lateron anyway.
> 
>>
>> I also fail to see how this would facilitate reusing this code for other
>> QSV filters.
> 
> Actually this is a part of an old patchset, see 
> https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4467, I want to use use
> functions and data structures defined in vf_vpp_qsv.c and qsvvpp.c to implement
> scale_qsv and deinterlace_qsv filters, hence we may remove vf_scale_qsv.c and
> vf_deinterlace_qsv.c 
> 
> E.g.
> saturation option is used in vpp_qsv filter but not in other qsv filters, so
> av_opt_set_defaults() works for saturation in vpp_qsv filter but not for other
> filters, we need to set a default value to vpp->saturation for other filters,
> otherwise vpp->saturation is 0 for other other filters. 
>  

If you need this for other filters, then why do you add and use a
preinit callback for the filter that doesn't need it?

- 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".

Re: [FFmpeg-devel] [PATCH 5/8] lavfi/vf_vpp_qsv: add has_passthrough flag in VPPContext

[Xiang, Haihao]

On Do, 2023-01-12 at 05:31 +0100, Andreas Rheinhardt wrote:
> Xiang, Haihao:
> > From: Haihao Xiang <haihao.xiang@intel.com>
> > 
> > QSV filters may set this flag in preinit callback to turn on / off pass
> > through mode
> > 
> > This is in preparation for reusing the code for other QSV filters. E.g.
> > scale_qsv filter doesn't support pass through mode.
> > 
> > Signed-off-by: Haihao Xiang <haihao.xiang@intel.com>
> > ---
> >  libavfilter/vf_vpp_qsv.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> > index 2a7b06fa33..b04307b644 100644
> > --- a/libavfilter/vf_vpp_qsv.c
> > +++ b/libavfilter/vf_vpp_qsv.c
> > @@ -102,6 +102,7 @@ typedef struct VPPContext{
> >  
> >      int async_depth;
> >      int eof;
> > +    int has_passthrough;        /* apply pass through mode if possible */
> >  } VPPContext;
> >  
> >  static const AVOption options[] = {
> > @@ -269,6 +270,8 @@ static av_cold int vpp_preinit(AVFilterContext *ctx)
> >      vpp->contrast = 1.0;
> >      vpp->transpose = -1;
> >  
> > +    vpp->has_passthrough = 1;
> > +
> >      return 0;
> >  }
> >  
> > @@ -552,7 +555,8 @@ static int config_output(AVFilterLink *outlink)
> >  
> >      if (vpp->use_frc || vpp->use_crop || vpp->deinterlace || vpp->denoise
> > ||
> >          vpp->detail || vpp->procamp || vpp->rotate || vpp->hflip ||
> > -        inlink->w != outlink->w || inlink->h != outlink->h || in_format !=
> > vpp->out_format)
> > +        inlink->w != outlink->w || inlink->h != outlink->h || in_format !=
> > vpp->out_format ||
> > +        !vpp->has_passthrough)
> >          return ff_qsvvpp_create(ctx, &vpp->qsv, &param);
> >      else {
> >          av_log(ctx, AV_LOG_VERBOSE, "qsv vpp pass through mode.\n");
> 
> I don't get how this patch either. In the current patchset,
> has_passthrough is always 1 (after the preinit callback) for the vpp_qsv
> filter. The code here is only used by said filter and no other filter at
> all, so has_passthrough is always 1 in config_output(), i.e. this whole
> change is pointless.

I'll reuse the above code for scale_qsv filter where pass through is not
supported (a qsv session is always created in scale_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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 4/8] lavfi/vf_vpp_qsv: add vpp_preinit callback

[Xiang, Haihao]

On Do, 2023-01-12 at 05:49 +0100, Andreas Rheinhardt wrote:
> Xiang, Haihao:
> > On Do, 2023-01-12 at 05:11 +0100, Andreas Rheinhardt wrote:
> > > Xiang, Haihao:
> > > > From: Haihao Xiang <haihao.xiang@intel.com>
> > > > 
> > > > Set the expected default value for options in this callback, hence we
> > > > have the right values even if these options are not included in the
> > > > option arrray.
> > > > 
> > > > 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 | 14 ++++++++++++++
> > > >  1 file changed, 14 insertions(+)
> > > > 
> > > > diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> > > > index cf11cd7fdc..2a7b06fa33 100644
> > > > --- a/libavfilter/vf_vpp_qsv.c
> > > > +++ b/libavfilter/vf_vpp_qsv.c
> > > > @@ -259,6 +259,19 @@ release:
> > > >      return ret;
> > > >  }
> > > >  
> > > > +static av_cold int vpp_preinit(AVFilterContext *ctx)
> > > > +{
> > > > +    VPPContext  *vpp  = ctx->priv;
> > > > +    /* For AV_OPT_TYPE_STRING options, NULL is handled in other way so
> > > > +     * we needn't set default value here
> > > > +     */
> > > > +    vpp->saturation= 1.0;
> > > > +    vpp->contrast = 1.0;
> > > > +    vpp->transpose = -1;
> > > > +
> > > > +    return 0;
> > > > +}
> > > > +
> > > >  static av_cold int vpp_init(AVFilterContext *ctx)
> > > >  {
> > > >      VPPContext  *vpp  = ctx->priv;
> > > > @@ -683,6 +696,7 @@ const AVFilter ff_vf_vpp_qsv = {
> > > >      .name          = "vpp_qsv",
> > > >      .description   = NULL_IF_CONFIG_SMALL("Quick Sync Video VPP."),
> > > >      .priv_size     = sizeof(VPPContext),
> > > > +    .preinit       = vpp_preinit,
> > > >      .init          = vpp_init,
> > > >      .uninit        = vpp_uninit,
> > > >      FILTER_INPUTS(vpp_inputs),
> > > 
> > > I do not get the point of this at all: None of these options are of type
> > > AV_OPT_TYPE_STRING. You are merely setting the default values that
> > > av_opt_set_defaults() would set lateron anyway.
> > > I also fail to see how this would facilitate reusing this code for other
> > > QSV filters.
> > 
> > Actually this is a part of an old patchset, see 
> > https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4467, I want to use
> > use
> > functions and data structures defined in vf_vpp_qsv.c and qsvvpp.c to
> > implement
> > scale_qsv and deinterlace_qsv filters, hence we may remove vf_scale_qsv.c
> > and
> > vf_deinterlace_qsv.c 
> > 
> > E.g.
> > saturation option is used in vpp_qsv filter but not in other qsv filters, so
> > av_opt_set_defaults() works for saturation in vpp_qsv filter but not for
> > other
> > filters, we need to set a default value to vpp->saturation for other
> > filters,
> > otherwise vpp->saturation is 0 for other other filters. 
> >  
> 
> If you need this for other filters, then why do you add and use a
> preinit callback for the filter that doesn't need it?

I want to make sure these filters (include vpp_qsv filter) have the same default
values for options at preinit stage. 

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".
_______________________________________________
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".

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

[Xiang, Haihao]

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".
_______________________________________________
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".

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

[Xiang, Haihao]

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

_______________________________________________
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".