[pulseaudio-discuss] [PATCH] tests: add tolerant variation for comparing the rewind result

[hwang4]

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;
>>   }
>>
>