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
