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

Guo, Yejun yejun.guo at intel.com
Wed Mar 13 09:02:03 EET 2019



> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Wednesday, March 13, 2019 8:18 AM
> To: ffmpeg-devel at 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 0000000000..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?

> +        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;

uint32_t old_self_size is needed here, similar comment for libx264 patch.

> +        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;
> +        }
> +        roi = (AVRegionOfInterest*)roi_ref->data;
> +
> +        for (i = 0; i < nb_roi - 1; i++) {
> +            old_roi = (const AVRegionOfInterest*)
> +                (sd->data + old_roi->self_size * i);
> +
> +            roi[i] = (AVRegionOfInterest) {
> +                .self_size = sizeof(*roi),
> +                .x         = old_roi->x,
> +                .y         = old_roi->y,
> +                .width     = old_roi->width,
> +                .height    = old_roi->height,
> +                .qoffset   = old_roi->qoffset,
> +            };
> +        }
> +
> +        roi[nb_roi - 1] = (AVRegionOfInterest) {
> +            .self_size = sizeof(*roi),
> +            .x         = ctx->region[X],
> +            .y         = ctx->region[Y],
> +            .width     = ctx->region[W],
> +            .height    = ctx->region[H],
> +            .qoffset   = ctx->qoffset,
> +        };
> +
> +        av_frame_remove_side_data(frame,
> AV_FRAME_DATA_REGIONS_OF_INTEREST);
> +
> +        sd = av_frame_new_side_data_from_buf(frame,
> +                                             AV_FRAME_DATA_REGIONS_OF_INTEREST,
> +                                             roi_ref);
> +        if (!sd) {
> +            av_buffer_unref(&roi_ref);
> +            err = AVERROR(ENOMEM);
> +            goto fail;
> +        }
> +
> +    } else {
> +        sd = av_frame_new_side_data(frame,
> AV_FRAME_DATA_REGIONS_OF_INTEREST,
> +                                    sizeof(AVRegionOfInterest));
> +        if (!sd) {
> +            err = AVERROR(ENOMEM);
> +            goto fail;
> +        }
> +        roi = (AVRegionOfInterest*)sd->data;
> +        *roi = (AVRegionOfInterest) {
> +            .self_size = sizeof(*roi),
> +            .x         = ctx->region[X],
> +            .y         = ctx->region[Y],
> +            .width     = ctx->region[W],
> +            .height    = ctx->region[H],
> +            .qoffset   = ctx->qoffset,
> +        };
> +    }
> +
> +    return ff_filter_frame(outlink, frame);
> +
> +fail:
> +    av_frame_free(&frame);
> +    return err;
> +}
> +
> +static av_cold int addroi_init(AVFilterContext *avctx)
> +{
> +    AddROIContext *ctx = avctx->priv;
> +    int i, err;
> +
> +    for (i = 0; i < 4; i++) {
> +        err = av_expr_parse(&ctx->region_expr[i], ctx->region_str[i],
> +                            addroi_var_names, NULL, NULL, NULL, NULL,

not quite understand why addroi_var_names here, it still woks if replaced with NULL.
From eval.h, it is used for 'constant identifiers, for example {"PI", "E", 0}'.

> +                            0, avctx);
> +        if (err < 0) {
> +            av_log(ctx, AV_LOG_ERROR,
> +                   "Error parsing %s expression '%s'.\n",
> +                   addroi_param_names[i], ctx->region_str[i]);
> +            return err;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +static av_cold void addroi_uninit(AVFilterContext *avctx)
> +{
> +    AddROIContext *ctx = avctx->priv;
> +    int i;
> +
> +    for (i = 0; i < 4; i++) {
> +        av_expr_free(ctx->region_expr[i]);
> +        ctx->region_expr[i] = NULL;
> +    }
> +}
> +
> +#define OFFSET(x) offsetof(AddROIContext, x)
> +#define FLAGS AV_OPT_FLAG_VIDEO_PARAM |
> AV_OPT_FLAG_FILTERING_PARAM
> +static const AVOption addroi_options[] = {
> +    { "x", "Region distance from top edge of frame.",

from left edge of frame?

> +      OFFSET(region_str[X]), AV_OPT_TYPE_STRING, { .str = "0" }, .flags =
> FLAGS },
> +    { "y", "Region distance from left edge of frame.",

top edge?

> +      OFFSET(region_str[Y]), AV_OPT_TYPE_STRING, { .str = "0" }, .flags =
> FLAGS },
> +    { "w", "Region width.",
> +      OFFSET(region_str[W]), AV_OPT_TYPE_STRING, { .str = "0" }, .flags =
> FLAGS },
> +    { "h", "Region height.",
> +      OFFSET(region_str[H]), AV_OPT_TYPE_STRING, { .str = "0" }, .flags =
> FLAGS },
> +
> +    { "qoffset", "Quantisation offset to apply in the region.",
> +      OFFSET(qoffset), AV_OPT_TYPE_RATIONAL, { .dbl = 0.0 }, -1, +1, FLAGS },

this filter is most likely for experimental try, maybe we can init qoffst with -0.3 or other values,
so people don't need to set it explicitly at command line during experiment.

> +
> +    { "clear", "Remove any existing regions of interest before adding the new
> one.",
> +      OFFSET(clear), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS },
> +
> +    { NULL }
> +};
> +
> +AVFILTER_DEFINE_CLASS(addroi);
> +
> +static const AVFilterPad addroi_inputs[] = {
> +    {
> +        .name         = "default",
> +        .type         = AVMEDIA_TYPE_VIDEO,
> +        .config_props = addroi_config_input,
> +        .filter_frame = addroi_filter_frame,
> +    },
> +    { NULL }
> +};
> +
> +static const AVFilterPad addroi_outputs[] = {
> +    {
> +        .name = "default",
> +        .type = AVMEDIA_TYPE_VIDEO,
> +    },
> +    { NULL }
> +};
> +
> +AVFilter ff_vf_addroi = {
> +    .name        = "addroi",
> +    .description = NULL_IF_CONFIG_SMALL("Add region of interest to
> frame."),
> +    .init        = addroi_init,
> +    .uninit      = addroi_uninit,
> +
> +    .priv_size   = sizeof(AddROIContext),
> +    .priv_class  = &addroi_class,
> +
> +    .inputs      = addroi_inputs,
> +    .outputs     = addroi_outputs,
> +};
> --
> 2.19.2
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


More information about the ffmpeg-devel mailing list