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

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> James Almer
> Sent: Tuesday, 28 September 2021 23:19
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2 1/1] avfilter/frames: Ensure
> frames are writable when processing in-place
> 
> On 9/28/2021 6:00 PM, Soft Works wrote:
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> >> James Almer
> >> Sent: Tuesday, 28 September 2021 22:08
> >> To: ffmpeg-devel at ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH v2 1/1] avfilter/frames: Ensure
> >> frames are writable when processing in-place
> >>
> >> 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
> >>
> >> Can't this flag used in these filters?
> >
> > Not in vf_vflip, because in case of flip_bayer, it's using a
> > custom allocation (.get_buffer.video)
> >
> >>>    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) {
> >>> +        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) {
> >>
> >> vf_vflip defines a custom get_buffer.video() function, so you
> can't
> >> use
> >> av_frame_make_writable() as the buffer it will return in case it
> >> needs
> >> to allocate a writable one may not be suitable. You need to use
> >> ff_inlink_make_frame_writable() instead, and in all three filters
> >> while
> >> at it.
> >
> > .get_buffer.video is used for the case of flip_bayer, but not
> otherwise.
> > Otherwise it was currently writing to the incoming frame directly,
> > and that's what this patch fixes.
> 
> Ok, but i insist you should use ff_inlink_make_frame_writable(). If
> it
> wont use the vflip custom get_buffer.video function, it will instead
> use
> the default allocator, which makes use of a lavfi frame pool.

I've gone through very single video filter's filter_frame function
and I've seen not a single one making use of it.

That might be a task for a different day, I suppose...?

softworkz