[pulseaudio-discuss] [PATCH] resampler: Support speex resampler compiled with FIXED_POINT

[Alexander E. Patrakov]

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