diff options
author | Peter Meerwald <pmeerw@pmeerw.net> | 2014-09-02 23:53:09 +0200 |
---|---|---|
committer | Peter Meerwald <pmeerw@pmeerw.net> | 2014-09-04 09:58:20 +0200 |
commit | 293a1739e25707c25321d5bc6cd3e2e8aa96a910 (patch) | |
tree | 804e45dc71c8db4c18fca3143bdc08f180293e08 /src/tests | |
parent | 5d7b5e509ce3a9a5c17f3ee3cec2eee2979e190f (diff) |
endianmacros: Replace borked PA_FLOAT32_SWAP() with PA_READ_FLOAT32RE() / PA_WRITE_FLOAT32RE()
building PA with -O0 leads to test failure in mix-test on i386
issue reported by Felipe, see
http://lists.freedesktop.org/archives/pulseaudio-discuss/2014-August/021406.html
the problem is the value 0xbeffbd7f: when byte-swapped it becomes 0x7fbdffbe and according
to IEEE-754 represents a signalling NaN (starting with s111 1111 10, see http://en.wikipedia.org/wiki/NaN)
when this value is assigned to a floating point register, it becomes 0x7ffdffbe, representing
a quiet NaN (starting with s111 1111 11) -- a signalling NaN is turned into a quiet NaN!
so PA_FLOAT32_SWAP(PA_FLOAT32_SWAP(x)) != x for certain values, uhuh!
the following test code can be used; due to volatile, it will always demonstrate the issue;
without volatile, it depends on the optimization level (i386, 32-bit, gcc 4.9):
// snip
static inline float PA_FLOAT32_SWAP(float x) {
union {
float f;
uint32_t u;
} t;
t.f = x;
t.u = bswap_32(t.u);
return t.f;
}
int main() {
unsigned x = 0xbeffbd7f;
volatile float f = PA_FLOAT32_SWAP(*(float *)&x);
printf("%08x %08x %08x %f\n", 0xbeffbd7f, *(unsigned *)&f, bswap_32(*(unsigned *)&f), f);
}
// snip
the problem goes away with optimization when no temporary floating point registers are used
the proposed solution is to avoid passing swapped floating point data in a
float; this is done with new functions PA_READ_FLOAT32RE() and PA_WRITE_FLOAT32RE()
which use uint32_t to dereference a pointer and byte-swap the data, hence no temporary
float variable is used
also delete PA_FLOAT32_TO_LE()/_BE(), not used
Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Reported-by: Felipe Sateler <fsateler@debian.org>
Diffstat (limited to 'src/tests')
-rw-r--r-- | src/tests/mix-test.c | 4 | ||||
-rw-r--r-- | src/tests/resampler-test.c | 4 |
2 files changed, 4 insertions, 4 deletions
diff --git a/src/tests/mix-test.c b/src/tests/mix-test.c index d5bae13ed..e95607dee 100644 --- a/src/tests/mix-test.c +++ b/src/tests/mix-test.c @@ -150,7 +150,7 @@ static void compare_block(const pa_sample_spec *ss, const pa_memchunk *chunk, in float *u = d; for (i = 0; i < chunk->length / pa_frame_size(ss); i++) { - float uu = PA_MAYBE_FLOAT32_SWAP(ss->format != PA_SAMPLE_FLOAT32NE, *u); + float uu = ss->format == PA_SAMPLE_FLOAT32NE ? *u : PA_READ_FLOAT32RE(u); fail_unless(fabsf(uu - *v) <= 1e-6f, NULL); ++u; ++v; @@ -264,7 +264,7 @@ static pa_memblock* generate_block(pa_mempool *pool, const pa_sample_spec *ss) { if (ss->format == PA_SAMPLE_FLOAT32RE) { float *u = d; for (i = 0; i < 10; i++) - u[i] = PA_FLOAT32_SWAP(float32ne_result[0][i]); + PA_WRITE_FLOAT32RE(&u[i], float32ne_result[0][i]); } else memcpy(d, float32ne_result[0], sizeof(float32ne_result[0])); diff --git a/src/tests/resampler-test.c b/src/tests/resampler-test.c index 60345afb3..f547770b8 100644 --- a/src/tests/resampler-test.c +++ b/src/tests/resampler-test.c @@ -98,7 +98,7 @@ static void dump_block(const char *label, const pa_sample_spec *ss, const pa_mem float *u = d; for (i = 0; i < chunk->length / pa_frame_size(ss); i++) { - printf("%4.3g ", ss->format == PA_SAMPLE_FLOAT32NE ? *u : PA_FLOAT32_SWAP(*u)); + printf("%4.3g ", ss->format == PA_SAMPLE_FLOAT32NE ? *u : PA_READ_FLOAT32RE(u)); u++; } @@ -222,7 +222,7 @@ static pa_memblock* generate_block(pa_mempool *pool, const pa_sample_spec *ss) { if (ss->format == PA_SAMPLE_FLOAT32RE) for (i = 0; i < 10; i++) - u[i] = PA_FLOAT32_SWAP(u[i]); + PA_WRITE_FLOAT32RE(&u[i], u[i]); break; } |