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

[Alexander E. Patrakov]

13.05.2014 02:28, Peter Meerwald wrote:
> comments below

I will send a new version of the patch as a follow-up.

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

Will reword to "assumption".

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

Will add this too.

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

OK.

> maybe log it just once

That would be inconsistent with other pa_log_* calls that log their line 
each time PulseAudio doesn't like something about the resampler.

>> +static bool speex_is_fixed_point(void) {
>> +    static bool result;
>
> result = false?

Initialization to false is already guaranteed by the C99 standard. 
However, in other places in PulseAudio similar static bool variables are 
explicitly initialized to false, so I will do that too, for consistency.

>>   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].
>       */

I have combined your wording with some additional sentences.

     /* 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]. speex
      * without --enable-fixed-point works fine with this range.
      * Care has been taken to call speex_resample_float() only
      * for speex compiled without --enable-fixed-point.
      */

-- 
Alexander E. Patrakov