[pulseaudio-discuss] [PATCH 2/4] Enabled libsoxr resampler backend.

[Andrey Semashev]

On Tuesday 13 January 2015 17:32:55 you wrote:
> Hello,
> 
> On Mon, 12 Jan 2015, Andrey Semashev wrote:
> 
> re-posting to list, hope that is OK
> 
> > On Monday 12 January 2015 17:36:19 you wrote:
> > > > Added ID and names for the resampler presets and also updated the
> > > > working
> > > > sample rate deduction to take the new resampler into account.
> > > 
> > > what to do with PA_RESAMPLER_VARIABLE_RATE?
> > > 
> > > resampler_soxr_update_rates() just recreates the context, I guess this
> > > will lead to crackling sound (not tested)
> > 
> > Not crackling, but the quality will degrade.
> 
> _update_rates() is used e.g. by module-rtp-recv to adjust the sample rate
> in tiny increments, i.e. while the stream is running, without stopping the
> stream
> 
> there is a flag SOXR_VR which may provide variable-rate resampling, but
> this is currently not used; not sure if it would work, the code is called
> experimental

Interesting, I forgot about that flag. I'll take a look at it. What 
immediately bothers me is that one is supposed to supply a ratio between 
input/output rates as a double. This might be imprecise, although I'm not sure 
how precise other resamplers are wrt this feature.

> > > probably SOXR should be added to the clause in fix_method() which checks
> > > for PA_RESAMPLER_VARIABLE_RATE?
> > 
> > Ok, but I see that by default it uses speex-float-1, which is
> > significantly
> > lower quality than any soxr mode. As I understand variable rate can be
> > used by applications transparently for the user, and there is no way to
> > influence this selection. Can I choose at least speex-float-5 as a
> > fallback? Ideally, this should be a configurable option, though.
> 
> I suggest do declare soxr to not support PA_RESAMPLER_VARIABLE_RATE in
> fix_method() for now and commit this work to -next (if there are no
> objections); fixups can be done later-on

I'm ok with that.

> one more comment; in
> int pa_resampler_soxr_init(pa_resampler *r) {
> ...
>         default:
>             pa_assert_not_reached();
>             return -1;
> the return statement can be removed

Right, I thought pa_assert_not_reached() would be empty in Release build.

> looking at choose_auto_resampler(), I agree that this could be a bit
> smarter; e.g.
> * if our default resampler (resample-method= in daemon.conf) is
> speex-fixed- or speex-float- with a higher quality, we could probably set
> that instead of speex-float-1

Yes, and in case of soxr we could fallback to speex-float-5.

> * if the default resampler is speex-fixed-, probably we should not
> auto-switch to speex-float-

The speex-float- is converted to the corresponding speex-fixed-, if needed, 
further down in fix_method().

> but this can be done as a separate patch in the future