From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 8CFA043E5A for ; Tue, 16 Aug 2022 11:17:02 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id D95AC68B865; Tue, 16 Aug 2022 14:16:59 +0300 (EEST) Received: from mail8.parnet.fi (mail8.parnet.fi [77.234.108.134]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 2541A68B6E8 for ; Tue, 16 Aug 2022 14:16:54 +0300 (EEST) Received: from mail9.parnet.fi (mail9.parnet.fi [77.234.108.21]) by mail8.parnet.fi with ESMTP id 27GBGmOw019367-27GBGmOx019367; Tue, 16 Aug 2022 14:16:48 +0300 Received: from foo.martin.st (host-97-187.parnet.fi [77.234.97.187]) by mail9.parnet.fi (Postfix) with ESMTPS id D29CEA146D; Tue, 16 Aug 2022 14:16:48 +0300 (EEST) Date: Tue, 16 Aug 2022 14:16:47 +0300 (EEST) From: =?ISO-8859-15?Q?Martin_Storsj=F6?= To: ffmpeg-devel@ffmpeg.org In-Reply-To: Message-ID: <6628ddc9-793-e520-918f-368a9993e978@martin.st> References: <20220713204716.3114529-1-martin@martin.st> <20220713204716.3114529-2-martin@martin.st> MIME-Version: 1.0 X-FE-Policy-ID: 3:14:2:SYSTEM X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [FFmpeg-devel] [PATCH 2/2] RFC: checkasm: motion: Test different h parameters X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Michael Niedermayer , Jonathan Swinney Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-15"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Thu, 4 Aug 2022, Martin Storsj=F6 wrote: > On Wed, 13 Jul 2022, Martin Storsj=F6 wrote: > >> Previously, the checkasm test always passed h=3D8, 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 =3D 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_fun= c = >> test_func) >> for (i =3D 0; i < ITERATIONS; i++) { >> x =3D rnd() % (WIDTH - look_ahead); >> y =3D rnd() % (HEIGHT - look_ahead); >> + // Pick a random h between 4 and 16; pick an even value. >> + h =3D 4 + ((rnd() % (16 + 1 - 4)) & ~1); >> >> ptr =3D img2 + y * WIDTH + x; >> - d2 =3D call_ref(NULL, img1, ptr, WIDTH, 8); >> - d1 =3D call_new(NULL, img1, ptr, WIDTH, 8); >> + d2 =3D call_ref(NULL, img1, ptr, WIDTH, h); >> + d1 =3D call_new(NULL, img1, ptr, WIDTH, h); >> >> if (d1 !=3D d2) { >> fail(); >> - printf("func: %s, x=3D%d y=3D%d, error: asm=3D%d c=3D%d= \n", name, = >> x, y, d1, d2); >> + printf("func: %s, x=3D%d y=3D%d h=3D%d, error: asm=3D%d= c=3D%d\n", = >> name, x, y, h, d1, d2); >> break; >> } >> } >> -- = >> 2.25.1 > > Ping As there doesn't seem to be much opinion on this, I'll go ahead and land = this (patch 1/2 was approved already), as this improves the test coverage = for the aarch64 neon me_cmp assembly patches that are in progress. // 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-request@ffmpeg.org with subject "unsubscribe".