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.wang at 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; >> } >> >
