[FFmpeg-devel] [PATCH] avfilter/vf_xpsnr: remove duplicated DSP infranstructure
Helmrich, Christian
christian.helmrich at hhi.fraunhofer.de
Mon Oct 14 13:23:24 EEST 2024
Hi,
thanks for taking a look, but I don't understand the two "unsigned error = main_line[j] - ref_line[j];" you added. Inline:
Fully reuse the existing one from vf_psnr, instead of halfways.
Signed-off-by: James Almer <jamrial at gmail.com>
---
libavfilter/Makefile | 4 +--
libavfilter/psnr.c | 64 +++++++++++++++++++++++++++++++++
libavfilter/psnr.h | 1 +
libavfilter/vf_psnr.c | 29 +--------------
libavfilter/vf_xpsnr.c | 28 ++++-----------
libavfilter/x86/Makefile | 3 +-
libavfilter/x86/vf_xpsnr_init.c | 43 ----------------------
libavfilter/xpsnr.h | 5 +--
8 files changed, 77 insertions(+), 100 deletions(-)
create mode 100644 libavfilter/psnr.c
delete mode 100644 libavfilter/x86/vf_xpsnr_init.c
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index a56c8e8b79..a3a63d326e 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -437,7 +437,7 @@ OBJS-$(CONFIG_PREWITT_OPENCL_FILTER) += vf_convolution_opencl.o opencl.o
OBJS-$(CONFIG_PROCAMP_VAAPI_FILTER) += vf_procamp_vaapi.o vaapi_vpp.o
OBJS-$(CONFIG_PROGRAM_OPENCL_FILTER) += vf_program_opencl.o opencl.o framesync.o
OBJS-$(CONFIG_PSEUDOCOLOR_FILTER) += vf_pseudocolor.o
-OBJS-$(CONFIG_PSNR_FILTER) += vf_psnr.o framesync.o
+OBJS-$(CONFIG_PSNR_FILTER) += vf_psnr.o framesync.o psnr.o
OBJS-$(CONFIG_PULLUP_FILTER) += vf_pullup.o
OBJS-$(CONFIG_QP_FILTER) += vf_qp.o
OBJS-$(CONFIG_QUIRC_FILTER) += vf_quirc.o
@@ -566,7 +566,7 @@ OBJS-$(CONFIG_XFADE_FILTER) += vf_xfade.o
OBJS-$(CONFIG_XFADE_OPENCL_FILTER) += vf_xfade_opencl.o opencl.o opencl/xfade.o
OBJS-$(CONFIG_XFADE_VULKAN_FILTER) += vf_xfade_vulkan.o vulkan.o vulkan_filter.o
OBJS-$(CONFIG_XMEDIAN_FILTER) += vf_xmedian.o framesync.o
-OBJS-$(CONFIG_XPSNR_FILTER) += vf_xpsnr.o framesync.o
+OBJS-$(CONFIG_XPSNR_FILTER) += vf_xpsnr.o framesync.o psnr.o
OBJS-$(CONFIG_XSTACK_FILTER) += vf_stack.o framesync.o
OBJS-$(CONFIG_YADIF_FILTER) += vf_yadif.o yadif_common.o
OBJS-$(CONFIG_YADIF_CUDA_FILTER) += vf_yadif_cuda.o vf_yadif_cuda.ptx.o \
diff --git a/libavfilter/psnr.c b/libavfilter/psnr.c
new file mode 100644
index 0000000000..a6b7f5969c
--- /dev/null
+++ b/libavfilter/psnr.c
@@ -0,0 +1,64 @@
+/*
+ * Copyright (c) 2015 Ronald S. Bultje <rsbultje at gmail.com>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "config.h"
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include "psnr.h"
+
+static uint64_t sse_line_8bit(const uint8_t *main_line, const uint8_t *ref_line, int outw)
+{
+ int j;
+ unsigned m2 = 0;
+
+ for (j = 0; j < outw; j++) {
+ unsigned error = main_line[j] - ref_line[j];
Why is error unsigned? The difference could be negative. In our code we had:
const int... error = (int...) blk_org[x] - (int...) blk_rec[x];
and the PSNR code had: m2 += pow_2(main_line[j] - ref_line[j]);
Is this equivalent, and I don't see it? Could this be a general issue even with the pow_2 variant?
+
+ m2 += error * error;
+ }
+
+ return m2;
+}
+
+static uint64_t sse_line_16bit(const uint8_t *_main_line, const uint8_t *_ref_line, int outw)
+{
+ int j;
+ uint64_t m2 = 0;
+ const uint16_t *main_line = (const uint16_t *) _main_line;
+ const uint16_t *ref_line = (const uint16_t *) _ref_line;
+
+ for (j = 0; j < outw; j++) {
+ unsigned error = main_line[j] - ref_line[j];
Same as above.
Best,
Christian
+
+ m2 += error * error;
+ }
+
+ return m2;
+}
+
+void ff_psnr_init(PSNRDSPContext *dsp, int bpp)
+{
+ dsp->sse_line = bpp > 8 ? sse_line_16bit : sse_line_8bit;
+#if ARCH_X86
+ ff_psnr_init_x86(dsp, bpp);
+#endif
+}
diff --git a/libavfilter/psnr.h b/libavfilter/psnr.h
index bbc4541135..2664aa5255 100644
--- a/libavfilter/psnr.h
+++ b/libavfilter/psnr.h
@@ -28,6 +28,7 @@ typedef struct PSNRDSPContext {
uint64_t (*sse_line)(const uint8_t *buf, const uint8_t *ref, int w);
} PSNRDSPContext;
+void ff_psnr_init(PSNRDSPContext *dsp, int bpp);
void ff_psnr_init_x86(PSNRDSPContext *dsp, int bpp);
#endif /* AVFILTER_PSNR_H */
diff --git a/libavfilter/vf_psnr.c b/libavfilter/vf_psnr.c
index 4a5db5df23..1e8f24fc52 100644
--- a/libavfilter/vf_psnr.c
+++ b/libavfilter/vf_psnr.c
@@ -82,30 +82,6 @@ static inline double get_psnr(double mse, uint64_t nb_frames, int max)
return 10.0 * log10(pow_2(max) / (mse / nb_frames));
}
-static uint64_t sse_line_8bit(const uint8_t *main_line, const uint8_t *ref_line, int outw)
-{
- int j;
- unsigned m2 = 0;
-
- for (j = 0; j < outw; j++)
- m2 += pow_2(main_line[j] - ref_line[j]);
-
- return m2;
-}
-
-static uint64_t sse_line_16bit(const uint8_t *_main_line, const uint8_t *_ref_line, int outw)
-{
- int j;
- uint64_t m2 = 0;
- const uint16_t *main_line = (const uint16_t *) _main_line;
- const uint16_t *ref_line = (const uint16_t *) _ref_line;
-
- for (j = 0; j < outw; j++)
- m2 += pow_2(main_line[j] - ref_line[j]);
-
- return m2;
-}
-
typedef struct ThreadData {
const uint8_t *main_data[4];
const uint8_t *ref_data[4];
@@ -358,10 +334,7 @@ static int config_input_ref(AVFilterLink *inlink)
}
s->average_max = lrint(average_max);
- s->dsp.sse_line = desc->comp[0].depth > 8 ? sse_line_16bit : sse_line_8bit;
-#if ARCH_X86
- ff_psnr_init_x86(&s->dsp, desc->comp[0].depth);
-#endif
+ ff_psnr_init(&s->dsp, desc->comp[0].depth);
s->score = av_calloc(s->nb_threads, sizeof(*s->score));
if (!s->score)
diff --git a/libavfilter/vf_xpsnr.c b/libavfilter/vf_xpsnr.c
index 097258bf47..f8557dc8f2 100644
--- a/libavfilter/vf_xpsnr.c
+++ b/libavfilter/vf_xpsnr.c
@@ -36,6 +36,7 @@
#include "drawutils.h"
#include "filters.h"
#include "framesync.h"
+#include "psnr.h"
#include "xpsnr.h"
/* XPSNR structure definition */
@@ -68,7 +69,8 @@ typedef struct XPSNRContext {
double sum_xpsnr [3];
int and_is_inf[3];
int is_rgb;
- PSNRDSPContext dsp;
+ XPSNRDSPContext dsp;
+ PSNRDSPContext pdsp;
} XPSNRContext;
/* required macro definitions */
@@ -142,22 +144,6 @@ static uint64_t diff2nd(const uint32_t w_act, const uint32_t h_act, const int16_
return (ta_act * XPSNR_GAMMA);
}
-static uint64_t sse_line_16bit(const uint8_t *blk_org8, const uint8_t *blk_rec8, int block_width)
-{
- const uint16_t *blk_org = (const uint16_t *) blk_org8;
- const uint16_t *blk_rec = (const uint16_t *) blk_rec8;
- uint64_t sse = 0; /* sum for one pixel line */
-
- for (int x = 0; x < block_width; x++) {
- const int64_t error = (int64_t) blk_org[x] - (int64_t) blk_rec[x];
-
- sse += error * error;
- }
-
- /* sum of squared errors for the pixel line */
- return sse;
-}
-
static inline uint64_t calc_squared_error(XPSNRContext const *s,
const int16_t *blk_org, const uint32_t stride_org,
const int16_t *blk_rec, const uint32_t stride_rec,
@@ -166,7 +152,7 @@ static inline uint64_t calc_squared_error(XPSNRContext const *s,
uint64_t sse = 0; /* sum of squared errors */
for (uint32_t y = 0; y < block_height; y++) {
- sse += s->dsp.sse_line((const uint8_t *) blk_org, (const uint8_t *) blk_rec, (int) block_width);
+ sse += s->pdsp.sse_line((const uint8_t *) blk_org, (const uint8_t *) blk_rec, (int) block_width);
blk_org += stride_org;
blk_rec += stride_rec;
}
@@ -609,13 +595,11 @@ static int config_input_ref(AVFilterLink *inlink)
s->plane_height[1] = s->plane_height[2] = AV_CEIL_RSHIFT(inlink->h, desc->log2_chroma_h);
s->plane_height[0] = s->plane_height[3] = inlink->h;
- s->dsp.sse_line = sse_line_16bit;
+ /* XPSNR always operates with 16-bit internal precision */
+ ff_psnr_init(&s->pdsp, 15);
s->dsp.highds_func = highds; /* initialize filtering methods */
s->dsp.diff1st_func = diff1st;
s->dsp.diff2nd_func = diff2nd;
-#if ARCH_X86
- ff_xpsnr_init_x86(&s->dsp, 15); /* initialize x86 SSE method */
-#endif
return 0;
}
diff --git a/libavfilter/x86/Makefile b/libavfilter/x86/Makefile
index 40d5b790ac..0d9a28a935 100644
--- a/libavfilter/x86/Makefile
+++ b/libavfilter/x86/Makefile
@@ -40,7 +40,7 @@ OBJS-$(CONFIG_TRANSPOSE_FILTER) += x86/vf_transpose_init.o
OBJS-$(CONFIG_VOLUME_FILTER) += x86/af_volume_init.o
OBJS-$(CONFIG_V360_FILTER) += x86/vf_v360_init.o
OBJS-$(CONFIG_W3FDIF_FILTER) += x86/vf_w3fdif_init.o
-OBJS-$(CONFIG_XPSNR_FILTER) += x86/vf_xpsnr_init.o
+OBJS-$(CONFIG_XPSNR_FILTER) += x86/vf_psnr_init.o
OBJS-$(CONFIG_YADIF_FILTER) += x86/vf_yadif_init.o
X86ASM-OBJS-$(CONFIG_SCENE_SAD) += x86/scene_sad.o
@@ -85,4 +85,5 @@ X86ASM-OBJS-$(CONFIG_TRANSPOSE_FILTER) += x86/vf_transpose.o
X86ASM-OBJS-$(CONFIG_VOLUME_FILTER) += x86/af_volume.o
X86ASM-OBJS-$(CONFIG_V360_FILTER) += x86/vf_v360.o
X86ASM-OBJS-$(CONFIG_W3FDIF_FILTER) += x86/vf_w3fdif.o
+X86ASM-OBJS-$(CONFIG_XPSNR_FILTER) += x86/vf_psnr.o
X86ASM-OBJS-$(CONFIG_YADIF_FILTER) += x86/vf_yadif.o x86/yadif-16.o x86/yadif-10.o
diff --git a/libavfilter/x86/vf_xpsnr_init.c b/libavfilter/x86/vf_xpsnr_init.c
deleted file mode 100644
index d33657dcd5..0000000000
--- a/libavfilter/x86/vf_xpsnr_init.c
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * Copyright (c) 2024 Christian R. Helmrich
- * Copyright (c) 2024 Christian Lehmann
- * Copyright (c) 2024 Christian Stoffers
- *
- * This file is part of FFmpeg.
- *
- * FFmpeg is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * FFmpeg is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with FFmpeg; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-/**
- * @file
- * SIMD initialization for calculation of extended perceptually weighted PSNR (XPSNR).
- *
- * Authors: Christian Helmrich, Lehmann, and Stoffers, Fraunhofer HHI, Berlin, Germany
- */
-
-#include "libavutil/x86/cpu.h"
-#include "libavfilter/xpsnr.h"
-
-uint64_t ff_sse_line_16bit_sse2(const uint8_t *buf, const uint8_t *ref, const int w);
-
-void ff_xpsnr_init_x86(PSNRDSPContext *dsp, const int bpp)
-{
- if (bpp <= 15) { /* XPSNR always operates with 16-bit internal precision */
- const int cpu_flags = av_get_cpu_flags();
-
- if (EXTERNAL_SSE2(cpu_flags))
- dsp->sse_line = ff_sse_line_16bit_sse2;
- }
-}
diff --git a/libavfilter/xpsnr.h b/libavfilter/xpsnr.h
index eb14e16062..99abbaf065 100644
--- a/libavfilter/xpsnr.h
+++ b/libavfilter/xpsnr.h
@@ -37,12 +37,9 @@
/* public XPSNR DSP structure definition */
typedef struct XPSNRDSPContext {
- uint64_t (*sse_line) (const uint8_t *buf, const uint8_t *ref, const int w);
uint64_t (*highds_func) (const int x_act, const int y_act, const int w_act, const int h_act, const int16_t *o_m0, const int o);
uint64_t (*diff1st_func)(const uint32_t w_act, const uint32_t h_act, const int16_t *o_m0, int16_t *o_m1, const int o);
uint64_t (*diff2nd_func)(const uint32_t w_act, const uint32_t h_act, const int16_t *o_m0, int16_t *o_m1, int16_t *o_m2, const int o);
-} PSNRDSPContext;
-
-void ff_xpsnr_init_x86(PSNRDSPContext *dsp, const int bpp);
+} XPSNRDSPContext;
#endif /* AVFILTER_XPSNR_H */
--
2.46.2
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel at ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list