[pulseaudio-discuss] [PATCH 07/13] loopback: Refactor latency initialization

[Tanu Kaskinen]

On Sun, 2015-11-22 at 13:21 +0100, Georg Chini wrote:
> On 22.11.2015 00:27, Tanu Kaskinen wrote:
> > On Sat, 2015-11-21 at 19:42 +0100, Georg Chini wrote:
> > > On 20.11.2015 16:18, Tanu Kaskinen wrote:
> > > > On Fri, 2015-11-20 at 08:03 +0100, Georg Chini wrote:
> > > > > On 20.11.2015 01:03, Tanu Kaskinen wrote:
> > > > > > On Wed, 2015-02-25 at 19:43 +0100, Georg Chini wrote:
> > > The point is, that the assumption that source_output and sink_input
> > > rate are the same is not valid. As long as they are, you will not hit a
> > > problem.
> > I did cover also the case where there is clock drift and the rate
> > hasn't yet been stabilized. (It's the paragraph starting with "In the
> > cases where...") I argued that the clock drift won't cause big enough
> > data shortage to warrant a 75% safety margin relative to the sink
> > latency.
> > 
> > I now believe that my initial intuition about what the safety margin
> > should be to cover for rate errors was wrong, though. I now think that
> > if the sink input is consuming data too fast (i.e. the configured rate
> > is too low), the error margin has to be big enough to cover for all
> > excess samples consumed before the rate controller slows down the sink
> > input data consumption rate to be at or below the rate at which the
> > source produces data. For example, if it takes one adjustment cycle to
> > change a too-fast-consuming sink input to not-too-fast-consuming, the
> > error margin needs to be "rate_error_in_hz * adjust_time" samples. The
> > sink and source latencies are irrelevant. If it takes more than one
> > adjustment cycle, it's more complicated, but an upper bound for the
> > minimum safety margin is "max_expected_rate_error_in_hz *
> > number_of_cycles * adjust_time" samples.
> 
> This can't be true. To transform it to the time you have to divide
> by the sample rate, so your formula (for the one step case) is
> basically
> safty_time = relative_rate_error * adjust_time
> The module keeps the relative rate error for a single step below
> 0.002, so you end up with 0.002 * adjust_time, which means for
> 10 s adjust time you would need 20 msec safety margin regardless
> of the sink latency. This is far more than the minimum possible
> latency so it does not make any sense to me. If you have a
> large initial latency error which would require multiple steps your
> estimate gets even worse.

Well, if the sink input consumes 20 ms more audio during 10 seconds
than what the source produces, then that's how much you need to buffer.
I don't see any way around that. Underrun-proof 4 ms latency is just
impossible with those parameters. Using smaller adjust_time is an easy
way to mitigate this. Maybe it would make sense to use frequent
adjustments in the beginning, and once the controller stabilizes,
increase the adjustment interval?

> The other big problem is that you cannot determine the number
> of cycles you will need to correct the initial latency error because
> this error is unknown before the first adjustment cycle.
> 
> When you calculate that safety margin you also have to consider
> that the controller might overshoot, so you temporarily could
> get less latency than you requested.

Can the overshoot be greater than the initial error? Getting less
latency than requested is exactly the problem that the safety margin is
supposed to solve. If the overshoot is less than the initial error, the
safety margin calculation doesn't need to take the overshoot into
account.

> It is however true, that the sink latency in itself is not relevant,
> but it controls when chunks of audio are transferred and how
> big those chunks are. So the connection is indirect, maybe
> max_request is a better indicator than the latency. I'll do some
> experiments the next days to find out.

I don't think max_request is any better. In practice it's almost the
same thing as the configured sink latency.

> > 
> > > Once you are in a steady state you only have to care about jitter.
> > > I cannot clearly remember how I derived that value, probably
> > > experiments, but I still believe that 0.75 is a "good" estimate. If
> > > you look at the 4 msec case, the buffer_latency is slightly lower
> > > than 3/4 of the sink latency (1.667 versus 1.75 ms) but this is
> > > also already slightly unstable.
> > > In a more general case the resulting latency will be
> > > 1.75 * minimum_sink_latency, which I would consider small enough.
> > I don't understand where that "1.75 * minimum_sink_latency" comes from.
> > I'd understand if you said "0.75 * maximum_sink_latency", because
> > that's what the code seems to do.
> 
> The 0.75 * sink_latency is just the part that is stored within the
> module (mostly in the memblockq), so you have to add the
> sink_latency to it. That's 1.75 * sink_latency then. The source
> latency does not seem to play any role, whatever you configure,
> the reported value is most of the time near 0.

Are you saying that the configured source latency doesn't actually
affect the real source latency (at least as reported)? That sounds like
a bug. (One possible explanation would be that since the latency
queries aren't done with random intervals, the queries might by chance
be "synchronized" with certain source buffer fill level.)

> All calculations assume that when I configure source and sink
> latency to 1/3 of the requested latency each, I'll end up with
> having about 1/3 of the latency in source and sink together.
> I know this is strange but so far I have not seen a case where
> this assumption fails.

It doesn't sound strange to me, because if you randomly sample the
buffer fill level of a sink or a source, on average it will be 50% full
(assuming that refills happen when the buffer gets empty, which is
approximately true when using timer-based scheduling). On average,
then, the sum of the sink and source latencies will be half of the sum
of the configured latencies.

> > Anyway, any formula involving the sink or source latency seems bogus to
> > me. adjust_time and the rate error (or an estimate of the maximum rate
> > error, if the real error isn't known) are what matter. Plus of course
> > some margin for cpu overhead/jitter, which should be constant (I
> > think). The jitter margin might contribute more than the margin for
> > covering for rate errors.
> 
> In the end adjust_time and rate_error don't matter because they are
> inversely proportional to each other, so that the product is roughly
> constant.

Can you elaborate? I don't see why they would be inversely proportional
to each other.

> > 
> > I guess none of this matters, if I'm right in saying that the sink and
> > source latencies are irrelevant for calculating the required safety
> > margin. Hmm... now it occurred to me that the sink latency has some
> > (smallish?) relevance after all, because when the sink input rate
> > changes, the sink will have some data buffered that is resampled using
> > the old rate. If the old rate was too low, the buffered data will be
> > played too fast. Therefore, the rate error safety margin formula that I
> > presented earlier has to be modified to include the sink latency. So
> > the rate error safety margin should be at least
> > "max_expected_rate_error_in_hz * (number_of_cycles * adjust_time +
> > sink_latency)" samples.
> > 
> Some additional remarks about the importance of buffer_latency and
> those safeguards:
> 
> 1) As long as you are not at the lower latency limit both are more
> or less irrelevant.

True. To minimize wakeups, it's still good to minimize buffer_latency,
though, so that the device buffers can be as large as possible.

> 2) Even if those safeguards are too small, the module will fix up
> buffer_latency during runtime until there are no longer any underruns.

I'm not yet familiar with that logic, but even if you don't adapt
buffer_latency, once the rate gets stabilized, underruns shouldn't
happen any more due to rate errors, because the errors will be very
small. Scheduling delays of course don't disappear, so increasing
buffer_latency on underruns can still be a good idea.

-- 
Tanu