[FFmpeg-devel] [PATCH 2/2] RFC: checkasm: motion: Test different h parameters

[Martin Storsjö]

On Wed, 13 Jul 2022, Martin Storsjö wrote:

> Previously, the checkasm test always passed h=8, so no other cases
> were tested.
>
> Out of the me_cmp functions, in practice, some functions are hardcoded
> to always assume a 8x8 block (ignoring the h parameter), while others
> do use the parameter. For those with hardcoded height, both the
> reference C function and the assembly implementations ignore the
> parameter similarly.
>
> The documentation for the functions indicate that heights between
> w/2 and 2*w, within the range of 4 to 16, should be supported. This
> patch just tests random heights in that range, without knowing what
> width the current function actually uses.
> ---
> I'm not sure if it's good to have checkasm exercise cases that
> don't occur in practice or not. In particular, the aarch64
> functions have a separate implementation for non-multiple-of-4
> height, which probably doesn't ever get called in practice, while
> other SIMD implementations lack that.
>
> Alternatively, we'd improve the documentation for the expectations
> for these functions and make the test match that, and remove the
> unused non-multiple-of-4 case in the aarch64 assembly.
> ---
> tests/checkasm/motion.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/tests/checkasm/motion.c b/tests/checkasm/motion.c
> index 0112822174..79e4358941 100644
> --- a/tests/checkasm/motion.c
> +++ b/tests/checkasm/motion.c
> @@ -45,7 +45,7 @@ static void test_motion(const char *name, me_cmp_func test_func)
>     /* motion estimation can look up to 17 bytes ahead */
>     static const int look_ahead = 17;
>
> -    int i, x, y, d1, d2;
> +    int i, x, y, h, d1, d2;
>     uint8_t *ptr;
>
>     LOCAL_ALIGNED_16(uint8_t, img1, [WIDTH * HEIGHT]);
> @@ -68,14 +68,16 @@ static void test_motion(const char *name, me_cmp_func test_func)
>         for (i = 0; i < ITERATIONS; i++) {
>             x = rnd() % (WIDTH - look_ahead);
>             y = rnd() % (HEIGHT - look_ahead);
> +            // Pick a random h between 4 and 16; pick an even value.
> +            h = 4 + ((rnd() % (16 + 1 - 4)) & ~1);
>
>             ptr = img2 + y * WIDTH + x;
> -            d2 = call_ref(NULL, img1, ptr, WIDTH, 8);
> -            d1 = call_new(NULL, img1, ptr, WIDTH, 8);
> +            d2 = call_ref(NULL, img1, ptr, WIDTH, h);
> +            d1 = call_new(NULL, img1, ptr, WIDTH, h);
>
>             if (d1 != d2) {
>                 fail();
> -                printf("func: %s, x=%d y=%d, error: asm=%d c=%d\n", name, x, y, d1, d2);
> +                printf("func: %s, x=%d y=%d h=%d, error: asm=%d c=%d\n", name, x, y, h, d1, d2);
>                 break;
>             }
>         }
> -- 
> 2.25.1

Ping

// Martin