[FFmpeg-devel] [PATCH] avfilter: add backgroundkey video filter

[Andreas Rheinhardt]

Paul B Mahol:
> +static int filter_frame(AVFilterLink *link, AVFrame *frame)
> +{
> +    AVFilterContext *avctx = link->dst;
> +    BackgroundkeyContext *s = avctx->priv;
> +    int64_t sum = 0;
> +    int ret;
> +
> +    if (!s->background) {
> +        s->background = av_frame_clone(frame);
> +        if (!s->background)
> +            return AVERROR(ENOMEM);
> +        av_frame_make_writable(s->background);

You are never writing to the background frame, so there is no point in
making it writable; what you actually want to achieve here is making
frame writable again and to achieve this you should make frame, not
background writable (and of course you should check said call).

(Actually, you never

> +    }
> +
> +    if (ret = ff_filter_execute(avctx, s->do_slice, frame, NULL,
> +                                FFMIN(frame->height, s->nb_threads)))
> +        return ret;
> +
> +    for (int n = 0; n < s->nb_threads; n++)
> +        sum += s->sums[n];
> +    if (s->max_sum * s->threshold < sum) {
> +        av_frame_free(&s->background);
> +        s->background = av_frame_clone(frame);
> +        if (!s->background)
> +            return AVERROR(ENOMEM);
> +        av_frame_make_writable(s->background);

Given that you never write to background, there is no need to make it
writable. This time, there is also no need to make frame writable (the
next filter in the chain may not need a writable frame anyway), so just
remove this.
And the av_frame_free+av_frame_clone can become an
av_frame_unref+av_frame_ref (this will necessitate modifying the check
above to not only check for to existence of s->background).

> +    }
> +
> +    return ff_filter_frame(avctx->outputs[0], frame);
> +}