[pulseaudio-discuss] [PATCH 12/13] loopback: Validate the rate parameter

[Georg Chini]

On 18.12.2015 09:14, Alexander E. Patrakov wrote:
> 18.12.2015 13:04, Georg Chini wrote:
>> On 18.12.2015 08:45, Alexander E. Patrakov wrote:
>>> 18.12.2015 11:47, Georg Chini wrote:
>>>> On 18.12.2015 06:49, Tanu Kaskinen wrote:
>>>>>
>>>>>
>>>>> Making pa_sample_rate_valid() accept values above PA_RATE_MAX isn't
>>>>> very nice, but I can see how it's better than the current behaviour,
>>>>> which probably can cause the daemon to crash. Alexander mentioned in
>>>>> IRC that he'd prefer doing the adaptive resampling inside module-
>>>>> loopback rather than using pa_sink_input_set_rate(), and this change
>>>>> demonstrates one good reason for that. I'm not asking you to 
>>>>> implement
>>>>> that change, but it certainly would be nice.
>>>>>
>>>>> -- 
>>>>> Tanu
>>>> Yes, doing it inside the module would really be a good idea. The
>>>> main issue my controller is fighting against are the systematic
>>>> errors introduced by changing the rate of the sink input. When
>>>> you change the rate, the next latency report has an extremely
>>>> high error.
>>>> Can you point me to some code that implements resampling
>>>> so that I have an example of what to do?
>>>
>>> Look at the ffmpeg resampler inside PulseAudio tree, it's the simplest
>>> possible one. But I'd say that it's pointless to look at the code
>>> without understanding the math behind all of that, so please look at
>>> these links, too:
>>>
>>> https://en.wikipedia.org/wiki/Sinc_filter
>>> https://ccrma.stanford.edu/~jos/resample/resample.pdf
>>>
>> I would first use the existing resamplers before trying to implement my
>> own.
>> So I was rather referring to the framework used in pulseaudio when using
>> existing resamplers. If I ever write a specific resampler for the
>> loopback module
>> it could be integrated into the normal framework.
>
> And that's exactly what I am against. Neither the normal framework nor 
> any of the existing (even outside PulseAudio) non-GPL resamplers 
> support sub-sample latency reporting. Please write your own resampler, 
> that's what I meant on IRC. I know that in a similar email to you I 
> also said "or treat the problem as unsolvable", this still stands, and 
> Tanu's "would be nice" also allows that.
>
I do not think that we need sub-sample latency reporting. You can see 
from the
results of your last tests that the main problem are the errors 
introduced by
using pa_sink_input_set_rate(). If you change the rate too often, 
systematic errors
are accumulating to a point where the resulting latency reports are 
nearly unusable
(your 100 usec threshold result). Those errors are not sub-sample but in 
the range
of a few hundred usec.
So if I could avoid changing the sink input rate it would help a lot. 
Using the resampler
within the module would also give me the possibility to use fractal 
resampling rates
(at least with speex, I did not look at the other resamplers yet), which 
would remove
the sawtooth we cannot avoid now.