> > 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)
