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

[Peter Meerwald]

> > A patch to fix speex was rejected saying that speex API is stable.
> > 
> > Further discussion is here
> > http://lists.openembedded.org/pipermail/openembedded-core/2014-January/087886.html
> 
> I don't see any pointer to the speex list, could you please add one to a
> comment below?

http://lists.xiph.org/pipermail/speex-dev/2013-December/008463.html

> > +/* 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. */
> > +
> > +static unsigned (*speex_resampler_float)(pa_resampler *r, const pa_memchunk
> > *input, unsigned in_n_frames, pa_memchunk *output, unsigned *out_n_frames) =
> > speex_resample_float;
> > +
> > +static void speex_probe_fixed_point() {
> > +    PA_ONCE_BEGIN {
> > +        float f_out = -1.0f, f_in = 1.0f;
> > +        spx_uint32_t in_len = 1, out_len = 1;
> > +
> > +        SpeexResamplerState *s = speex_resampler_init(1, 1, 1,
> > +            SPEEX_RESAMPLER_QUALITY_MIN, NULL);
> > +        if (!s)
> > +            goto done;
> > +
> > +        /* feed one sample */
> > +        if (speex_resampler_process_float(s, 0, &f_in, &in_len,
> > +            &f_out, &out_len) != RESAMPLER_ERR_SUCCESS)
> > +            goto done;
> > +
> > +        /* expecting sample has been processed, one sample output */
> > +        if (in_len != 1 && out_len != 1)
> > +            goto done;
> > +
> > +        /* FIXED_POINT implementation will output 0.0 */
> > +        if (fabsf(f_out) < 0.00001f)
> > +            speex_resampler_float = speex_resample_float_fixed;
> > +
> > +done:
> > +        if (s)
> > +            speex_resampler_destroy(s);
> > +    } PA_ONCE_END;
> > +}
> 
> I'd say that this function either should not be void or should abort on
> speex_resampler_init() failure. Right now, you just hide this failure.

the idea was to try and if something unexpected happens, just continue as 
before
 
> My overall opinion on this patch is mixed. If the right solution is to work
> around this speex bug and if my notes above are taken into account, then the
> patch is fine. However, there are other choices that need to be considered
> before applying this patch:
> 
>  * Poking speex guys again - their stable API is unusable.

a couple of trivial patches were accepted recently; so maybe we can have 
an #define in the speex resampler header file that indicates how speex was 
compiled

>  * A compile-time check (with a way to provide an answer if cross-compiling)
> that just fails on speex compiled with --enable-fixed-point.

that sounds difficult...
one someone could still swap the speexdsp.so after building

>  * Documentation of the requirement that speex must not be compiled with
> --enable-fixed-point, with a clear focus on the fact that it is a bug in
> speex.

>  * Putting a copy of the speex resampler into PulseAudio source tree and
> fixing it locally, with appropriate warnings to the distributors. Possible
> problem: conflict with the external speex echo canceller.

> I defer the choice between the original patch and these alternatives to
> someone else.

thanks for bringing up all the options 

what's so bad about probing at runtime?? (if done correctly of course :)

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?

regards, p.
-- 

Peter Meerwald
+43-664-2444418 (mobile)