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

[Peter Meerwald]

Hi,

> speex_resample_float() does not work with speex compiled with
> --enable-fixed-point, because speex expects its float input
> to be normalized to ±32768 instead of the more usual ±1.
> 
> It is possible to fix speex_resample_float(), as demonstrated at
> http://lists.freedesktop.org/archives/pulseaudio-discuss/2014-May/020617.html
> However, a better idea is to avoid using the speex-float resampler and
> the associated s16 <-> float conversions that speex will immediately undo
> internally if it is known that speex has been compiled with FIXED_POINT.
> So, transparently change speex-float-* to speex-fixed-* in that case.
> 
> Signed-off-by: Alexander E. Patrakov <patrakov at gmail.com>
> Reported-by: Fahad Arslan <fahad_arslan at mentor.com>
> Cc: Damir Jelić <poljarinho at gmail.com>
> Cc: Peter Meerwald <pmeerw at pmeerw.net>

comments below
thank you for following up on this issue
> 
> FIXED_POINT detection is based on code by Peter Meerwald.
> ---
>  src/pulsecore/resampler.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 54 insertions(+), 1 deletion(-)
> 
> diff --git a/src/pulsecore/resampler.c b/src/pulsecore/resampler.c
> index 1153281..0e3c7da 100644
> --- a/src/pulsecore/resampler.c
> +++ b/src/pulsecore/resampler.c
> @@ -39,6 +39,7 @@
>  #include <pulsecore/macro.h>
>  #include <pulsecore/strbuf.h>
>  #include <pulsecore/remap.h>
> +#include <pulsecore/once.h>
>  #include <pulsecore/core-util.h>
>  #include "ffmpeg/avcodec.h"
>  
> @@ -185,6 +186,8 @@ static int (* const init_table[])(pa_resampler*r) = {
>      [PA_RESAMPLER_PEAKS]                   = peaks_init,
>  };
>  
> +static bool speex_is_fixed_point(void);
> +
>  static pa_resample_method_t choose_auto_resampler(pa_resample_flags_t flags) {
>      pa_resample_method_t method;
>  
> @@ -250,6 +253,19 @@ static pa_resample_method_t pa_resampler_fix_method(
>      if (method == PA_RESAMPLER_AUTO)
>          method = choose_auto_resampler(flags);
>  
> +    /* At this point, method is supported in the sense that it
> +     * has an init function and supports the required flags. However,
> +     * speex-float implementation in PulseAudio relies on the range
> +     * optimization that is invalid if speex has been compiled with
> +     * --enable-fixed-point. Fix this up.
> +     */

I am not sure if 'range optimization' can be understood without more 
background info

maybe the better argument is that speex-float with fixed point speex is 
not a good idea

> +    if (method >= PA_RESAMPLER_SPEEX_FLOAT_BASE && method <= PA_RESAMPLER_SPEEX_FLOAT_MAX) {
> +        if (speex_is_fixed_point()) {
> +	    pa_log_info("Speex appears to be compiled with --enable-fixed-point.");
> +	    pa_log_info("Switching to a fixed-point resampler because it should be faster.");

one line of logging would be enough I think;
maybe log it just once

> +	    method = method - PA_RESAMPLER_SPEEX_FLOAT_BASE + PA_RESAMPLER_SPEEX_FIXED_BASE;
> +	}
> +    }
>      return method;
>  }
>  
> @@ -1469,9 +1485,37 @@ static int libsamplerate_init(pa_resampler *r) {
>  }
>  #endif
>  
> -#ifdef HAVE_SPEEX
>  /*** speex based implementation ***/
>  
> +static bool speex_is_fixed_point(void) {
> +    static bool result;

result = false?

> +#ifdef HAVE_SPEEX
> +    PA_ONCE_BEGIN {
> +        float f_out = -1.0f, f_in = 1.0f;
> +        spx_uint32_t in_len = 1, out_len = 1;
> +        SpeexResamplerState *s;
> +
> +        pa_assert_se(s = speex_resampler_init(1, 1, 1,
> +            SPEEX_RESAMPLER_QUALITY_MIN, NULL));
> +
> +        /* feed one sample that is too soft for fixed-point speex */
> +        pa_assert_se(speex_resampler_process_float(s, 0, &f_in, &in_len,
> +            &f_out, &out_len) == RESAMPLER_ERR_SUCCESS);
> +
> +        /* expecting sample has been processed, one sample output */
> +        pa_assert_se(in_len == 1 && out_len == 1);
> +
> +        /* speex compiled with --enable-fixed-point will output 0.0 due to insufficient precision */
> +        if (fabsf(f_out) < 0.00001f)
> +            result = true;
> +
> +        speex_resampler_destroy(s);
> +    } PA_ONCE_END;
> +#endif
> +    return result;
> +}
> +
> +#ifdef HAVE_SPEEX
>  static unsigned speex_resample_float(pa_resampler *r, const pa_memchunk *input, unsigned in_n_frames, pa_memchunk *output, unsigned *out_n_frames) {
>      float *in, *out;
>      uint32_t inf = in_n_frames, outf = *out_n_frames;
> @@ -1487,6 +1531,15 @@ static unsigned speex_resample_float(pa_resampler *r, const pa_memchunk *input,
>      in = pa_memblock_acquire_chunk(input);
>      out = pa_memblock_acquire_chunk(output);
>  
> +    /* Strictly speaking, speex resampler expects its input
> +     * to be normalized to the [-32768.0 .. 32767.0] range.
> +     * This matters if speex has been compiled with --enable-fixed-point,
> +     * because such speex will round the samples to the nearest
> +     * integer, which is 0, -1 or 1 for the floating-point sample
> +     * range which is used in PulseAudio. However, with fixed-point
> +     * speex, this line is not reached, and with normal speex,

which line?

> +     * the traditional [-1.0 .. 1.0] range works fine.
> +     */

maybe better:

    /* Strictly speaking, speex resampler expects its input
     * to be normalized to the [-32768.0 .. 32767.0] range.
     * This matters if speex has been compiled with --enable-fixed-point,
     * because such speex will round the samples to the nearest
     * integer. speex with --enable-fixed-point is therefore incompatible
     * with PulseAudio's floating-point sample range [-1 .. 1].
     */

>      pa_assert_se(speex_resampler_process_interleaved_float(state, in, &inf, out, &outf) == 0);
>  
>      pa_memblock_release(input->memblock);
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)