Re: [FFmpeg-devel] [PATCH v2 6/6] lavfi: addroi filter

2019-03-13 Thread Guo, Yejun


> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Wednesday, March 13, 2019 8:18 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v2 6/6] lavfi: addroi filter
> 
> This can be used to add region of interest side data to video frames.
> ---
> Now using the x,y,w,h style to match other filters.
> 
> 
>  libavfilter/Makefile |   1 +
>  libavfilter/allfilters.c |   1 +
>  libavfilter/vf_addroi.c  | 265

filter documentation is missed.

> +++
>  3 files changed, 267 insertions(+)
>  create mode 100644 libavfilter/vf_addroi.c
> 
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index fef6ec5c55..31ae738a50 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -149,6 +149,7 @@ OBJS-$(CONFIG_SINE_FILTER)   += 
> asrc_sine.o
>  OBJS-$(CONFIG_ANULLSINK_FILTER)  += asink_anullsink.o
> 
>  # video filters
> +OBJS-$(CONFIG_ADDROI_FILTER) += vf_addroi.o
>  OBJS-$(CONFIG_ALPHAEXTRACT_FILTER)   += vf_extractplanes.o
>  OBJS-$(CONFIG_ALPHAMERGE_FILTER) += vf_alphamerge.o
>  OBJS-$(CONFIG_AMPLIFY_FILTER)+= vf_amplify.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index c51ae0f3c7..52413c8f0d 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -140,6 +140,7 @@ extern AVFilter ff_asrc_sine;
> 
>  extern AVFilter ff_asink_anullsink;
> 
> +extern AVFilter ff_vf_addroi;
>  extern AVFilter ff_vf_alphaextract;
>  extern AVFilter ff_vf_alphamerge;
>  extern AVFilter ff_vf_amplify;
> diff --git a/libavfilter/vf_addroi.c b/libavfilter/vf_addroi.c
> new file mode 100644
> index 00..8bca5e7371
> --- /dev/null
> +++ b/libavfilter/vf_addroi.c
> @@ -0,0 +1,265 @@
> +/*
> + * 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 "libavutil/eval.h"
> +#include "libavutil/opt.h"
> +#include "avfilter.h"
> +#include "internal.h"
> +
> +enum {
> +X, Y, W, H,
> +};
> +static const char *addroi_param_names[] = {
> +"x", "y", "w", "h",
> +};
> +
> +enum {
> +VAR_IW,
> +VAR_IH,
> +NB_VARS,
> +};
> +static const char *const addroi_var_names[] = {
> +"iw",
> +"ih",
> +};
> +
> +typedef struct AddROIContext {
> +const AVClass *class;
> +
> +char *region_str[4];
> +
> +AVExpr *region_expr[4];
> +double vars[NB_VARS];
> +
> +int region[4];
> +AVRational qoffset;
> +
> +int clear;
> +} AddROIContext;

some people don't like the abbreviation of "ROI" in struct type name, see 
comment for my patch.
shall we rename it to AddRegionOfInterestContext?

> +
> +static int addroi_config_input(AVFilterLink *inlink)
> +{
> +AVFilterContext *avctx = inlink->dst;
> +AddROIContext *ctx = avctx->priv;
> +int i;
> +double val;
> +
> +ctx->vars[VAR_IW] = inlink->w;
> +ctx->vars[VAR_IH] = inlink->h;
> +
> +for (i = 0; i < 4; i++) {
> +int max_value;
> +switch (i) {
> +case X: max_value = inlink->w;  break;
> +case Y: max_value = inlink->h;  break;
> +case W: max_value = inlink->w - ctx->region[X]; break;
> +case H: max_value = inlink->h - ctx->region[Y]; break;
> +}
> +
> +val = av_expr_eval(ctx->region_expr[i], ctx->vars, NULL);

I understand that av_expr_* function is used to calculate expressions such as 
"8*9", but
not quite understand why ctx->vars here, also the third parameter of 
av_expr_parse below.

If needed, we can use a local variable vars within this function, instead of 
within ctx?


[FFmpeg-devel] [PATCH v2 6/6] lavfi: addroi filter

2019-03-12 Thread Mark Thompson
This can be used to add region of interest side data to video frames.
---
Now using the x,y,w,h style to match other filters.


 libavfilter/Makefile |   1 +
 libavfilter/allfilters.c |   1 +
 libavfilter/vf_addroi.c  | 265 +++
 3 files changed, 267 insertions(+)
 create mode 100644 libavfilter/vf_addroi.c

diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index fef6ec5c55..31ae738a50 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -149,6 +149,7 @@ OBJS-$(CONFIG_SINE_FILTER)   += asrc_sine.o
 OBJS-$(CONFIG_ANULLSINK_FILTER)  += asink_anullsink.o
 
 # video filters
+OBJS-$(CONFIG_ADDROI_FILTER) += vf_addroi.o
 OBJS-$(CONFIG_ALPHAEXTRACT_FILTER)   += vf_extractplanes.o
 OBJS-$(CONFIG_ALPHAMERGE_FILTER) += vf_alphamerge.o
 OBJS-$(CONFIG_AMPLIFY_FILTER)+= vf_amplify.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index c51ae0f3c7..52413c8f0d 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -140,6 +140,7 @@ extern AVFilter ff_asrc_sine;
 
 extern AVFilter ff_asink_anullsink;
 
+extern AVFilter ff_vf_addroi;
 extern AVFilter ff_vf_alphaextract;
 extern AVFilter ff_vf_alphamerge;
 extern AVFilter ff_vf_amplify;
diff --git a/libavfilter/vf_addroi.c b/libavfilter/vf_addroi.c
new file mode 100644
index 00..8bca5e7371
--- /dev/null
+++ b/libavfilter/vf_addroi.c
@@ -0,0 +1,265 @@
+/*
+ * 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 "libavutil/eval.h"
+#include "libavutil/opt.h"
+#include "avfilter.h"
+#include "internal.h"
+
+enum {
+X, Y, W, H,
+};
+static const char *addroi_param_names[] = {
+"x", "y", "w", "h",
+};
+
+enum {
+VAR_IW,
+VAR_IH,
+NB_VARS,
+};
+static const char *const addroi_var_names[] = {
+"iw",
+"ih",
+};
+
+typedef struct AddROIContext {
+const AVClass *class;
+
+char *region_str[4];
+
+AVExpr *region_expr[4];
+double vars[NB_VARS];
+
+int region[4];
+AVRational qoffset;
+
+int clear;
+} AddROIContext;
+
+static int addroi_config_input(AVFilterLink *inlink)
+{
+AVFilterContext *avctx = inlink->dst;
+AddROIContext *ctx = avctx->priv;
+int i;
+double val;
+
+ctx->vars[VAR_IW] = inlink->w;
+ctx->vars[VAR_IH] = inlink->h;
+
+for (i = 0; i < 4; i++) {
+int max_value;
+switch (i) {
+case X: max_value = inlink->w;  break;
+case Y: max_value = inlink->h;  break;
+case W: max_value = inlink->w - ctx->region[X]; break;
+case H: max_value = inlink->h - ctx->region[Y]; break;
+}
+
+val = av_expr_eval(ctx->region_expr[i], ctx->vars, NULL);
+if (val < 0.0) {
+av_log(avctx, AV_LOG_WARNING, "Calculated value %g for %s is "
+   "less than zero - using zero instead.\n", val,
+   addroi_param_names[i]);
+val = 0.0;
+} else if (val > max_value) {
+av_log(avctx, AV_LOG_WARNING, "Calculated value %g for %s is "
+   "greater than maximum allowed value %d - "
+   "using %d instead.\n", val, addroi_param_names[i],
+   max_value, max_value);
+val = max_value;
+}
+ctx->region[i] = val;
+}
+
+return 0;
+}
+
+static int addroi_filter_frame(AVFilterLink *inlink, AVFrame *frame)
+{
+AVFilterContext *avctx = inlink->dst;
+AVFilterLink  *outlink = avctx->outputs[0];
+AddROIContext *ctx = avctx->priv;
+AVRegionOfInterest *roi;
+AVFrameSideData *sd;
+int err;
+
+if (ctx->clear) {
+av_frame_remove_side_data(frame, AV_FRAME_DATA_REGIONS_OF_INTEREST);
+sd = NULL;
+} else {
+sd = av_frame_get_side_data(frame, AV_FRAME_DATA_REGIONS_OF_INTEREST);
+}
+if (sd) {
+const AVRegionOfInterest *old_roi;
+AVBufferRef *roi_ref;
+int nb_roi, i;
+
+old_roi = (const AVRegionOfInterest*)sd->data;
+nb_roi = sd->size / old_roi->self_size + 1;
+
+roi_ref = av_buffer_alloc(sizeof(*roi) * nb_roi);
+if (!roi_ref) {
+err = AVERROR(ENOMEM);
+goto fail;
+