Re: [libav-devel] [PATCH 2/2] checkasm: vp8: mc: test unequal width/height for partitions

[Martin Storsjö]

On Sun, 10 Jul 2016, Janne Grunau wrote:

On 2016-07-10 13:15:27 +0300, Martin Storsjö wrote:

On Sun, 10 Jul 2016, Janne Grunau wrote:

---
tests/checkasm/vp8dsp.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tests/checkasm/vp8dsp.c b/tests/checkasm/vp8dsp.c
index 3fe09c5..9733ff7 100644
--- a/tests/checkasm/vp8dsp.c
+++ b/tests/checkasm/vp8dsp.c
@@ -268,15 +268,17 @@ static void check_mc(void)
   LOCAL_ALIGNED_16(uint8_t, dst0, [16 * 16]);
   LOCAL_ALIGNED_16(uint8_t, dst1, [16 * 16]);
   VP8DSPContext d;
-    int type, hsize, dx, dy;
+    int type, k, dx, dy;
   declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *, ptrdiff_t, uint8_t *, 
ptrdiff_t, int, int, int);

   ff_vp78dsp_init(&d);

   for (type = 0; type < 2; type++) {
       vp8_mc_func (*tab)[3][3] = type ? d.put_vp8_bilinear_pixels_tab : 
d.put_vp8_epel_pixels_tab;
-        for (hsize = 0; hsize < 3; hsize++) {
-            int size = 16 >> hsize;
+        for (k = 1; k < 8; k++) {
+            int hsize  = k / 3;
+            int size   = 16 >> hsize;
+            int height = (size << 1) >> (k % 3);
           for (dy = 0; dy < 3; dy++) {
               for (dx = 0; dx < 3; dx++) {
                   char str[100];
@@ -309,11 +311,11 @@ static void check_mc(void)
                           src[i                 ] = val;
                           src[i * SRC_BUF_STRIDE] = val;
                       }
-                        call_ref(dst0, size, src, SRC_BUF_STRIDE, size, mx, 
my);
-                        call_new(dst1, size, src, SRC_BUF_STRIDE, size, mx, 
my);
-                        if (memcmp(dst0, dst1, size * size))
+                        call_ref(dst0, size, src, SRC_BUF_STRIDE, height, mx, 
my);
+                        call_new(dst1, size, src, SRC_BUF_STRIDE, height, mx, 
my);
+                        if (memcmp(dst0, dst1, size * height))
                           fail();
-                        bench_new(dst1, size, src, SRC_BUF_STRIDE, size, mx, 
my);
+                        bench_new(dst1, size, src, SRC_BUF_STRIDE, height, mx, 
my);
                   }
               }
           }
--
2.9.0

Ok with me, although this makes the benchmarking a little icky. (In case it
does things like skipping runs that are much longer than the average, it'd
be problematic, but I'm not sure if it does that - I remember seeing such
code somewhere (at least in START_TIMER/STOP_TIMER) but not sure if it ever
was in checkasm.)

checkasm has that too but only inside bench_new i.e. calls with the same
parameters. It is intended to reject benchmark runs which were
interrupted or scheduled to another cpu by the scheduler since the cpu
cycle count is invalid if that happens.

Right, ok

Should we only benchmark the height=size case to make it simpler?

I don't think it makes anything simpler. The numbers printed by
benchmark will be of course the average of all heights.

Ok, fair enough. Patch ok with me then.

// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel