Re: [pulseaudio-discuss] [PATCH] tests: add tolerant variation for comparing the rewind result
On 2015-05-25 06:49, Hui Wang wrote:
On 32bits OS, this test case fails. The reason is when rewinding to
the middle of a block, some of float parameters in the saved_state
are stored in the memory from FPU registers, and those parameters will
be used for next time to process data with lfe. Here if FPU register
is over 32bits, the storing from FPU register to memory will introduce
some variation, and this small variation will introduce small
variation to the rewinding result.
Very interesting finding. I didn't know that storing things back and
forth to memory could change the computation result.
And the fact that it only happens on 32-bit platforms and only with
optimisations makes it even stranger. Makes me wonder if this is
actually an gcc optimisation bug.
So adding the tolerant variation for comparing the rewind result, make
this test case can work on both 64bits OS and 32bits OS.
Signed-off-by: Hui Wang hui.w...@canonical.com
---
I wrote a simple testcase to show the variation exists on 32bits OS.
When compile this test case on 64bits OS, it will not fail when running
it; while on 32bits OS if you just compile it without -O2, this
testcase still pass without any variation, but if you add -O2 when
compiling it, you will see variation when you running it.
http://pastebin.ubuntu.com/11342537/
src/tests/lfe-filter-test.c | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/tests/lfe-filter-test.c b/src/tests/lfe-filter-test.c
index 2c6d597..50636a9 100644
--- a/src/tests/lfe-filter-test.c
+++ b/src/tests/lfe-filter-test.c
@@ -37,6 +37,7 @@ static uint8_t *ori_sample_ptr;
#define ONE_BLOCK_SAMPLES 4096
#define TOTAL_SAMPLES 8192
+#define TOLERANT_VARIATION 1
static void save_data_block(struct lfe_filter_test *lft, void *d, pa_memblock
*blk) {
uint8_t *dst = d, *src;
@@ -63,15 +64,26 @@ static pa_memblock* generate_data_block(struct
lfe_filter_test *lft, int start)
static int compare_data_block(struct lfe_filter_test *lft, void *a, void *b) {
int ret = 0;
uint32_t i;
-uint32_t fz = pa_frame_size(lft-ss);
-uint8_t *r = a, *u = b;
-for (i = 0; i ONE_BLOCK_SAMPLES * fz; i++) {
-if (*r++ != *u++) {
-pa_log_error(lfe-filter-test: test failed, the output data in the
position 0x%x of a block does not equal!\n, i);
-ret = -1;
+switch (lft-ss-format) {
+case PA_SAMPLE_S16NE:
+case PA_SAMPLE_S16RE: {
Do we need to support PA_SAMPLE_S16RE? If not, then just replace with
assert(PA_SAMPLE_S16NE == lft-ss-format).
If you need S16RE, then you need to swap the bytes before comparing.
+uint16_t *r = a, *u = b;
+for (i = 0; i ONE_BLOCK_SAMPLES; i++) {
+uint16_t va = *r++, vb = *u++;
+uint16_t var = (va = vb) ? (va - vb) : (vb - va);
Agree with Alexander, use abs() here.
+if (var TOLERANT_VARIATION) {
+pa_log_error(lfe-filter-test: test failed, the output data in
the position 0x%x of a block does not equal!\n, i);
+ret = -1;
+break;
+}
+}
break;
}
+default:
+pa_log_error(lfe-filter-test: not a suppported sample format yet in
this testcase!\n);
+ret = -1;
+break;
}
return ret;
}
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tests: add tolerant variation for comparing the rewind result
25.05.2015 09:49, Hui Wang wrote:
On 32bits OS, this test case fails. The reason is when rewinding to
the middle of a block, some of float parameters in the saved_state
are stored in the memory from FPU registers, and those parameters will
be used for next time to process data with lfe. Here if FPU register
is over 32bits, the storing from FPU register to memory will introduce
some variation, and this small variation will introduce small
variation to the rewinding result.
So adding the tolerant variation for comparing the rewind result, make
this test case can work on both 64bits OS and 32bits OS.
Signed-off-by: Hui Wang hui.w...@canonical.com
ACK in general. But I'd remove a switch and replace it with a simple
assertion that the format matches what we expect. And I don't like an
open-coded abs().
---
I wrote a simple testcase to show the variation exists on 32bits OS.
When compile this test case on 64bits OS, it will not fail when running
it; while on 32bits OS if you just compile it without -O2, this
testcase still pass without any variation, but if you add -O2 when
compiling it, you will see variation when you running it.
http://pastebin.ubuntu.com/11342537/
src/tests/lfe-filter-test.c | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/tests/lfe-filter-test.c b/src/tests/lfe-filter-test.c
index 2c6d597..50636a9 100644
--- a/src/tests/lfe-filter-test.c
+++ b/src/tests/lfe-filter-test.c
@@ -37,6 +37,7 @@ static uint8_t *ori_sample_ptr;
#define ONE_BLOCK_SAMPLES 4096
#define TOTAL_SAMPLES 8192
+#define TOLERANT_VARIATION 1
static void save_data_block(struct lfe_filter_test *lft, void *d, pa_memblock
*blk) {
uint8_t *dst = d, *src;
@@ -63,15 +64,26 @@ static pa_memblock* generate_data_block(struct
lfe_filter_test *lft, int start)
static int compare_data_block(struct lfe_filter_test *lft, void *a, void *b) {
int ret = 0;
uint32_t i;
-uint32_t fz = pa_frame_size(lft-ss);
-uint8_t *r = a, *u = b;
-for (i = 0; i ONE_BLOCK_SAMPLES * fz; i++) {
-if (*r++ != *u++) {
-pa_log_error(lfe-filter-test: test failed, the output data in the
position 0x%x of a block does not equal!\n, i);
-ret = -1;
+switch (lft-ss-format) {
+case PA_SAMPLE_S16NE:
+case PA_SAMPLE_S16RE: {
+uint16_t *r = a, *u = b;
+for (i = 0; i ONE_BLOCK_SAMPLES; i++) {
+uint16_t va = *r++, vb = *u++;
+uint16_t var = (va = vb) ? (va - vb) : (vb - va);
+if (var TOLERANT_VARIATION) {
+pa_log_error(lfe-filter-test: test failed, the output data in
the position 0x%x of a block does not equal!\n, i);
+ret = -1;
+break;
+}
+}
break;
}
+default:
+pa_log_error(lfe-filter-test: not a suppported sample format yet in
this testcase!\n);
+ret = -1;
+break;
}
return ret;
}
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tests: add tolerant variation for comparing the rewind result
On 2015年05月25日 14:40, David Henningsson wrote:
On 2015-05-25 06:49, Hui Wang wrote:
On 32bits OS, this test case fails. The reason is when rewinding to
the middle of a block, some of float parameters in the saved_state
are stored in the memory from FPU registers, and those parameters will
be used for next time to process data with lfe. Here if FPU register
is over 32bits, the storing from FPU register to memory will introduce
some variation, and this small variation will introduce small
variation to the rewinding result.
Very interesting finding. I didn't know that storing things back and
forth to memory could change the computation result.
And the fact that it only happens on 32-bit platforms and only with
optimisations makes it even stranger. Makes me wonder if this is
actually an gcc optimisation bug.
Probably.
So adding the tolerant variation for comparing the rewind result, make
this test case can work on both 64bits OS and 32bits OS.
Signed-off-by: Hui Wang hui.w...@canonical.com
---
I wrote a simple testcase to show the variation exists on 32bits OS.
When compile this test case on 64bits OS, it will not fail when running
it; while on 32bits OS if you just compile it without -O2, this
testcase still pass without any variation, but if you add -O2 when
compiling it, you will see variation when you running it.
http://pastebin.ubuntu.com/11342537/
src/tests/lfe-filter-test.c | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/tests/lfe-filter-test.c b/src/tests/lfe-filter-test.c
index 2c6d597..50636a9 100644
--- a/src/tests/lfe-filter-test.c
+++ b/src/tests/lfe-filter-test.c
@@ -37,6 +37,7 @@ static uint8_t *ori_sample_ptr;
#define ONE_BLOCK_SAMPLES 4096
#define TOTAL_SAMPLES 8192
+#define TOLERANT_VARIATION 1
static void save_data_block(struct lfe_filter_test *lft, void *d,
pa_memblock *blk) {
uint8_t *dst = d, *src;
@@ -63,15 +64,26 @@ static pa_memblock* generate_data_block(struct
lfe_filter_test *lft, int start)
static int compare_data_block(struct lfe_filter_test *lft, void *a,
void *b) {
int ret = 0;
uint32_t i;
-uint32_t fz = pa_frame_size(lft-ss);
-uint8_t *r = a, *u = b;
-for (i = 0; i ONE_BLOCK_SAMPLES * fz; i++) {
-if (*r++ != *u++) {
-pa_log_error(lfe-filter-test: test failed, the output
data in the position 0x%x of a block does not equal!\n, i);
-ret = -1;
+switch (lft-ss-format) {
+case PA_SAMPLE_S16NE:
+case PA_SAMPLE_S16RE: {
Do we need to support PA_SAMPLE_S16RE? If not, then just replace with
assert(PA_SAMPLE_S16NE == lft-ss-format).
If you need S16RE, then you need to swap the bytes before comparing.
Don't want to support S16RE, will change to assert() in the V2.
+uint16_t *r = a, *u = b;
+for (i = 0; i ONE_BLOCK_SAMPLES; i++) {
+uint16_t va = *r++, vb = *u++;
+uint16_t var = (va = vb) ? (va - vb) : (vb - va);
Agree with Alexander, use abs() here.
Got it, will fix it in the V2. Thanks.
+if (var TOLERANT_VARIATION) {
+pa_log_error(lfe-filter-test: test failed, the
output data in the position 0x%x of a block does not equal!\n, i);
+ret = -1;
+break;
+}
+}
break;
}
+default:
+pa_log_error(lfe-filter-test: not a suppported sample
format yet in this testcase!\n);
+ret = -1;
+break;
}
return ret;
}
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tests: add tolerant variation for comparing the rewind result
On 2015年05月25日 14:10, Alexander E. Patrakov wrote:
25.05.2015 09:49, Hui Wang wrote:
On 32bits OS, this test case fails. The reason is when rewinding to
the middle of a block, some of float parameters in the saved_state
are stored in the memory from FPU registers, and those parameters will
be used for next time to process data with lfe. Here if FPU register
is over 32bits, the storing from FPU register to memory will introduce
some variation, and this small variation will introduce small
variation to the rewinding result.
So adding the tolerant variation for comparing the rewind result, make
this test case can work on both 64bits OS and 32bits OS.
Signed-off-by: Hui Wang hui.w...@canonical.com
ACK in general. But I'd remove a switch and replace it with a simple
assertion that the format matches what we expect. And I don't like an
open-coded abs().
Got it, will fix them in the V2. Thanks.
---
I wrote a simple testcase to show the variation exists on 32bits OS.
When compile this test case on 64bits OS, it will not fail when running
it; while on 32bits OS if you just compile it without -O2, this
testcase still pass without any variation, but if you add -O2 when
compiling it, you will see variation when you running it.
http://pastebin.ubuntu.com/11342537/
src/tests/lfe-filter-test.c | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/tests/lfe-filter-test.c b/src/tests/lfe-filter-test.c
index 2c6d597..50636a9 100644
--- a/src/tests/lfe-filter-test.c
+++ b/src/tests/lfe-filter-test.c
@@ -37,6 +37,7 @@ static uint8_t *ori_sample_ptr;
#define ONE_BLOCK_SAMPLES 4096
#define TOTAL_SAMPLES 8192
+#define TOLERANT_VARIATION 1
static void save_data_block(struct lfe_filter_test *lft, void *d,
pa_memblock *blk) {
uint8_t *dst = d, *src;
@@ -63,15 +64,26 @@ static pa_memblock* generate_data_block(struct
lfe_filter_test *lft, int start)
static int compare_data_block(struct lfe_filter_test *lft, void *a,
void *b) {
int ret = 0;
uint32_t i;
-uint32_t fz = pa_frame_size(lft-ss);
-uint8_t *r = a, *u = b;
-for (i = 0; i ONE_BLOCK_SAMPLES * fz; i++) {
-if (*r++ != *u++) {
-pa_log_error(lfe-filter-test: test failed, the output
data in the position 0x%x of a block does not equal!\n, i);
-ret = -1;
+switch (lft-ss-format) {
+case PA_SAMPLE_S16NE:
+case PA_SAMPLE_S16RE: {
+uint16_t *r = a, *u = b;
+for (i = 0; i ONE_BLOCK_SAMPLES; i++) {
+uint16_t va = *r++, vb = *u++;
+uint16_t var = (va = vb) ? (va - vb) : (vb - va);
+if (var TOLERANT_VARIATION) {
+pa_log_error(lfe-filter-test: test failed, the
output data in the position 0x%x of a block does not equal!\n, i);
+ret = -1;
+break;
+}
+}
break;
}
+default:
+pa_log_error(lfe-filter-test: not a suppported sample
format yet in this testcase!\n);
+ret = -1;
+break;
}
return ret;
}
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tests: add tolerant variation for comparing the rewind result
On 25 May 2015 at 12:49, hwang4 hui.w...@canonical.com wrote: On 2015年05月25日 14:40, David Henningsson wrote: On 2015-05-25 06:49, Hui Wang wrote: On 32bits OS, this test case fails. The reason is when rewinding to the middle of a block, some of float parameters in the saved_state are stored in the memory from FPU registers, and those parameters will be used for next time to process data with lfe. Here if FPU register is over 32bits, the storing from FPU register to memory will introduce some variation, and this small variation will introduce small variation to the rewinding result. Very interesting finding. I didn't know that storing things back and forth to memory could change the computation result. And the fact that it only happens on 32-bit platforms and only with optimisations makes it even stranger. Makes me wonder if this is actually an gcc optimisation bug. Probably. Did I misunderstand something here? Fixing a problem with the compiler in our test seems to be incorrect. -- Arun ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tests: add tolerant variation for comparing the rewind result
On 2015-05-25 14:57, Arun Raghavan wrote: On 25 May 2015 at 12:49, hwang4 hui.w...@canonical.com wrote: On 2015年05月25日 14:40, David Henningsson wrote: On 2015-05-25 06:49, Hui Wang wrote: On 32bits OS, this test case fails. The reason is when rewinding to the middle of a block, some of float parameters in the saved_state are stored in the memory from FPU registers, and those parameters will be used for next time to process data with lfe. Here if FPU register is over 32bits, the storing from FPU register to memory will introduce some variation, and this small variation will introduce small variation to the rewinding result. Very interesting finding. I didn't know that storing things back and forth to memory could change the computation result. And the fact that it only happens on 32-bit platforms and only with optimisations makes it even stranger. Makes me wonder if this is actually an gcc optimisation bug. Probably. Did I misunderstand something here? Fixing a problem with the compiler in our test seems to be incorrect. My assumption is that as long as we have -ffast-math on, then these are things we have to live with (and thus calling it a gcc optimisation bug is inaccurate, sorry about that). Being pragmatic: since the difference is one bit only, no one will hear any difference. But if you feel like digging into it further, I'm curious to know the result. :-) -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] tests: add tolerant variation for comparing the rewind result
On 32bits OS, this test case fails. The reason is when rewinding to
the middle of a block, some of float parameters in the saved_state
are stored in the memory from FPU registers, and those parameters will
be used for next time to process data with lfe. Here if FPU register
is over 32bits, the storing from FPU register to memory will introduce
some variation, and this small variation will introduce small
variation to the rewinding result.
So adding the tolerant variation for comparing the rewind result, make
this test case can work on both 64bits OS and 32bits OS.
Signed-off-by: Hui Wang hui.w...@canonical.com
---
I wrote a simple testcase to show the variation exists on 32bits OS.
When compile this test case on 64bits OS, it will not fail when running
it; while on 32bits OS if you just compile it without -O2, this
testcase still pass without any variation, but if you add -O2 when
compiling it, you will see variation when you running it.
http://pastebin.ubuntu.com/11342537/
src/tests/lfe-filter-test.c | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/tests/lfe-filter-test.c b/src/tests/lfe-filter-test.c
index 2c6d597..50636a9 100644
--- a/src/tests/lfe-filter-test.c
+++ b/src/tests/lfe-filter-test.c
@@ -37,6 +37,7 @@ static uint8_t *ori_sample_ptr;
#define ONE_BLOCK_SAMPLES 4096
#define TOTAL_SAMPLES 8192
+#define TOLERANT_VARIATION 1
static void save_data_block(struct lfe_filter_test *lft, void *d, pa_memblock
*blk) {
uint8_t *dst = d, *src;
@@ -63,15 +64,26 @@ static pa_memblock* generate_data_block(struct
lfe_filter_test *lft, int start)
static int compare_data_block(struct lfe_filter_test *lft, void *a, void *b) {
int ret = 0;
uint32_t i;
-uint32_t fz = pa_frame_size(lft-ss);
-uint8_t *r = a, *u = b;
-for (i = 0; i ONE_BLOCK_SAMPLES * fz; i++) {
-if (*r++ != *u++) {
-pa_log_error(lfe-filter-test: test failed, the output data in the
position 0x%x of a block does not equal!\n, i);
-ret = -1;
+switch (lft-ss-format) {
+case PA_SAMPLE_S16NE:
+case PA_SAMPLE_S16RE: {
+uint16_t *r = a, *u = b;
+for (i = 0; i ONE_BLOCK_SAMPLES; i++) {
+uint16_t va = *r++, vb = *u++;
+uint16_t var = (va = vb) ? (va - vb) : (vb - va);
+if (var TOLERANT_VARIATION) {
+pa_log_error(lfe-filter-test: test failed, the output
data in the position 0x%x of a block does not equal!\n, i);
+ret = -1;
+break;
+}
+}
break;
}
+default:
+pa_log_error(lfe-filter-test: not a suppported sample format yet
in this testcase!\n);
+ret = -1;
+break;
}
return ret;
}
--
1.9.1
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
