10.05.2014 23:03, Peter Meerwald wrote: > http://lists.xiph.org/pipermail/speex-dev/2013-December/008463.html Well, that patch is bad, they forgot to scale the output. Thus the rejection is correct. > >>> +/* speexdsp has a nasty issue: when built with FIXED_POINT, the float >>> resampler >>> + * expects samples in [-32768,32768] instead of [-1,1]; there is no easy >>> way >>> + * to figure out how speexdsp has been compiled. */ In fact that's also true without FIXED_POINT, but doesn't matter then. > what's so bad about probing at runtime?? (if done correctly of course :) That was written under the wrong assumption that you try to work around a bug in speex. But there is actually no bug, just an incorrect API usage in PulseAudio, strictly speaking, even in the non-fixed-point case. Probing at runtime in order to optimize out two multiplications is fine. > another option would be avoid using speex resampling with floating point > sample format when compiled with fixed point, output a bit fat warning > to the PA logging, and switch back to speex resampling with s16 sample > format? > > but this requires runtime detection > > > I think scaling floating point samples for a library supposed to avoid > floting point operations is pretty pointless -- probably the user simply > wants to use speex-fixed resampling instead of speex-float but isn't > aware of it? Good thoughts. One possibly-stupid idea: measure whether speex-fixed vs speex-float matters on x86/amd64 given s16 input and speex compiled without FIXED_POINT. The reason I am asking is based on reading the speex source and some well-known facts: * outside of the JACK world, floating-point samples are uncommon; * with FIXED_POINT undefined, the first thing that speex_resampler_process_int() does is to fill in the "x" array, which is declared as "spx_word16_t *x", which expands to "float *x" (i.e. converts samples to float), and then proceeds to speex_resampler_process_native(); * with FIXED_POINT undefined, the first thing that speex_resampler_process_float() does is to fill in the "x" array, which is declared as "spx_word16_t *x", which expands to "float *x" (i.e. copies samples), and then proceeds to speex_resampler_process_native(); * when PulseAudio starts from s16 samples and decides to use the speex-float resampler via speex_resampler_process_float(), it has to convert s16 to float first anyway; * it should not matter who converts s16 to float and back - speex or PulseAudio. So the proposal is: after a benchmark, kill the speex-float family of resamplers and make it an alias of speex-fixed on all platforms (not just on ARM) for compatibility. -- Alexander E. Patrakov
