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

[Tanu Kaskinen]

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.

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

> But still you cannot get rid of the effective_source_latency variable, 
> it becomes relevant latest at the moment when we also have to take the offsets into 
> account.

I'm not sure about that, but that's a discussion for another thread (I
haven't yet read the patch in question).

(You didn't reply to my last mail in the "Handling of port latency
offsets" thread, so I'm not sure if you understood what I tried to say.
I tried to convince you that you shouldn't need special handling for
the latency offsets. But maybe you do need some special handling - I'll
find out when I get to that patch.)

-- 
Tanu

https://www.patreon.com/tanuk