diff mbox series

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

Message ID MN2PR04MB5981DCA5EB0114147EC5D37ABAA79@MN2PR04MB5981.namprd04.prod.outlook.com
State Superseded, archived
Headers show
Series [FFmpeg-devel] avfilter/frames: Ensure frames are writable when processing in-place | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Soft Works Sept. 27, 2021, 12:07 p.m. UTC 
With the introduction of subtitle filtering, subsequent video frames
which are sharing the same data won't be as rare as this happened
to occur yet.
Ensuring per-frame uniqueness when data is modified is not only important
to avoid issues in the future, but a common API requirement anyway.

Signed-off-by: softworkz <softworkz@hotmail.com>
---
 libavfilter/vf_chromakey.c        | 5 +++++
 libavfilter/vf_codecview.c        | 6 ++++++
 libavfilter/vf_colorcontrast.c    | 5 +++++
 libavfilter/vf_colorcorrect.c     | 6 ++++++
 libavfilter/vf_colorize.c         | 6 ++++++
 libavfilter/vf_colorkey.c         | 5 +++++
 libavfilter/vf_colortemperature.c | 6 ++++++
 libavfilter/vf_cover_rect.c       | 7 +++++--
 libavfilter/vf_datascope.c        | 7 ++++++-
 libavfilter/vf_despill.c          | 5 +++++
 libavfilter/vf_exposure.c         | 6 ++++++
 libavfilter/vf_fade.c             | 6 ++++++
 libavfilter/vf_fillborders.c      | 6 ++++++
 libavfilter/vf_floodfill.c        | 5 +++++
 libavfilter/vf_hsvkey.c           | 5 +++++
 libavfilter/vf_lumakey.c          | 5 +++++
 libavfilter/vf_subtitles.c        | 7 ++++++-
 libavfilter/vf_swaprect.c         | 5 +++++
 libavfilter/vf_vflip.c            | 7 ++++++-
 libavfilter/vf_vibrance.c         | 5 +++++
 20 files changed, 110 insertions(+), 5 deletions(-)

Comments

Nicolas George Sept. 27, 2021, 12:10 p.m. UTC | #1 
Soft Works (12021-09-27):
> With the introduction of subtitle filtering, subsequent video frames
> which are sharing the same data won't be as rare as this happened
> to occur yet.
> Ensuring per-frame uniqueness when data is modified is not only important
> to avoid issues in the future, but a common API requirement anyway.
> 
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---

NAK, redundant.
Soft Works Sept. 27, 2021, 12:11 p.m. UTC | #2 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas George
> Sent: Monday, 27 September 2021 14:10
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] avfilter/frames: Ensure frames
> are writable when processing in-place
> 
> Soft Works (12021-09-27):
> > With the introduction of subtitle filtering, subsequent video
> frames
> > which are sharing the same data won't be as rare as this happened
> > to occur yet.
> > Ensuring per-frame uniqueness when data is modified is not only
> important
> > to avoid issues in the future, but a common API requirement anyway.
> >
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> > ---
> 
> NAK, redundant.

Why?
Soft Works Sept. 27, 2021, 1:23 p.m. UTC | #3 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Soft Works
> Sent: Monday, 27 September 2021 14:12
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] avfilter/frames: Ensure frames
> are writable when processing in-place
> 
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Nicolas George
> > Sent: Monday, 27 September 2021 14:10
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH] avfilter/frames: Ensure frames
> > are writable when processing in-place
> >
> > Soft Works (12021-09-27):
> > > With the introduction of subtitle filtering, subsequent video
> > frames
> > > which are sharing the same data won't be as rare as this happened
> > > to occur yet.
> > > Ensuring per-frame uniqueness when data is modified is not only
> > important
> > > to avoid issues in the future, but a common API requirement
> anyway.
> > >
> > > Signed-off-by: softworkz <softworkz@hotmail.com>
> > > ---
> >
> > NAK, redundant.
> 
> Why?

You mean because of AVFILTERPAD_FLAG_NEEDS_WRITABLE?

It should be deprecated IMO, it's not a good API design.

softworkz
diff mbox series

Patch

diff --git a/libavfilter/vf_chromakey.c b/libavfilter/vf_chromakey.c
index 304cb9ee6c..cf28cbadcb 100644
--- a/libavfilter/vf_chromakey.c
+++ b/libavfilter/vf_chromakey.c
@@ -260,6 +260,11 @@  static int filter_frame(AVFilterLink *link, AVFrame *frame)
     ChromakeyContext *ctx = avctx->priv;
     int res;
 
+    if ((res = av_frame_make_writable(frame)) < 0) {
+        av_frame_free(&frame);
+        return res;
+    }
+
     if (res = ff_filter_execute(avctx, ctx->do_slice, frame, NULL,
                                 FFMIN(frame->height, ff_filter_get_nb_threads(avctx))))
         return res;
diff --git a/libavfilter/vf_codecview.c b/libavfilter/vf_codecview.c
index dc3d3acd82..61416ae733 100644
--- a/libavfilter/vf_codecview.c
+++ b/libavfilter/vf_codecview.c
@@ -215,7 +215,13 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
     AVFilterContext *ctx = inlink->dst;
     CodecViewContext *s = ctx->priv;
     AVFilterLink *outlink = ctx->outputs[0];
+    int ret;
 
+    if ((ret = av_frame_make_writable(frame)) < 0) {
+        av_frame_free(&frame);
+        return ret;
+    }
+    
     if (s->qp) {
         int qstride, qp_type, ret;
         int8_t *qp_table;
diff --git a/libavfilter/vf_colorcontrast.c b/libavfilter/vf_colorcontrast.c
index e89b3e7af4..410a51cba7 100644
--- a/libavfilter/vf_colorcontrast.c
+++ b/libavfilter/vf_colorcontrast.c
@@ -303,6 +303,11 @@  static int filter_frame(AVFilterLink *link, AVFrame *frame)
     ColorContrastContext *s = ctx->priv;
     int res;
 
+    if ((res = av_frame_make_writable(frame)) < 0) {
+        av_frame_free(&frame);
+        return res;
+    }
+
     if (res = ff_filter_execute(ctx, s->do_slice, frame, NULL,
                                 FFMIN(frame->height, ff_filter_get_nb_threads(ctx))))
         return res;
diff --git a/libavfilter/vf_colorcorrect.c b/libavfilter/vf_colorcorrect.c
index 2570bb105c..ce18413697 100644
--- a/libavfilter/vf_colorcorrect.c
+++ b/libavfilter/vf_colorcorrect.c
@@ -398,6 +398,7 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
     AVFilterContext *ctx = inlink->dst;
     ColorCorrectContext *s = ctx->priv;
     const int nb_threads = s->analyze == MEDIAN ? 1 : FFMIN(s->planeheight[1], ff_filter_get_nb_threads(ctx));
+    int ret;
 
     if (s->analyze) {
         const int nb_athreads = s->analyze == MEDIAN ? 1 : nb_threads;
@@ -423,6 +424,11 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
         s->rh = -rh;
     }
 
+    if ((ret = av_frame_make_writable(frame)) < 0) {
+        av_frame_free(&frame);
+        return ret;
+    }
+
     ff_filter_execute(ctx, s->do_slice, frame, NULL, nb_threads);
 
     return ff_filter_frame(ctx->outputs[0], frame);
diff --git a/libavfilter/vf_colorize.c b/libavfilter/vf_colorize.c
index 4b57998a22..3f14547c00 100644
--- a/libavfilter/vf_colorize.c
+++ b/libavfilter/vf_colorize.c
@@ -199,6 +199,12 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
     AVFilterContext *ctx = inlink->dst;
     ColorizeContext *s = ctx->priv;
     float c[3];
+    int ret;
+
+    if ((ret = av_frame_make_writable(frame)) < 0) {
+        av_frame_free(&frame);
+        return ret;
+    }
 
     hsl2rgb(s->hue, s->saturation, s->lightness, &c[0], &c[1], &c[2]);
     rgb2yuv(c[0], c[1], c[2], &s->c[0], &s->c[1], &s->c[2], s->depth);
diff --git a/libavfilter/vf_colorkey.c b/libavfilter/vf_colorkey.c
index bef3e37414..45a61df104 100644
--- a/libavfilter/vf_colorkey.c
+++ b/libavfilter/vf_colorkey.c
@@ -135,6 +135,11 @@  static int filter_frame(AVFilterLink *link, AVFrame *frame)
     ColorkeyContext *ctx = avctx->priv;
     int res;
 
+    if ((res = av_frame_make_writable(frame)) < 0) {
+        av_frame_free(&frame);
+        return res;
+    }
+
     if (res = ff_filter_execute(avctx, ctx->do_slice, frame, NULL,
                                 FFMIN(frame->height, ff_filter_get_nb_threads(avctx))))
         return res;
diff --git a/libavfilter/vf_colortemperature.c b/libavfilter/vf_colortemperature.c
index da2e3bf908..4e8e18af30 100644
--- a/libavfilter/vf_colortemperature.c
+++ b/libavfilter/vf_colortemperature.c
@@ -267,9 +267,15 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
 {
     AVFilterContext *ctx = inlink->dst;
     ColorTemperatureContext *s = ctx->priv;
+    int ret;
 
     kelvin2rgb(s->temperature, s->color);
 
+    if ((ret = av_frame_make_writable(frame)) < 0) {
+        av_frame_free(&frame);
+        return ret;
+    }
+
     ff_filter_execute(ctx, s->do_slice, frame, NULL,
                       FFMIN(frame->height, ff_filter_get_nb_threads(ctx)));
 
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_datascope.c b/libavfilter/vf_datascope.c
index a60d061d18..c5b78d90cd 100644
--- a/libavfilter/vf_datascope.c
+++ b/libavfilter/vf_datascope.c
@@ -1051,7 +1051,12 @@  static int oscilloscope_filter_frame(AVFilterLink *inlink, AVFrame *frame)
     float average[4] = { 0 };
     int max[4] = { 0 };
     int min[4] = { INT_MAX, INT_MAX, INT_MAX, INT_MAX };
-    int i, c;
+    int i, c, ret;
+
+    if ((ret = av_frame_make_writable(frame)) < 0) {
+        av_frame_free(&frame);
+        return ret;
+    }
 
     s->nb_values = 0;
     draw_scope(s, s->x1, s->y1, s->x2, s->y2, frame, s->values, inlink->frame_count_in & 1);
diff --git a/libavfilter/vf_despill.c b/libavfilter/vf_despill.c
index 2c60112b48..c6c12b9b4a 100644
--- a/libavfilter/vf_despill.c
+++ b/libavfilter/vf_despill.c
@@ -94,6 +94,11 @@  static int filter_frame(AVFilterLink *link, AVFrame *frame)
     AVFilterContext *ctx = link->dst;
     int ret;
 
+    if ((ret = av_frame_make_writable(frame)) < 0) {
+        av_frame_free(&frame);
+        return ret;
+    }
+
     if (ret = ff_filter_execute(ctx, do_despill_slice, frame, NULL,
                                 FFMIN(frame->height, ff_filter_get_nb_threads(ctx))))
         return ret;
diff --git a/libavfilter/vf_exposure.c b/libavfilter/vf_exposure.c
index 2b6e6f1586..07c45acc1b 100644
--- a/libavfilter/vf_exposure.c
+++ b/libavfilter/vf_exposure.c
@@ -67,6 +67,12 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
 {
     AVFilterContext *ctx = inlink->dst;
     ExposureContext *s = ctx->priv;
+    int ret;
+
+    if ((ret = av_frame_make_writable(frame)) < 0) {
+        av_frame_free(&frame);
+        return ret;
+    }
 
     s->scale = 1.f / (exp2f(-s->exposure) - s->black);
     ff_filter_execute(ctx, s->do_slice, frame, NULL,
diff --git a/libavfilter/vf_fade.c b/libavfilter/vf_fade.c
index 71d40c4928..563c4819ab 100644
--- a/libavfilter/vf_fade.c
+++ b/libavfilter/vf_fade.c
@@ -492,6 +492,12 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
     }
 
     if (s->factor < UINT16_MAX) {
+        int ret;
+        if ((ret = av_frame_make_writable(frame)) < 0) {
+            av_frame_free(&frame);
+            return ret;
+        }
+
         if (s->alpha) {
             ff_filter_execute(ctx, s->filter_slice_alpha, frame, NULL,
                               FFMIN(frame->height, ff_filter_get_nb_threads(ctx)));
diff --git a/libavfilter/vf_fillborders.c b/libavfilter/vf_fillborders.c
index 4a799422b4..eaabf5f36f 100644
--- a/libavfilter/vf_fillborders.c
+++ b/libavfilter/vf_fillborders.c
@@ -587,8 +587,14 @@  static void margins_borders16(FillBordersContext *s, AVFrame *frame)
 
 static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
 {
+    int ret;
     FillBordersContext *s = inlink->dst->priv;
 
+    if ((ret = av_frame_make_writable(frame)) < 0) {
+        av_frame_free(&frame);
+        return ret;
+    }
+
     s->fillborders(s, frame);
 
     return ff_filter_frame(inlink->dst->outputs[0], frame);
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_hsvkey.c b/libavfilter/vf_hsvkey.c
index 2f2f037714..83e17f00c7 100644
--- a/libavfilter/vf_hsvkey.c
+++ b/libavfilter/vf_hsvkey.c
@@ -216,6 +216,11 @@  static int filter_frame(AVFilterLink *link, AVFrame *frame)
     HSVKeyContext *s = avctx->priv;
     int res;
 
+    if ((res = av_frame_make_writable(frame)) < 0) {
+        av_frame_free(&frame);
+        return res;
+    }
+
     s->hue = FFSIGN(s->hue_opt) *M_PI * fmodf(526.f - fabsf(s->hue_opt), 360.f) / 180.f;
     if (res = ff_filter_execute(avctx, s->do_slice, frame, NULL,
                                 FFMIN(frame->height, ff_filter_get_nb_threads(avctx))))
diff --git a/libavfilter/vf_lumakey.c b/libavfilter/vf_lumakey.c
index b3bfb0a278..60a75c6253 100644
--- a/libavfilter/vf_lumakey.c
+++ b/libavfilter/vf_lumakey.c
@@ -135,6 +135,11 @@  static int filter_frame(AVFilterLink *link, AVFrame *frame)
     LumakeyContext *s = ctx->priv;
     int ret;
 
+    if ((ret = av_frame_make_writable(frame)) < 0) {
+        av_frame_free(&frame);
+        return ret;
+    }
+
     if (ret = ff_filter_execute(ctx, s->do_lumakey_slice, frame, NULL,
                                 FFMIN(frame->height, ff_filter_get_nb_threads(ctx))))
         return ret;
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index 9b2e57fe0e..ac43a8f1ed 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -179,7 +179,7 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
     AVFilterContext *ctx = inlink->dst;
     AVFilterLink *outlink = ctx->outputs[0];
     AssContext *ass = ctx->priv;
-    int detect_change = 0;
+    int detect_change = 0, ret;
     double time_ms = picref->pts * av_q2d(inlink->time_base) * 1000;
     ASS_Image *image = ass_render_frame(ass->renderer, ass->track,
                                         time_ms, &detect_change);
@@ -187,6 +187,11 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
     if (detect_change)
         av_log(ctx, AV_LOG_DEBUG, "Change happened at time ms:%f\n", time_ms);
 
+    if ((ret = av_frame_make_writable(picref)) < 0) {
+        av_frame_free(&picref);
+        return ret;
+    }
+
     overlay_ass_image(ass, picref, image);
 
     return ff_filter_frame(outlink, picref);
diff --git a/libavfilter/vf_swaprect.c b/libavfilter/vf_swaprect.c
index e03603fe5a..d776aef3c0 100644
--- a/libavfilter/vf_swaprect.c
+++ b/libavfilter/vf_swaprect.c
@@ -134,6 +134,11 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
     if (ret < 0)
         return ret;
 
+    if ((ret = av_frame_make_writable(in)) < 0) {
+        av_frame_free(&in);
+        return ret;
+    }
+
     w = dw; h = dh; x1[0] = dx1; y1[0] = dy1; x2[0] = dx2; y2[0] = dy2;
 
     x1[0] = av_clip(x1[0], 0, inlink->w - 1);
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);
diff --git a/libavfilter/vf_vibrance.c b/libavfilter/vf_vibrance.c
index 7253ebd594..9abeb6e3a7 100644
--- a/libavfilter/vf_vibrance.c
+++ b/libavfilter/vf_vibrance.c
@@ -285,6 +285,11 @@  static int filter_frame(AVFilterLink *link, AVFrame *frame)
     VibranceContext *s = avctx->priv;
     int res;
 
+    if ((res = av_frame_make_writable(frame)) < 0) {
+        av_frame_free(&frame);
+        return res;
+    }
+
     if (res = ff_filter_execute(avctx, s->do_slice, frame, NULL,
                                 FFMIN(frame->height, ff_filter_get_nb_threads(avctx))))
         return res;