[pulseaudio-discuss] [PATCH 02/21 v2] loopback: Initialize latency at startup and during source/sink changes

[Georg Chini]

On 27.02.2017 11:17, Tanu Kaskinen wrote:
> On Sat, 2017-02-25 at 22:23 +0100, Georg Chini wrote:
>>>> Sorry, slowly I am thinking that you don't want the patches at all and
>>>> are nitpicking to discourage me. If you don't want the patches, just say so.
>>> I assure you, I want these patches, especially this one, because this
>>> patch should fix two bugs that I know have caused people trouble (slow
>>> adjustment to target latency and accumulating a lot of latency when the
>>> sink is slow to start).
>>>
>>> I fully understand that the process is frustrating when we have trouble
>>> understanding each other. It's frustrating to me too. I wish I knew how
>>> to make it easier for us to reach common understanding.
>>>
>>> If you think that some criticism is excessive nitpicking, and you don't
>>> want to do the suggested change, you can say "I prefer doing it this
>>> way. If you want to change it, you can write a patch later yourself,
>>> ok?" That has been known to work sometimes.
>> Thanks.
>>
>>>> Thinking twice, there is not even a way you could get to that call
>>>> while the sink is suspended, so pa_sink_get_latency_in_thread() will
>>>> always return some value.
>>> Are you still referring to this code:
>>>
>>> +            /* If pop has not been called yet, make sure the latency does not grow too much.
>>> +             * Don't push any silence here, because we already have new data in the queue */
>>> +            if (!u->output_thread_info.pop_called)
>>> +                 memblockq_adjust(u, pa_sink_get_latency_within_thread(u->sink_input->sink), false);
>>>
>>> The only condition is that pop hasn't been called, and from that you
>>> certainly can't draw the conclusion that the sink is not suspended. I
>>> am again very confused.
>>>
>> You are right. I even needed a check for the sink being suspended when
>> detecting an underrun one line further down in the code ...
>> The call is more or less there for symmetry reasons similar to the the
>> SINK_CHANGED message (which I will remove). I can remove it as well
>> if you like. It should not have much effect.
> Which call do you mean? Do you mean the memblockq_adjust() call quoted
> above, the one that is called when pop hasn't been called? That
> adjustment seems useful, because it prevents the memblockq storing huge
> amounts of data if it takes a long time for the sink to start. I
> thought that was the reason for having the call, not just to have
> symmetry. But it's true that the call is not absolutely necessary.

I am not talking about the memblockq_adjust() call itself, but
of the pa_sink_get_latency_within_thread() call within the
memblockq_adjust() call, which is the one you complained about.

>
>> This also applies for the other call in sink_input_pop_cb().
> Do you mean that you can remove also the memblockq_adjust() call from
> pop_cb()? Yes, that one can safely be removed, if you generate silence
> in a different way than relying on the memblockq to produce it.
Uh, sorry, I mean removing the effective_source_latency variable
from the memblockq_adjust() call in sink_input_pop_cb()