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

[Paul B Mahol]

On 11/3/22, Andreas Rheinhardt <andreas.rheinhardt at outlook.com> wrote:
> Paul B Mahol:
>> On 11/2/22, Andreas Rheinhardt <andreas.rheinhardt at outlook.com> wrote:
>>> 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).
>>>
>>
>> This is invalid, input pad receives always writable frames as there is
>> flag.
>>
>
> But in case this branch here is executed, the av_frame_clone() makes
> frame non-writable, so it needs to be made writable again.

Than will use copy frame.

>
>>> (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);
>>>> +}
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>