[FFmpeg-devel] [PATCH v5 2/2][GSoC 2024] tests/checkasm: Add check_vvc_sad to vvc_mc.c

[James Almer]

On 5/21/2024 9:00 PM, Stone Chen wrote:
> Adds checkasm for DMVR SAD AVX2 implementation.
> 
> Benchmarks ( AMD 7940HS )
> vvc_sad_8x8_c: 50.3
> vvc_sad_8x8_avx2: 0.3
> vvc_sad_16x16_c: 250.3
> vvc_sad_16x16_avx2: 10.3
> vvc_sad_32x32_c: 1020.3
> vvc_sad_32x32_avx2: 60.3
> vvc_sad_64x64_c: 3850.3
> vvc_sad_64x64_avx2: 220.3
> vvc_sad_128x128_c: 14100.3
> vvc_sad_128x128_avx2: 840.3
> ---
>   tests/checkasm/vvc_mc.c | 38 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 38 insertions(+)
> 
> diff --git a/tests/checkasm/vvc_mc.c b/tests/checkasm/vvc_mc.c
> index 97f57cb401..f2d7a6d561 100644
> --- a/tests/checkasm/vvc_mc.c
> +++ b/tests/checkasm/vvc_mc.c
> @@ -322,8 +322,46 @@ static void check_avg(void)
>       report("avg");
>   }
>   
> +static void check_vvc_sad(void)
> +{
> +    const int bit_depth = 10;
> +    VVCDSPContext c;
> +    LOCAL_ALIGNED_32(uint16_t, src0, [MAX_CTU_SIZE * MAX_CTU_SIZE * 4]);
> +    LOCAL_ALIGNED_32(uint16_t, src1, [MAX_CTU_SIZE * MAX_CTU_SIZE * 4]);
> +    declare_func(int, const int16_t *src0, const int16_t *src1, intptr_t dx, intptr_t dy, int block_w, int block_h);

Not related to this patch, but dsp.h should mention alignment 
requirements for all the parameters in dsp functions.

> +
> +    ff_vvc_dsp_init(&c, bit_depth);
> +    memset(src0, 0, MAX_CTU_SIZE * MAX_CTU_SIZE * 4);
> +    memset(src1, 0, MAX_CTU_SIZE * MAX_CTU_SIZE * 4);

MAX_CTU_SIZE * MAX_CTU_SIZE * 4 * sizeof(uint16_t)

> +
> +    randomize_pixels(src0, src1, MAX_CTU_SIZE * MAX_CTU_SIZE * 2);

Seeing randomize_buffers() is written for uint8_t buffers, it should be 
updated for this, like so:

#define randomize_buffers(buf0, buf1, size, mask)           \
     do {                                                    \
         int k;                                              \
         for (k = 0; k < size; k += 4 / sizeof(*buf0)) {     \
             uint32_t r = rnd() & mask;                      \
             AV_WN32A(buf0 + k, r);                          \
             AV_WN32A(buf1 + k, r);                          \
         }                                                   \
     } while (0)

And the argument changed to "MAX_CTU_SIZE * MAX_CTU_SIZE * 4". Otherwise 
the loop will write 4 bytes and leave the next 4 untouched.

> +     for (int h = 8; h <= MAX_CTU_SIZE; h *= 2) {

There's an extra whitespace of indentation here.

> +        for (int w = 8; w <= MAX_CTU_SIZE; w *= 2) {
> +            for(int offy = 0; offy <= 4; offy++) {
> +                for(int offx = 0; offx <= 4; offx++) {
> +                    if(check_func(c.inter.sad, "vvc_sad_%dx%d", w, h)) {

"sad_%dx%d"

> +                        int result0;
> +                        int result1;
> +
> +                        result0 =  call_ref(src0 + PIXEL_STRIDE * 2 + 2, src1 + PIXEL_STRIDE * 2 + 2, offx, offy, w, h);
> +                        result1 =  call_new(src0 + PIXEL_STRIDE * 2 + 2, src1 + PIXEL_STRIDE * 2 + 2, offx, offy, w, h);
> +
> +                        if (result1 != result0)
> +                            fail();
> +                        if(w == h && offx == 0 && offy == 0)
> +                            bench_new(src0 + PIXEL_STRIDE * 2 + 2, src1 + PIXEL_STRIDE * 2 + 2, offx, offy, w, h);
> +                    }
> +                }
> +            }
> +        }
> +     }
> +
> +    report("check_vvc_sad");

report("sad");

> +}
> +
>   void checkasm_check_vvc_mc(void)
>   {
> +    check_vvc_sad();
>       check_put_vvc_luma();
>       check_put_vvc_luma_uni();
>       check_put_vvc_chroma();