On Tue, 5 Dec 2023, J. Dekker wrote:
Signed-off-by: J. Dekker <j...@itanimul.li>
---
tests/checkasm/hevc_deblock.c | 110 ++++++++++++++++++++++++++++++++--
1 file changed, 106 insertions(+), 4 deletions(-)
Yes, this only supports 8bit. 10/12bit should be trivial, will add if this
looks reasonable (I checked code paths using gdb, and as far as I can tell it
does test all three). Tested on known good x86 asm.
Overall I think this looks mostly good.
A few pointers for things that could be improved later, in addition to
10/12 bit support, and a few minor things to fix before pushing:
diff --git a/tests/checkasm/hevc_deblock.c b/tests/checkasm/hevc_deblock.c
index 66fc8d5646..3f970a470a 100644
--- a/tests/checkasm/hevc_deblock.c
+++ b/tests/checkasm/hevc_deblock.c
@@ -29,8 +29,8 @@
static const uint32_t pixel_mask[3] = { 0xffffffff, 0x03ff03ff, 0x0fff0fff };
#define SIZEOF_PIXEL ((bit_depth + 7) / 8)
-#define BUF_STRIDE (8 * 2)
-#define BUF_LINES (8)
+#define BUF_STRIDE (16 * 2)
+#define BUF_LINES (16)
#define BUF_OFFSET (BUF_STRIDE * BUF_LINES)
#define BUF_SIZE (BUF_STRIDE * BUF_LINES + BUF_OFFSET * 2)
For these calculations, make sure to check that things stay within the
expected bounds for all reads/writes, if you extend it to 10/12 bits
later.
@@ -88,14 +88,116 @@ static void check_deblock_chroma(HEVCDSPContext *h, int
bit_depth)
}
}
+// line zero
+#define P3 buf[-4 * xstride]
+#define P2 buf[-3 * xstride]
+#define P1 buf[-2 * xstride]
+#define P0 buf[-1 * xstride]
+#define Q0 buf[0 * xstride]
+#define Q1 buf[1 * xstride]
+#define Q2 buf[2 * xstride]
+#define Q3 buf[3 * xstride]
+
+// line three. used only for deblocking decision
I don't immediately see what these comments about line zero and line three
mean here.
+#define TP3 buf[-4 * xstride + 3 * ystride]
+#define TP2 buf[-3 * xstride + 3 * ystride]
+#define TP1 buf[-2 * xstride + 3 * ystride]
+#define TP0 buf[-1 * xstride + 3 * ystride]
+#define TQ0 buf[0 * xstride + 3 * ystride]
+#define TQ1 buf[1 * xstride + 3 * ystride]
+#define TQ2 buf[2 * xstride + 3 * ystride]
+#define TQ3 buf[3 * xstride + 3 * ystride]
Hmm, this seems to be unused currently?
+
+static void randomize_luma_buffers(int type, uint8_t *buf, ptrdiff_t xstride,
ptrdiff_t ystride)
+{
+ int i;
+ buf += BUF_OFFSET;
+ switch (type) {
+ case 0: // strong
+ for (i = 0; i < 16; i++) {
+ P3 = P2 = P1 = P0 = 64;
+ Q0 = Q1 = Q2 = Q3 = 80;
+ buf += ystride;
+ }
+ break;
+ case 1: // weak
+ for (i = 0; i < 16; i++) {
+ P3 = P2 = 60; P1 = P0 = 64;
+ Q0 = Q1 = 74; Q2 = Q3 = 80;
+ buf += ystride;
+ }
These two aren't very random right now :-) Can we fit within the same
criteria while keeping things somewhat random? E.g. "P3 = P2 + (rand() %
2*range) - range", or something along those lines?
+static void check_deblock_luma(HEVCDSPContext *h, int bit_depth)
+{
+ const char *type;
+ const char *types[3] = { "strong", "normal", "skip" };
+ int beta;
+ int32_t tc[2] = { 0, 0 };
+ // no_p, no_q can only be { 0,0 } for the simpler assembly (non *_c
+ // variant) functions, see deblocking_filter_CTB() in hevc_filter.c
Can we extend this to test the _c variant as well?
+ uint8_t no_p[2] = { 0, 0 };
+ uint8_t no_q[2] = { 0, 0 };
+ LOCAL_ALIGNED_32(uint8_t, buf0, [BUF_SIZE]);
+ LOCAL_ALIGNED_32(uint8_t, buf1, [BUF_SIZE]);
+
+ declare_func(void, uint8_t *pix, ptrdiff_t stride, int beta, int32_t *tc,
uint8_t *no_p, uint8_t *no_q);
+
+ for (int j = 0; j < 3; j++) {
+ beta = (j == 3) ? 0 : 32; // beta easy way to turn off filtering
We never hit j==3 here, did you intend to have a fourth type included
here?
Can we use a less hardcoded beta value here, while still triggering the
right cases?
+ type = types[j];
+
+ // see betatable[] in hevc_filter.c
+ tc[0] = (rnd() & 63) + (rnd() & 1);
+ tc[1] = (rnd() & 63) + (rnd() & 1);
Hmm, I don't immediately see how the tc[] values relate to betatable[] on
a first glance
// Martin
_______________________________________________
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".
