[x265] [PATCH] denoiseDct: unit test code

[Steve Borho]

On 09/15, praveen at multicorewareinc.com wrote:
> # HG changeset patch
> # User Praveen Tiwari
> # Date 1410775657 -19800
> # Node ID 36f5477f54ba8047f9abc1b42c5b56c6d223dc5a
> # Parent  184e56afa951815f4e295b4fcce094ee03361a2e
> denoiseDct: unit test code

a few nits and questions

> diff -r 184e56afa951 -r 36f5477f54ba source/test/mbdstharness.cpp
> --- a/source/test/mbdstharness.cpp	Fri Sep 12 12:02:46 2014 +0530
> +++ b/source/test/mbdstharness.cpp	Mon Sep 15 15:37:37 2014 +0530
> @@ -66,14 +66,17 @@
>          short_test_buff[0][i]    = (rand() & PIXEL_MAX) - (rand() & PIXEL_MAX);
>          int_test_buff[0][i]      = rand() % PIXEL_MAX;
>          int_idct_test_buff[0][i] = (rand() % (SHORT_MAX - SHORT_MIN)) - SHORT_MAX;
> +        int_denoise_test_buff1[0][i] = int_denoise_test_buff2[0][i] = (rand() & UNSIGNED_SHORT_MAX) - (rand() & UNSIGNED_SHORT_MAX);
>  
>          short_test_buff[1][i]    = -PIXEL_MAX;
>          int_test_buff[1][i]      = -PIXEL_MAX;
>          int_idct_test_buff[1][i] = SHORT_MIN;
> +        int_denoise_test_buff1[1][i] = int_denoise_test_buff2[1][i] = -UNSIGNED_SHORT_MAX;
>  
>          short_test_buff[2][i]    = PIXEL_MAX;
>          int_test_buff[2][i]      = PIXEL_MAX;
>          int_idct_test_buff[2][i] = SHORT_MAX;
> +        int_denoise_test_buff1[2][i] = int_denoise_test_buff2[1][i] = UNSIGNED_SHORT_MAX;
>  
>          mbuf1[i] = rand() & PIXEL_MAX;
>          mbufdct[i] = (rand() & PIXEL_MAX) - (rand() & PIXEL_MAX);
> @@ -313,6 +316,46 @@
>      return true;
>  }
>  
> +bool MBDstHarness::check_denoise_dct_primitive(denoiseDct_t ref, denoiseDct_t opt)
> +{
> +    int j = 0;
> +
> +    for (int i = 0; i < 4; i++)
> +    {
> +        int log2TrSize = i + 2;
> +        int num = 1 << (log2TrSize * 2);

This loop second confuses me? what's the point of it?

> +        for (int n = 0; n <= num; n++)
> +        {
> +            memset(mubuf1, 0, num * sizeof(uint32_t));
> +            memset(mubuf2, 0, num * sizeof(uint32_t));
> +            memset(mushortbuf1, 0,  num * sizeof(uint16_t));
> +
> +            for (int k = 0; k < n; j++)
> +            {
> +                mushortbuf1[k] = rand() % UNSIGNED_SHORT_MAX;
> +            }

we don't use braces for single-line expressions

> +            int index = rand() % TEST_CASES;
> +            int cmp_size = sizeof(int) * num;
> +
> +            ref(int_denoise_test_buff1[index] + j, mubuf1, mushortbuf1, num);
> +            checked(opt, int_denoise_test_buff2[index] + j, mubuf2, mushortbuf1, num);
> +
> +            if (memcmp(int_denoise_test_buff1[index] + j, int_denoise_test_buff2[index] + j, cmp_size))
> +            return false;

white-space

> +            if (memcmp(mubuf1, mubuf2, cmp_size))
> +            return false;
> +
> +            reportfail();
> +            j += INCR;

is this bounds safe? TEST_BUF_SIZE is allocated for a max of ITERS
iterations (128). It seems like num can be 32*32.

> +        }
> +    }
> +
> +    return true;
> +}
> +
>  bool MBDstHarness::testCorrectness(const EncoderPrimitives& ref, const EncoderPrimitives& opt)
>  {
>      for (int i = 0; i < NUM_DCTS; i++)
> @@ -393,6 +436,15 @@
>          }
>      }
>  
> +    if (opt.denoiseDct)
> +    {
> +        if (!check_denoise_dct_primitive(ref.denoiseDct, opt.denoiseDct))
> +        {
> +            printf("denoiseDct: Failed!\n");
> +            return false;
> +        }
> +    }
> +
>      return true;
>  }
>  
> @@ -448,4 +500,10 @@
>              REPORT_SPEEDUP(opt.count_nonzero, ref.count_nonzero, mbuf1, i * i)
>          }
>      }
> +
> +    if (opt.denoiseDct)
> +    {
> +        printf("denoiseDct\t\t");
> +        REPORT_SPEEDUP(opt.denoiseDct, ref.denoiseDct, int_denoise_test_buff1[0], mubuf1, mushortbuf1, 32 * 32);
> +    }
>  }
> diff -r 184e56afa951 -r 36f5477f54ba source/test/mbdstharness.h
> --- a/source/test/mbdstharness.h	Fri Sep 12 12:02:46 2014 +0530
> +++ b/source/test/mbdstharness.h	Mon Sep 15 15:37:37 2014 +0530
> @@ -44,6 +44,10 @@
>      int16_t mbufdct[TEST_BUF_SIZE];
>      int     mbufidct[TEST_BUF_SIZE];
>  
> +    ALIGN_VAR_32(uint32_t, mubuf1[MAX_TU_SIZE]);
> +    ALIGN_VAR_32(uint32_t, mubuf2[MAX_TU_SIZE]);
> +    ALIGN_VAR_32(uint16_t, mushortbuf1[MAX_TU_SIZE]);

does denoise need all new buffers? can it reuse existing buffers?

There's no need to declare them aligned here. The first array is
declared aligned and since all below it are also aligned in size every
array is implicitly aligned.

>      int16_t mshortbuf2[MAX_TU_SIZE];
>      int16_t mshortbuf3[MAX_TU_SIZE];
>  
> @@ -56,6 +60,9 @@
>      int     int_test_buff[TEST_CASES][TEST_BUF_SIZE];
>      int     int_idct_test_buff[TEST_CASES][TEST_BUF_SIZE];
>  
> +    int int_denoise_test_buff1[TEST_CASES][TEST_BUF_SIZE];
> +    int int_denoise_test_buff2[TEST_CASES][TEST_BUF_SIZE];
> +
>      bool check_dequant_primitive(dequant_scaling_t ref, dequant_scaling_t opt);
>      bool check_dequant_primitive(dequant_normal_t ref, dequant_normal_t opt);
>      bool check_quant_primitive(quant_t ref, quant_t opt);
> @@ -63,6 +70,7 @@
>      bool check_dct_primitive(dct_t ref, dct_t opt, intptr_t width);
>      bool check_idct_primitive(idct_t ref, idct_t opt, intptr_t width);
>      bool check_count_nonzero_primitive(count_nonzero_t ref, count_nonzero_t opt);
> +    bool check_denoise_dct_primitive(denoiseDct_t ref, denoiseDct_t opt);
>  
>  public:
>  
> diff -r 184e56afa951 -r 36f5477f54ba source/test/testharness.h
> --- a/source/test/testharness.h	Fri Sep 12 12:02:46 2014 +0530
> +++ b/source/test/testharness.h	Mon Sep 15 15:37:37 2014 +0530
> @@ -40,6 +40,7 @@
>  #define PIXEL_MIN 0
>  #define SHORT_MAX  32767
>  #define SHORT_MIN -32767
> +#define UNSIGNED_SHORT_MAX 65535
>  
>  using namespace x265;
>  
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel

-- 
Steve Borho