[FFmpeg-devel] [PATCH v2 1/1] avfilter/frames: Ensure frames are writable when processing in-place

[James Almer]

On 9/28/2021 4:54 PM, Soft Works wrote:
> Signed-off-by: softworkz <softworkz at hotmail.com>
> ---
> v2: Reduced to cases without AVFILTERPAD_FLAG_NEEDS_WRITABLE
> 
>   libavfilter/vf_cover_rect.c | 7 +++++--
>   libavfilter/vf_floodfill.c  | 5 +++++
>   libavfilter/vf_vflip.c      | 7 ++++++-
>   3 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/libavfilter/vf_cover_rect.c b/libavfilter/vf_cover_rect.c
> index 0a8c10e06d..2367afb4b3 100644
> --- a/libavfilter/vf_cover_rect.c
> +++ b/libavfilter/vf_cover_rect.c
> @@ -136,7 +136,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>       AVFilterContext *ctx = inlink->dst;
>       CoverContext *cover = ctx->priv;
>       AVDictionaryEntry *ex, *ey, *ew, *eh;
> -    int x = -1, y = -1, w = -1, h = -1;
> +    int x = -1, y = -1, w = -1, h = -1, ret;
>       char *xendptr = NULL, *yendptr = NULL, *wendptr = NULL, *hendptr = NULL;
>   
>       ex = av_dict_get(in->metadata, "lavfi.rect.x", NULL, AV_DICT_MATCH_CASE);
> @@ -181,7 +181,10 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>       x = av_clip(x, 0, in->width  - w);
>       y = av_clip(y, 0, in->height - h);
>   
> -    av_frame_make_writable(in);
> +    if ((ret = av_frame_make_writable(in)) < 0) {
> +        av_frame_free(&in);
> +        return ret;
> +    }
>   
>       if (cover->mode == MODE_BLUR) {
>           blur (cover, in, x, y);
> diff --git a/libavfilter/vf_floodfill.c b/libavfilter/vf_floodfill.c
> index 21741cdb4f..292b27505e 100644
> --- a/libavfilter/vf_floodfill.c
> +++ b/libavfilter/vf_floodfill.c
> @@ -294,6 +294,11 @@ static int filter_frame(AVFilterLink *link, AVFrame *frame)
>       const int h = frame->height;
>       int i, ret;
>   
> +    if ((ret = av_frame_make_writable(frame)) < 0) {

Is this one even needed? s->pick_pixel() does not change the frame, and 
an av_frame_make_writable() call is already in place before any 
s->set_pixel() call in this same function.

The ff_filter_frame() at the end also does not require a writable frame. 
Unless I'm missing something, it just adds it into a frame queue.

> +        av_frame_free(&frame);
> +        return ret;
> +    }
> +
>       if (is_inside(s->x, s->y, w, h)) {
>           s->pick_pixel(frame, s->x, s->y, &s0, &s1, &s2, &s3);
>   
> diff --git a/libavfilter/vf_vflip.c b/libavfilter/vf_vflip.c
> index 0d624512f9..622bd46db3 100644
> --- a/libavfilter/vf_vflip.c
> +++ b/libavfilter/vf_vflip.c
> @@ -108,11 +108,16 @@ static int flip_bayer(AVFilterLink *link, AVFrame *in)
>   static int filter_frame(AVFilterLink *link, AVFrame *frame)
>   {
>       FlipContext *flip = link->dst->priv;
> -    int i;
> +    int i, ret;
>   
>       if (flip->bayer)
>           return flip_bayer(link, frame);
>   
> +    if ((ret = av_frame_make_writable(frame)) < 0) {
> +        av_frame_free(&frame);
> +        return ret;
> +    }
> +
>       for (i = 0; i < 4; i ++) {
>           int vsub = i == 1 || i == 2 ? flip->vsub : 0;
>           int height = AV_CEIL_RSHIFT(link->h, vsub);
>