[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:09 +0100, Georg Chini wrote:
> On 25.02.2017 20:29, Tanu Kaskinen wrote:
> > On Sat, 2017-02-25 at 19:21 +0200, Tanu Kaskinen wrote:
> > > On Sat, 2017-02-25 at 14:01 +0100, Georg Chini wrote:
> > > > On 25.02.2017 13:51, Tanu Kaskinen wrote:
> > > > > The second adjustment does the right thing regardless of whether the
> > > > > first adjustment was done or not, so why not skip the first adjustment
> > > > > altogether?
> > > > 
> > > > Because it might be the case that we are only switching the source OR the
> > > > sink and if we are changing both or starting up, we don't know which will
> > > > come first.
> > > 
> > > So you're saying that skipping the first adjustment as I suggested
> > > can't be done? I think this should do the trick:
> > > 
> > > if (pop_adjust) {
> > >      /* Final adjustment. */
> > >      offset = sink_latency + source_latency;
> > >      memblockq_adjust(offset, true);
> > >      pop_adjust = false;
> > > } else if (!pop_called) {
> > >      /* Waiting for the sink to start. */
> > >      offset = 0;
> > >      memblockq_adjust(offset, false);
> > > }
> > > push_called = true;
> > 
> > That doesn't handle correctly the case where just the source changes.
> > The first if condition needs to be changed to
> > 
> > if (pop_adjust || (pop_called && !push_called))
> > 
> 
> Sure it can be done. I'm not saying it can't. But why make it complicated?

Do you mean that the if condition is too complicated to your taste? It
accurately describes the cases where the final adjustment needs to be
done, and having one more condition in addition to your original code
doesn't seem very significant increase in complexity.

> The only situation were this part is called twice is when the source starts
> earlier than the sink.
> It should be enough to put in something like
> 
> if (!pop_called)
>     skip_the_adjustment
> 
> somewhere, because then we know it will be done again when pop has
> been called, but I thought that it's not worth the effort. In the end, this
> adjustment can only be called twice, if the module is starting up or if we
> have a profile change which involves source and sink.

Where would you put that check? Would you put it just before the
memblockq_adjust() call, like this:

                ...

                if (u->output_thread_info.pop_called) {
                    memblockq_adjust(u, time_delta, true);
                    u->output_thread_info.pop_adjust = false;
                }

                u->output_thread_info.push_called = true;

That would work, but then all the latency calculations would be done
for no reason, because the results of the calculation won't be used. To
me it would make sense to have the check in the if condition for the
whole code block, but that would then become exactly what I suggested,
which you found somehow unnecessarily complicated.

-- 
Tanu

https://www.patreon.com/tanuk