<div dir="ltr"><br><div class="gmail_quote">---------- Forwarded message ----------<br>From: <b class="gmail_sendername">Steve Borho</b> <span dir="ltr"><<a href="mailto:steve@borho.org">steve@borho.org</a>></span><br>Date: Mon, Sep 15, 2014 at 4:28 PM<br>Subject: Re: [x265] [PATCH] denoiseDct: unit test code<br>To: Development for x265 <<a href="mailto:x265-devel@videolan.org">x265-devel@videolan.org</a>><br><br><br><span class="">On 09/15, <a href="mailto:praveen@multicorewareinc.com">praveen@multicorewareinc.com</a> wrote:<br>
> # HG changeset patch<br>
> # User Praveen Tiwari<br>
> # Date 1410775657 -19800<br>
> # Node ID 36f5477f54ba8047f9abc1b42c5b56c6d223dc5a<br>
> # Parent 184e56afa951815f4e295b4fcce094ee03361a2e<br>
> denoiseDct: unit test code<br>
<br>
</span>a few nits and questions<br>
<div><div class="h5"><br>
> diff -r 184e56afa951 -r 36f5477f54ba source/test/mbdstharness.cpp<br>
> --- a/source/test/mbdstharness.cpp Fri Sep 12 12:02:46 2014 +0530<br>
> +++ b/source/test/mbdstharness.cpp Mon Sep 15 15:37:37 2014 +0530<br>
> @@ -66,14 +66,17 @@<br>
> short_test_buff[0][i] = (rand() & PIXEL_MAX) - (rand() & PIXEL_MAX);<br>
> int_test_buff[0][i] = rand() % PIXEL_MAX;<br>
> int_idct_test_buff[0][i] = (rand() % (SHORT_MAX - SHORT_MIN)) - SHORT_MAX;<br>
> + int_denoise_test_buff1[0][i] = int_denoise_test_buff2[0][i] = (rand() & UNSIGNED_SHORT_MAX) - (rand() & UNSIGNED_SHORT_MAX);<br>
><br>
> short_test_buff[1][i] = -PIXEL_MAX;<br>
> int_test_buff[1][i] = -PIXEL_MAX;<br>
> int_idct_test_buff[1][i] = SHORT_MIN;<br>
> + int_denoise_test_buff1[1][i] = int_denoise_test_buff2[1][i] = -UNSIGNED_SHORT_MAX;<br>
><br>
> short_test_buff[2][i] = PIXEL_MAX;<br>
> int_test_buff[2][i] = PIXEL_MAX;<br>
> int_idct_test_buff[2][i] = SHORT_MAX;<br>
> + int_denoise_test_buff1[2][i] = int_denoise_test_buff2[1][i] = UNSIGNED_SHORT_MAX;<br>
><br>
> mbuf1[i] = rand() & PIXEL_MAX;<br>
> mbufdct[i] = (rand() & PIXEL_MAX) - (rand() & PIXEL_MAX);<br>
> @@ -313,6 +316,46 @@<br>
> return true;<br>
> }<br>
><br>
> +bool MBDstHarness::check_denoise_dct_primitive(denoiseDct_t ref, denoiseDct_t opt)<br>
> +{<br>
> + int j = 0;<br>
> +<br>
> + for (int i = 0; i < 4; i++)<br>
> + {<br>
> + int log2TrSize = i + 2;<br>
> + int num = 1 << (log2TrSize * 2);<br>
<br>
</div></div>This loop second confuses me? what's the point of it?<br>
<span class=""><br>
> + for (int n = 0; n <= num; n++)<br>
> + {<br>
> + memset(mubuf1, 0, num * sizeof(uint32_t));<br>
> + memset(mubuf2, 0, num * sizeof(uint32_t));<br>
> + memset(mushortbuf1, 0, num * sizeof(uint16_t));<br>
> +<br>
> + for (int k = 0; k < n; j++)<br>
> + {<br>
> + mushortbuf1[k] = rand() % UNSIGNED_SHORT_MAX;<br>
> + }<br>
<br>
</span>we don't use braces for single-line expressions<br>
<span class=""><br>
> + int index = rand() % TEST_CASES;<br>
> + int cmp_size = sizeof(int) * num;<br>
> +<br>
> + ref(int_denoise_test_buff1[index] + j, mubuf1, mushortbuf1, num);<br>
> + checked(opt, int_denoise_test_buff2[index] + j, mubuf2, mushortbuf1, num);<br>
> +<br>
> + if (memcmp(int_denoise_test_buff1[index] + j, int_denoise_test_buff2[index] + j, cmp_size))<br>
> + return false;<br>
<br>
</span>white-space<br>
<span class=""><br>
> + if (memcmp(mubuf1, mubuf2, cmp_size))<br>
> + return false;<br>
> +<br>
> + reportfail();<br>
> + j += INCR;<br>
<br>
</span>is this bounds safe? TEST_BUF_SIZE is allocated for a max of ITERS<br>
iterations (128). It seems like num can be 32*32.<br>
<div><div class="h5"><br>
> + }<br>
> + }<br>
> +<br>
> + return true;<br>
> +}<br>
> +<br>
> bool MBDstHarness::testCorrectness(const EncoderPrimitives& ref, const EncoderPrimitives& opt)<br>
> {<br>
> for (int i = 0; i < NUM_DCTS; i++)<br>
> @@ -393,6 +436,15 @@<br>
> }<br>
> }<br>
><br>
> + if (opt.denoiseDct)<br>
> + {<br>
> + if (!check_denoise_dct_primitive(ref.denoiseDct, opt.denoiseDct))<br>
> + {<br>
> + printf("denoiseDct: Failed!\n");<br>
> + return false;<br>
> + }<br>
> + }<br>
> +<br>
> return true;<br>
> }<br>
><br>
> @@ -448,4 +500,10 @@<br>
> REPORT_SPEEDUP(opt.count_nonzero, ref.count_nonzero, mbuf1, i * i)<br>
> }<br>
> }<br>
> +<br>
> + if (opt.denoiseDct)<br>
> + {<br>
> + printf("denoiseDct\t\t");<br>
> + REPORT_SPEEDUP(opt.denoiseDct, ref.denoiseDct, int_denoise_test_buff1[0], mubuf1, mushortbuf1, 32 * 32);<br>
> + }<br>
> }<br>
> diff -r 184e56afa951 -r 36f5477f54ba source/test/mbdstharness.h<br>
> --- a/source/test/mbdstharness.h Fri Sep 12 12:02:46 2014 +0530<br>
> +++ b/source/test/mbdstharness.h Mon Sep 15 15:37:37 2014 +0530<br>
> @@ -44,6 +44,10 @@<br>
> int16_t mbufdct[TEST_BUF_SIZE];<br>
> int mbufidct[TEST_BUF_SIZE];<br>
><br>
> + ALIGN_VAR_32(uint32_t, mubuf1[MAX_TU_SIZE]);<br>
> + ALIGN_VAR_32(uint32_t, mubuf2[MAX_TU_SIZE]);<br>
> + ALIGN_VAR_32(uint16_t, mushortbuf1[MAX_TU_SIZE]);<br>
<br>
</div></div>>>does denoise need all new buffers? can it reuse existing buffers?<br>
I need unsigned buffers, so I prepared to attain new ones over interpreting sign buffer as unsign using type casting, the residuum of the things I have update in my patch.<br><br>
There's no need to declare them aligned here. The first array is<br>
declared aligned and since all below it are also aligned in size every<br>
array is implicitly aligned.<br>
<div><div class="h5"><br>
> int16_t mshortbuf2[MAX_TU_SIZE];<br>
> int16_t mshortbuf3[MAX_TU_SIZE];<br>
><br>
> @@ -56,6 +60,9 @@<br>
> int int_test_buff[TEST_CASES][TEST_BUF_SIZE];<br>
> int int_idct_test_buff[TEST_CASES][TEST_BUF_SIZE];<br>
><br>
> + int int_denoise_test_buff1[TEST_CASES][TEST_BUF_SIZE];<br>
> + int int_denoise_test_buff2[TEST_CASES][TEST_BUF_SIZE];<br>
> +<br>
> bool check_dequant_primitive(dequant_scaling_t ref, dequant_scaling_t opt);<br>
> bool check_dequant_primitive(dequant_normal_t ref, dequant_normal_t opt);<br>
> bool check_quant_primitive(quant_t ref, quant_t opt);<br>
> @@ -63,6 +70,7 @@<br>
> bool check_dct_primitive(dct_t ref, dct_t opt, intptr_t width);<br>
> bool check_idct_primitive(idct_t ref, idct_t opt, intptr_t width);<br>
> bool check_count_nonzero_primitive(count_nonzero_t ref, count_nonzero_t opt);<br>
> + bool check_denoise_dct_primitive(denoiseDct_t ref, denoiseDct_t opt);<br>
><br>
> public:<br>
><br>
> diff -r 184e56afa951 -r 36f5477f54ba source/test/testharness.h<br>
> --- a/source/test/testharness.h Fri Sep 12 12:02:46 2014 +0530<br>
> +++ b/source/test/testharness.h Mon Sep 15 15:37:37 2014 +0530<br>
> @@ -40,6 +40,7 @@<br>
> #define PIXEL_MIN 0<br>
> #define SHORT_MAX 32767<br>
> #define SHORT_MIN -32767<br>
> +#define UNSIGNED_SHORT_MAX 65535<br>
><br>
> using namespace x265;<br>
><br>
</div></div>> _______________________________________________<br>
> x265-devel mailing list<br>
> <a href="mailto:x265-devel@videolan.org">x265-devel@videolan.org</a><br>
> <a href="https://mailman.videolan.org/listinfo/x265-devel" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
<span class=""><font color="#888888"><br>
--<br>
Steve Borho<br>
_______________________________________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org">x265-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/x265-devel" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
</font></span></div><br></div>