Re: [FFmpeg-devel] [PATCH] avfilter/scale: refactor common code for scaling height/width expressions

[Mark Thompson]

On 01/02/17 00:46, Aman Gupta wrote:
> From: Aman Gupta 
> 
> Implements support for height/width expressions in vf_scale_vaapi,
> by refactoring common code into a new libavfilter/scale.c

Nice!  Patch as a whole LGTM, some minor nits below.

> ---
>  libavfilter/Makefile |   8 +--
>  libavfilter/scale.c  | 143 
> +++
>  libavfilter/scale.h  |  31 ++
>  libavfilter/vf_scale.c   | 109 +++--
>  libavfilter/vf_scale_npp.c   |  87 +++---
>  libavfilter/vf_scale_vaapi.c |  18 +-
>  6 files changed, 208 insertions(+), 188 deletions(-)
>  create mode 100644 libavfilter/scale.c
>  create mode 100644 libavfilter/scale.h
> 
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 68a94be..3231f08 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -257,10 +257,10 @@ OBJS-$(CONFIG_REPEATFIELDS_FILTER)   += 
> vf_repeatfields.o
>  OBJS-$(CONFIG_REVERSE_FILTER)+= f_reverse.o
>  OBJS-$(CONFIG_ROTATE_FILTER) += vf_rotate.o
>  OBJS-$(CONFIG_SAB_FILTER)+= vf_sab.o
> -OBJS-$(CONFIG_SCALE_FILTER)  += vf_scale.o
> -OBJS-$(CONFIG_SCALE_NPP_FILTER)  += vf_scale_npp.o
> -OBJS-$(CONFIG_SCALE_VAAPI_FILTER)+= vf_scale_vaapi.o
> -OBJS-$(CONFIG_SCALE2REF_FILTER)  += vf_scale.o
> +OBJS-$(CONFIG_SCALE_FILTER)  += vf_scale.o scale.o
> +OBJS-$(CONFIG_SCALE_NPP_FILTER)  += vf_scale_npp.o scale.o
> +OBJS-$(CONFIG_SCALE_VAAPI_FILTER)+= vf_scale_vaapi.o scale.o
> +OBJS-$(CONFIG_SCALE2REF_FILTER)  += vf_scale.o scale.o
>  OBJS-$(CONFIG_SELECT_FILTER) += f_select.o
>  OBJS-$(CONFIG_SELECTIVECOLOR_FILTER) += vf_selectivecolor.o
>  OBJS-$(CONFIG_SENDCMD_FILTER)+= f_sendcmd.o
> diff --git a/libavfilter/scale.c b/libavfilter/scale.c
> new file mode 100644
> index 000..b0f4be2
> --- /dev/null
> +++ b/libavfilter/scale.c
> @@ -0,0 +1,143 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */
> +
> +#include "scale.h"
> +
> +static const char *const var_names[] = {
> +"PI",
> +"PHI",
> +"E",
> +"in_w",   "iw",
> +"in_h",   "ih",
> +"out_w",  "ow",
> +"out_h",  "oh",
> +"a",
> +"sar",
> +"dar",
> +"hsub",
> +"vsub",
> +"ohsub",
> +"ovsub",
> +NULL
> +};
> +
> +enum var_name {
> +VAR_PI,
> +VAR_PHI,
> +VAR_E,
> +VAR_IN_W,   VAR_IW,
> +VAR_IN_H,   VAR_IH,
> +VAR_OUT_W,  VAR_OW,
> +VAR_OUT_H,  VAR_OH,
> +VAR_A,
> +VAR_SAR,
> +VAR_DAR,
> +VAR_HSUB,
> +VAR_VSUB,
> +VAR_OHSUB,
> +VAR_OVSUB,
> +VARS_NB
> +};
> +
> +int ff_scale_eval_dimensions(void *ctx,
> +const char *w_expr, const char *h_expr,
> +AVFilterLink *inlink, AVFilterLink *outlink,
> +int *ret_w, int *ret_h)
> +{
> +const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
> +const AVPixFmtDescriptor *out_desc = 
> av_pix_fmt_desc_get(outlink->format);
> +const char *expr;
> +int w, h;
> +int factor_w, factor_h;
> +int eval_w, eval_h;
> +int ret;
> +double var_values[VARS_NB], res;
> +
> +var_values[VAR_IN_W]  = var_values[VAR_IW] = inlink->w;
> +var_values[VAR_IN_H]  = var_values[VAR_IH] = inlink->h;
> +var_values[VAR_OUT_W] = var_values[VAR_OW] = NAN;
> +var_values[VAR_OUT_H] = var_values[VAR_OH] = NAN;
> +var_values[VAR_A] = (double) inlink->w / inlink->h;
> +var_values[VAR_SAR]   = inlink->sample_aspect_ratio.num ?
> +(double) inlink->sample_aspect_ratio.num / 
> inlink->sample_aspect_ratio.den : 1;
> +var_values[VAR_DAR]   = var_values[VAR_A] * var_values[VAR_SAR];
> +var_values[VAR_HSUB]  = 1 << desc->log2_chroma_w;
> +var_values[VAR_VSUB]  = 1 << desc->log2_chroma_h;
> +var_values[VAR_OHSUB] = 1 << out_desc->log2_chroma_w;
> +var_values[VAR_OVSUB] = 1 << out_desc->log2_chroma_h;
> +
> +/* evaluate width and height */
> +av_expr_parse_and_eval(, (expr = w_expr),
> +   var_names, var_values,
> +   

Re: [FFmpeg-devel] [PATCH] avfilter/scale: refactor common code for scaling height/width expressions

[Michael Niedermayer]

On Tue, Jan 31, 2017 at 04:46:25PM -0800, Aman Gupta wrote:
> From: Aman Gupta 
> 
> Implements support for height/width expressions in vf_scale_vaapi,
> by refactoring common code into a new libavfilter/scale.c
> ---
>  libavfilter/Makefile |   8 +--
>  libavfilter/scale.c  | 143 
> +++
>  libavfilter/scale.h  |  31 ++
>  libavfilter/vf_scale.c   | 109 +++--
>  libavfilter/vf_scale_npp.c   |  87 +++---
>  libavfilter/vf_scale_vaapi.c |  18 +-
>  6 files changed, 208 insertions(+), 188 deletions(-)
>  create mode 100644 libavfilter/scale.c
>  create mode 100644 libavfilter/scale.h
> 
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 68a94be..3231f08 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -257,10 +257,10 @@ OBJS-$(CONFIG_REPEATFIELDS_FILTER)   += 
> vf_repeatfields.o
>  OBJS-$(CONFIG_REVERSE_FILTER)+= f_reverse.o
>  OBJS-$(CONFIG_ROTATE_FILTER) += vf_rotate.o
>  OBJS-$(CONFIG_SAB_FILTER)+= vf_sab.o
> -OBJS-$(CONFIG_SCALE_FILTER)  += vf_scale.o
> -OBJS-$(CONFIG_SCALE_NPP_FILTER)  += vf_scale_npp.o
> -OBJS-$(CONFIG_SCALE_VAAPI_FILTER)+= vf_scale_vaapi.o
> -OBJS-$(CONFIG_SCALE2REF_FILTER)  += vf_scale.o
> +OBJS-$(CONFIG_SCALE_FILTER)  += vf_scale.o scale.o
> +OBJS-$(CONFIG_SCALE_NPP_FILTER)  += vf_scale_npp.o scale.o
> +OBJS-$(CONFIG_SCALE_VAAPI_FILTER)+= vf_scale_vaapi.o scale.o
> +OBJS-$(CONFIG_SCALE2REF_FILTER)  += vf_scale.o scale.o
>  OBJS-$(CONFIG_SELECT_FILTER) += f_select.o
>  OBJS-$(CONFIG_SELECTIVECOLOR_FILTER) += vf_selectivecolor.o
>  OBJS-$(CONFIG_SENDCMD_FILTER)+= f_sendcmd.o
> diff --git a/libavfilter/scale.c b/libavfilter/scale.c
> new file mode 100644
> index 000..b0f4be2
> --- /dev/null
> +++ b/libavfilter/scale.c
> @@ -0,0 +1,143 @@
> +/*
> + * This file is part of FFmpeg.

Theres a copyright line in libavfilter/vf_scale.c
but none in the file the code is moved too


> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */
> +
> +#include "scale.h"
> +
> +static const char *const var_names[] = {
> +"PI",
> +"PHI",
> +"E",
> +"in_w",   "iw",
> +"in_h",   "ih",
> +"out_w",  "ow",
> +"out_h",  "oh",
> +"a",
> +"sar",
> +"dar",
> +"hsub",
> +"vsub",
> +"ohsub",
> +"ovsub",
> +NULL
> +};
> +
> +enum var_name {
> +VAR_PI,
> +VAR_PHI,
> +VAR_E,
> +VAR_IN_W,   VAR_IW,
> +VAR_IN_H,   VAR_IH,
> +VAR_OUT_W,  VAR_OW,
> +VAR_OUT_H,  VAR_OH,
> +VAR_A,
> +VAR_SAR,
> +VAR_DAR,
> +VAR_HSUB,
> +VAR_VSUB,
> +VAR_OHSUB,
> +VAR_OVSUB,
> +VARS_NB
> +};
> +

> +int ff_scale_eval_dimensions(void *ctx,

please call ctx, log_ctx


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avfilter/scale: refactor common code for scaling height/width expressions

[Aman Gupta]

From: Aman Gupta 

Implements support for height/width expressions in vf_scale_vaapi,
by refactoring common code into a new libavfilter/scale.c
---
 libavfilter/Makefile |   8 +--
 libavfilter/scale.c  | 143 +++
 libavfilter/scale.h  |  31 ++
 libavfilter/vf_scale.c   | 109 +++--
 libavfilter/vf_scale_npp.c   |  87 +++---
 libavfilter/vf_scale_vaapi.c |  18 +-
 6 files changed, 208 insertions(+), 188 deletions(-)
 create mode 100644 libavfilter/scale.c
 create mode 100644 libavfilter/scale.h

diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 68a94be..3231f08 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -257,10 +257,10 @@ OBJS-$(CONFIG_REPEATFIELDS_FILTER)   += 
vf_repeatfields.o
 OBJS-$(CONFIG_REVERSE_FILTER)+= f_reverse.o
 OBJS-$(CONFIG_ROTATE_FILTER) += vf_rotate.o
 OBJS-$(CONFIG_SAB_FILTER)+= vf_sab.o
-OBJS-$(CONFIG_SCALE_FILTER)  += vf_scale.o
-OBJS-$(CONFIG_SCALE_NPP_FILTER)  += vf_scale_npp.o
-OBJS-$(CONFIG_SCALE_VAAPI_FILTER)+= vf_scale_vaapi.o
-OBJS-$(CONFIG_SCALE2REF_FILTER)  += vf_scale.o
+OBJS-$(CONFIG_SCALE_FILTER)  += vf_scale.o scale.o
+OBJS-$(CONFIG_SCALE_NPP_FILTER)  += vf_scale_npp.o scale.o
+OBJS-$(CONFIG_SCALE_VAAPI_FILTER)+= vf_scale_vaapi.o scale.o
+OBJS-$(CONFIG_SCALE2REF_FILTER)  += vf_scale.o scale.o
 OBJS-$(CONFIG_SELECT_FILTER) += f_select.o
 OBJS-$(CONFIG_SELECTIVECOLOR_FILTER) += vf_selectivecolor.o
 OBJS-$(CONFIG_SENDCMD_FILTER)+= f_sendcmd.o
diff --git a/libavfilter/scale.c b/libavfilter/scale.c
new file mode 100644
index 000..b0f4be2
--- /dev/null
+++ b/libavfilter/scale.c
@@ -0,0 +1,143 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "scale.h"
+
+static const char *const var_names[] = {
+"PI",
+"PHI",
+"E",
+"in_w",   "iw",
+"in_h",   "ih",
+"out_w",  "ow",
+"out_h",  "oh",
+"a",
+"sar",
+"dar",
+"hsub",
+"vsub",
+"ohsub",
+"ovsub",
+NULL
+};
+
+enum var_name {
+VAR_PI,
+VAR_PHI,
+VAR_E,
+VAR_IN_W,   VAR_IW,
+VAR_IN_H,   VAR_IH,
+VAR_OUT_W,  VAR_OW,
+VAR_OUT_H,  VAR_OH,
+VAR_A,
+VAR_SAR,
+VAR_DAR,
+VAR_HSUB,
+VAR_VSUB,
+VAR_OHSUB,
+VAR_OVSUB,
+VARS_NB
+};
+
+int ff_scale_eval_dimensions(void *ctx,
+const char *w_expr, const char *h_expr,
+AVFilterLink *inlink, AVFilterLink *outlink,
+int *ret_w, int *ret_h)
+{
+const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
+const AVPixFmtDescriptor *out_desc = av_pix_fmt_desc_get(outlink->format);
+const char *expr;
+int w, h;
+int factor_w, factor_h;
+int eval_w, eval_h;
+int ret;
+double var_values[VARS_NB], res;
+
+var_values[VAR_IN_W]  = var_values[VAR_IW] = inlink->w;
+var_values[VAR_IN_H]  = var_values[VAR_IH] = inlink->h;
+var_values[VAR_OUT_W] = var_values[VAR_OW] = NAN;
+var_values[VAR_OUT_H] = var_values[VAR_OH] = NAN;
+var_values[VAR_A] = (double) inlink->w / inlink->h;
+var_values[VAR_SAR]   = inlink->sample_aspect_ratio.num ?
+(double) inlink->sample_aspect_ratio.num / 
inlink->sample_aspect_ratio.den : 1;
+var_values[VAR_DAR]   = var_values[VAR_A] * var_values[VAR_SAR];
+var_values[VAR_HSUB]  = 1 << desc->log2_chroma_w;
+var_values[VAR_VSUB]  = 1 << desc->log2_chroma_h;
+var_values[VAR_OHSUB] = 1 << out_desc->log2_chroma_w;
+var_values[VAR_OVSUB] = 1 << out_desc->log2_chroma_h;
+
+/* evaluate width and height */
+av_expr_parse_and_eval(, (expr = w_expr),
+   var_names, var_values,
+   NULL, NULL, NULL, NULL, NULL, 0, ctx);
+eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
+
+if ((ret = av_expr_parse_and_eval(, (expr = h_expr),
+  var_names, var_values,
+  NULL, NULL, NULL, NULL, NULL, 0, ctx)) < 
0)
+goto fail;
+eval_h =