Patches attached
From fb71c0d60ff589969338aff9fce9370cee13a32f Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
Date: Thu, 9 Jan 2025 16:50:39 +0100
Subject: [PATCH 1/2] avfilter/vf_xpsnr: Fix leaks
This filter uses the AVBuffer API to allocate buffers that are never
shared at all and frees them via av_freep() (actually, it passes
pointers to AVBufferRefs to av_freep, so that only the AVBuffer
structures are freed at all (because AVBufferRef has a AVBuffer* as its
first member), not the AVBufferRef and not the underlying buffers;
and due to a wrong check the AVBuffers corresponding
to buf_org[c] with c>0 were never freed at all). This is a violation
of the AVBuffer API and causes a memleak. Fix this by avoiding the
AVBuffer API altogether.
(The FATE tests don't catch this, because they use piping to awk,
so that the error code from ffmpeg is ignored.)
Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
---
libavfilter/vf_xpsnr.c | 42 +++++++++++++++++-------------------------
1 file changed, 17 insertions(+), 25 deletions(-)
diff --git a/libavfilter/vf_xpsnr.c b/libavfilter/vf_xpsnr.c
index f8557dc8f2..f7348fdd6d 100644
--- a/libavfilter/vf_xpsnr.c
+++ b/libavfilter/vf_xpsnr.c
@@ -60,10 +60,10 @@ typedef struct XPSNRContext {
/* XPSNR specific variables */
double *sse_luma;
double *weights;
- AVBufferRef *buf_org [3];
- AVBufferRef *buf_org_m1[3];
- AVBufferRef *buf_org_m2[3];
- AVBufferRef *buf_rec [3];
+ int16_t *buf_org [3];
+ int16_t *buf_org_m1[3];
+ int16_t *buf_org_m2[3];
+ int16_t *buf_rec [3];
uint64_t max_error_64;
double sum_wdist [3];
double sum_xpsnr [3];
@@ -432,12 +432,12 @@ static int do_xpsnr(FFFrameSync *fs)
const int stride_org_bpp = (s->bpp == 1 ? s->plane_width[c] : s->line_sizes[c] / s->bpp);
if (!s->buf_org_m1[c])
- s->buf_org_m1[c] = av_buffer_allocz(stride_org_bpp * s->plane_height[c] * sizeof(int16_t));
+ s->buf_org_m1[c] = av_calloc(s->plane_height[c], stride_org_bpp * sizeof(int16_t));
if (!s->buf_org_m2[c])
- s->buf_org_m2[c] = av_buffer_allocz(stride_org_bpp * s->plane_height[c] * sizeof(int16_t));
+ s->buf_org_m2[c] = av_calloc(s->plane_height[c], stride_org_bpp * sizeof(int16_t));
- porg_m1[c] = (int16_t *) s->buf_org_m1[c]->data;
- porg_m2[c] = (int16_t *) s->buf_org_m2[c]->data;
+ porg_m1[c] = s->buf_org_m1[c];
+ porg_m2[c] = s->buf_org_m2[c];
}
}
@@ -448,12 +448,12 @@ static int do_xpsnr(FFFrameSync *fs)
const int o = s->plane_width[c]; /* XPSNR stride */
if (!s->buf_org[c])
- s->buf_org[c] = av_buffer_allocz(s->plane_width[c] * s->plane_height[c] * sizeof(int16_t));
+ s->buf_org[c] = av_calloc(s->plane_width[c], s->plane_height[c] * sizeof(int16_t));
if (!s->buf_rec[c])
- s->buf_rec[c] = av_buffer_allocz(s->plane_width[c] * s->plane_height[c] * sizeof(int16_t));
+ s->buf_rec[c] = av_calloc(s->plane_width[c], s->plane_height[c] * sizeof(int16_t));
- porg[c] = (int16_t *) s->buf_org[c]->data;
- prec[c] = (int16_t *) s->buf_rec[c]->data;
+ porg[c] = s->buf_org[c];
+ prec[c] = s->buf_rec[c];
for (int y = 0; y < s->plane_height[c]; y++) {
for (int x = 0; x < s->plane_width[c]; x++) {
@@ -692,19 +692,11 @@ static av_cold void uninit(AVFilterContext *ctx)
av_freep(&s->sse_luma);
av_freep(&s->weights );
- for (c = 0; c < s->num_comps; c++) { /* free extra temporal org buf memory */
- if(s->buf_org_m1[c])
- av_freep(s->buf_org_m1[c]);
- if(s->buf_org_m2[c])
- av_freep(s->buf_org_m2[c]);
- }
- if (s->bpp == 1) { /* 8 bit */
- for (c = 0; c < s->num_comps; c++) { /* and org/rec picture buf memory */
- if(s->buf_org_m2[c])
- av_freep(s->buf_org[c]);
- if(s->buf_rec[c])
- av_freep(s->buf_rec[c]);
- }
+ for (c = 0; c < s->num_comps; c++) {
+ av_freep(&s->buf_org_m1[c]);
+ av_freep(&s->buf_org_m2[c]);
+ av_freep(&s->buf_org[c]);
+ av_freep(&s->buf_rec[c]);
}
}
--
2.45.2
From 73e6ba0cbd91f8c1c718d9674784904eac3d28eb Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
Date: Thu, 9 Jan 2025 17:42:47 +0100
Subject: [PATCH 2/2] avfilter/vf_xpsnr: Avoid array only one of whose elements
is used
Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
---
libavfilter/vf_xpsnr.c | 45 ++++++++++++++++--------------------------
1 file changed, 17 insertions(+), 28 deletions(-)
diff --git a/libavfilter/vf_xpsnr.c b/libavfilter/vf_xpsnr.c
index f7348fdd6d..87e777cdde 100644
--- a/libavfilter/vf_xpsnr.c
+++ b/libavfilter/vf_xpsnr.c
@@ -60,9 +60,9 @@ typedef struct XPSNRContext {
/* XPSNR specific variables */
double *sse_luma;
double *weights;
+ int16_t *buf_org_m1;
+ int16_t *buf_org_m2;
int16_t *buf_org [3];
- int16_t *buf_org_m1[3];
- int16_t *buf_org_m2[3];
int16_t *buf_rec [3];
uint64_t max_error_64;
double sum_wdist [3];
@@ -266,8 +266,8 @@ static inline double get_avg_xpsnr (const double sqrt_wsse_val, const double su
return sum_xpsnr_val / (double) num_frames_64; /* older log-domain average */
}
-static int get_wsse(AVFilterContext *ctx, int16_t **org, int16_t **org_m1, int16_t **org_m2, int16_t **rec,
- uint64_t *const wsse64)
+static int get_wsse(AVFilterContext *ctx, int16_t **org, int16_t *org_m1,
+ int16_t *org_m2, int16_t **rec, uint64_t *const wsse64)
{
XPSNRContext *const s = ctx->priv;
const uint32_t w = s->plane_width [0]; /* luma image width in pixels */
@@ -299,8 +299,6 @@ static int get_wsse(AVFilterContext *ctx, int16_t **org, int16_t **org_m1, int16
const uint32_t s_org = stride_org[0] / s->bpp;
const int16_t *p_rec = rec[0];
const uint32_t s_rec = s->plane_width[0];
- int16_t *p_org_m1 = org_m1[0]; /* pixel */
- int16_t *p_org_m2 = org_m2[0]; /* memory */
double wsse_luma = 0.0;
for (y = 0; y < h; y += b) { /* calculate block SSE and perceptual weights */
@@ -311,7 +309,8 @@ static int get_wsse(AVFilterContext *ctx, int16_t **org, int16_t **org_m1, int16
double ms_act = 1.0, ms_act_prev = 0.0;
sse_luma[idx_blk] = calc_squared_error_and_weight(s, p_org, s_org,
- p_org_m1, p_org_m2,
+ org_m1 /* pixel */,
+ org_m2 /* memory */,
p_rec, s_rec,
x, y,
block_width, block_height,
@@ -405,12 +404,10 @@ static int do_xpsnr(FFFrameSync *fs)
const uint32_t h_blk = (h + b - 1) / b; /* luma height in units of blocks */
AVFrame *master, *ref = NULL;
int16_t *porg [3];
- int16_t *porg_m1[3];
- int16_t *porg_m2[3];
int16_t *prec [3];
uint64_t wsse64 [3] = {0, 0, 0};
double cur_xpsnr[3] = {INFINITY, INFINITY, INFINITY};
- int c, ret_value;
+ int c, ret_value, stride_org_bpp;
AVDictionary **metadata;
if ((ret_value = ff_framesync_dualinput_get(fs, &master, &ref)) < 0)
@@ -425,21 +422,15 @@ static int do_xpsnr(FFFrameSync *fs)
if (!s->weights)
s->weights = av_malloc_array(w_blk * h_blk, sizeof(double));
- for (c = 0; c < s->num_comps; c++) { /* create temporal org buffer memory */
+ for (c = 0; c < s->num_comps; c++) /* create temporal org buffer memory */
s->line_sizes[c] = master->linesize[c];
- if (c == 0) { /* luma ch. */
- const int stride_org_bpp = (s->bpp == 1 ? s->plane_width[c] : s->line_sizes[c] / s->bpp);
+ stride_org_bpp = (s->bpp == 1 ? s->plane_width[0] : s->line_sizes[0] / s->bpp);
- if (!s->buf_org_m1[c])
- s->buf_org_m1[c] = av_calloc(s->plane_height[c], stride_org_bpp * sizeof(int16_t));
- if (!s->buf_org_m2[c])
- s->buf_org_m2[c] = av_calloc(s->plane_height[c], stride_org_bpp * sizeof(int16_t));
-
- porg_m1[c] = s->buf_org_m1[c];
- porg_m2[c] = s->buf_org_m2[c];
- }
- }
+ if (!s->buf_org_m1)
+ s->buf_org_m1 = av_calloc(s->plane_height[0], stride_org_bpp * sizeof(int16_t));
+ if (!s->buf_org_m2)
+ s->buf_org_m2 = av_calloc(s->plane_height[0], stride_org_bpp * sizeof(int16_t));
if (s->bpp == 1) { /* 8 bit */
for (c = 0; c < s->num_comps; c++) { /* allocate org/rec buffer memory */
@@ -470,8 +461,7 @@ static int do_xpsnr(FFFrameSync *fs)
}
/* extended perceptually weighted peak signal-to-noise ratio (XPSNR) value */
- ret_value = get_wsse(ctx, (int16_t **) &porg, (int16_t **) &porg_m1, (int16_t **) &porg_m2,
- (int16_t **) &prec, wsse64);
+ ret_value = get_wsse(ctx, porg, s->buf_org_m1, s->buf_org_m2, prec, wsse64);
if ( ret_value < 0 )
return ret_value; /* an error here means something went wrong earlier! */
@@ -530,8 +520,6 @@ static av_cold int init(AVFilterContext *ctx)
for (c = 0; c < 3; c++) { /* initialize XPSNR data of each color component */
s->buf_org [c] = NULL;
- s->buf_org_m1[c] = NULL;
- s->buf_org_m2[c] = NULL;
s->buf_rec [c] = NULL;
s->sum_wdist [c] = 0.0;
s->sum_xpsnr [c] = 0.0;
@@ -692,9 +680,10 @@ static av_cold void uninit(AVFilterContext *ctx)
av_freep(&s->sse_luma);
av_freep(&s->weights );
+ av_freep(&s->buf_org_m1);
+ av_freep(&s->buf_org_m2);
+
for (c = 0; c < s->num_comps; c++) {
- av_freep(&s->buf_org_m1[c]);
- av_freep(&s->buf_org_m2[c]);
av_freep(&s->buf_org[c]);
av_freep(&s->buf_rec[c]);
}
--
2.45.2
_______________________________________________
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".
