[FFmpeg-devel] [PATCH] avfilter/vf_cropdetect: add ability to change limit/reset at runtime

[James Almer]

On 12/27/2022 8:46 AM, Jeffrey CHAPUIS wrote:
> Hello, first attempt to contribute.
> 
> Related to https://trac.ffmpeg.org/ticket/9851.
> 
> Tested with ffmpeg and mpv, amazing results.
> 
> Signed-off-by: Ashyni <jeffrey.c at tuta.io>
> ---
>   doc/filters.texi                           | 13 +++++
>   libavfilter/vf_cropdetect.c                | 42 +++++++++++++--
>   tests/ref/fate/filter-metadata-cropdetect  | 60 +++++++++++-----------
>   tests/ref/fate/filter-metadata-cropdetect1 | 14 ++---
>   tests/ref/fate/filter-metadata-cropdetect2 | 14 ++---
>   5 files changed, 94 insertions(+), 49 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index ceab0ea0f..bbb778368 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -10552,6 +10552,19 @@ ffmpeg -flags2 +export_mvs -i file.mp4 -vf 
> cropdetect=mode=mvedges,metadata=mode
>   @end example
>   @end itemize
>   + at subsection Commands
> +
> +This filter supports the following commands:
> + at table @option
> + at item limit
> + at item reset, reset_count
> +
> +The command accepts the same syntax of the corresponding option.
> +
> +If the specified expression is not valid, it is kept at its current
> +value.
> + at end table
> +
>   @anchor{cue}
>   @section cue
>   diff --git a/libavfilter/vf_cropdetect.c b/libavfilter/vf_cropdetect.c
> index 7e985fb27..fda803651 100644
> --- a/libavfilter/vf_cropdetect.c
> +++ b/libavfilter/vf_cropdetect.c
> @@ -422,26 +422,57 @@ static int filter_frame(AVFilterLink *inlink, 
> AVFrame *frame)
>           SET_META("lavfi.cropdetect.h",  h);
>           SET_META("lavfi.cropdetect.x",  x);
>           SET_META("lavfi.cropdetect.y",  y);
> +        SET_META("lavfi.cropdetect.pts", frame->pts);
> +        SET_META("lavfi.cropdetect.limit", limit);
> +        SET_META("lavfi.cropdetect.reset", s->reset_count);
>            av_log(ctx, AV_LOG_INFO,
> -               "x1:%d x2:%d y1:%d y2:%d w:%d h:%d x:%d y:%d 
> pts:%"PRId64" t:%f crop=%d:%d:%d:%d\n",
> +               "x1:%d x2:%d y1:%d y2:%d w:%d h:%d x:%d y:%d 
> pts:%"PRId64" t:%f limit:%d crop=%d:%d:%d:%d\n",
>                  s->x1, s->x2, s->y1, s->y2, w, h, x, y, frame->pts,
>                  frame->pts == AV_NOPTS_VALUE ? -1 : frame->pts * 
> av_q2d(inlink->time_base),
> -               w, h, x, y);
> +               limit, w, h, x, y);
>       }
>        return ff_filter_frame(inlink->dst->outputs[0], frame);
>   }
>   +static int process_command(AVFilterContext *ctx, const char *cmd, 
> const char *args,
> +                           char *res, int res_len, int flags)
> +{
> +    CropDetectContext *s = ctx->priv;
> +    int ret;
> +
> +    if (!strcmp(cmd, "limit") || !strcmp(cmd, "reset") || !strcmp(cmd, 
> "reset_count")) {

You could just call ff_filter_process_command() instead of hardcoding 
supported commands here. It will ignore any option without the 
AV_OPT_FLAG_RUNTIME_PARAM flag.

> +
> +        int old_limit = s->limit;
> +        int old_reset_count = s->reset_count;
> +
> +        AVFilterLink *inlink = ctx->inputs[0];
> +
> +        av_opt_set(s, cmd, args, 0);
> +
> +        if ((ret = config_input(inlink)) < 0) {

This is going to generate memleaks, and needlessly reallocate unrelated 
buffers.
You should instead av_realloc all four s->bboxes buffers here, and reset 
s->limit.

> +            s->limit = old_limit;
> +            s->reset_count = old_reset_count;
> +            return ret;
> +        }
> +    }
> +    else
> +        ret = AVERROR(ENOSYS);
> +
> +    return ret;
> +}
> +
>   #define OFFSET(x) offsetof(CropDetectContext, x)
>   #define FLAGS AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
> +#define TFLAGS AV_OPT_FLAG_FILTERING_PARAM | AV_OPT_FLAG_VIDEO_PARAM | 
> AV_OPT_FLAG_RUNTIME_PARAM
>    static const AVOption cropdetect_options[] = {
> -    { "limit", "Threshold below which the pixel is considered black", 
> OFFSET(limit),       AV_OPT_TYPE_FLOAT, { .dbl = 24.0/255 }, 0, 65535, 
> FLAGS },
> +    { "limit", "Threshold below which the pixel is considered black", 
> OFFSET(limit),       AV_OPT_TYPE_FLOAT, { .dbl = 24.0/255 }, 0, 65535, 
> TFLAGS },
>       { "round", "Value by which the width/height should be divisible", 
> OFFSET(round),       AV_OPT_TYPE_INT, { .i64 = 16 }, 0, INT_MAX, FLAGS },
> -    { "reset", "Recalculate the crop area after this many frames", 
> OFFSET(reset_count), AV_OPT_TYPE_INT, { .i64 = 0 },  0, INT_MAX, FLAGS },
> +    { "reset", "Recalculate the crop area after this many frames", 
> OFFSET(reset_count), AV_OPT_TYPE_INT, { .i64 = 0 },  0, INT_MAX, TFLAGS },
>       { "skip",  "Number of initial frames to skip", 
> OFFSET(skip),        AV_OPT_TYPE_INT, { .i64 = 2 },  0, INT_MAX, FLAGS },
> -    { "reset_count", "Recalculate the crop area after this many 
> frames",OFFSET(reset_count),AV_OPT_TYPE_INT,{ .i64 = 0 },  0, INT_MAX, 
> FLAGS },
> +    { "reset_count", "Recalculate the crop area after this many 
> frames",OFFSET(reset_count),AV_OPT_TYPE_INT,{ .i64 = 0 },  0, INT_MAX, 
> TFLAGS },
>       { "max_outliers", "Threshold count of outliers", 
> OFFSET(max_outliers),AV_OPT_TYPE_INT, { .i64 = 0 },  0, INT_MAX, FLAGS },
>       { "mode", "set mode", OFFSET(mode), AV_OPT_TYPE_INT, 
> {.i64=MODE_BLACK}, 0, MODE_NB-1, FLAGS, "mode" },
>           { "black",    "detect black pixels surrounding the video", 0, 
> AV_OPT_TYPE_CONST, {.i64=MODE_BLACK},    INT_MIN, INT_MAX, FLAGS, "mode" },
> @@ -481,4 +512,5 @@ const AVFilter ff_vf_cropdetect = {
>       FILTER_OUTPUTS(avfilter_vf_cropdetect_outputs),
>       FILTER_PIXFMTS_ARRAY(pix_fmts),
>       .flags         = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC | 
> AVFILTER_FLAG_METADATA_ONLY,
> +    .process_command = process_command,
>   };