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

[David Henningsson]

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.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.

> +            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