On 9/28/2021 7:00 PM, Soft Works wrote:
-----Original Message-----
From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
James Almer
Sent: Tuesday, 28 September 2021 23:50
To: ffmpeg-devel@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:22 PM, Soft Works wrote:
-----Original Message-----
From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
James Almer
Sent: Tuesday, 28 September 2021 23:19
To: ffmpeg-devel@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-boun...@ffmpeg.org> On Behalf
Of
James Almer
Sent: Tuesday, 28 September 2021 22:08
To: ffmpeg-devel@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 <softwo...@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'd be because it's called automatically for filters that set
AVFILTERPAD_FLAG_NEEDS_WRITABLE. Since these don't, you should do it
manually.
That might be a task for a different day, I suppose...?
Why? You're adding av_frame_make_writable() calls. I'm asking you to
instead make them ff_inlink_make_frame_writable() calls.
25 filters are calling av_frame_make_writable() and another 4 filters
are using it implicitly via ff_framesync_dualinput_get_writable().
0 filters are using ff_inlink_make_frame_writable().
That's why I meant that changing filters from using av_frame_make_writable()
to ff_inlink_make_frame_writable() would be a separate and unrelated
change.
Doesn't that make sense?
Of course, but "It's done elsewhere" is not a good reason to refuse to
do something if can be better.
Either way, I'm not going to insist. It can be changed later.
Kind regards,
softworkz
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
