[pulseaudio-discuss] [PATCH 2/2] combine-sink: add suport for DYNAMIC_LATENCY

[Tanu Kaskinen]

On Tue, 2014-05-27 at 21:37 +0200, Wim Taymans wrote:
> +/* Called from sink I/O thread context */
> +static void sink_update_requested_latency(pa_sink *s) {
> +    struct userdata *u;
> +    struct output *o;
> +    pa_usec_t latency;
> +
> +    pa_sink_assert_ref(s);
> +    pa_assert_se(u = s->userdata);
> +
> +    latency = pa_sink_get_requested_latency_within_thread(s);
> +    /* We want to avoid letting the sinks decide a latency themselves because
> +     * we all want the sinks to have the same latency. Also, we somehow don't deal
> +     * well with switching between latency of -1 and something else */
> +    if (latency == (pa_usec_t) -1)
> +        return;

We don't want all the sinks to have the same latency. That's not an
achievable goal anyway (not all sinks have dynamic latency, so what do
you do if two sinks have different fixed latencies?). The thing that we
want is that all outputs to have the same latency. If there are sinks
that have different latencies, then some outputs (the ones connected to
a lower-latency sink) have more data buffered in o->memblockq than
others, which will offset the sink latency differences.

If there are problems with switching between -1 and something else, that
sounds like something that needs to be fixed. What kind of problems are
we talking about here? One problem is that combine-sink doesn't
implement rewinding, so if the combine-sink is configured to have high
latency, then things (volume changes, starting new streams etc.) happen
with a delay, because there's a lot of audio buffered that can't be
rewound. For this reason, my earlier suggestion to forward -1 downstream
was not a good one. Instead, we should pass the sink maximum latency to
downstream if our own sink's requested latency is -1, and the sink
maximum latency should be capped at BLOCK_USEC. (BTW, I now noticed that
u->block_usec is a redundant variable, because it's always the same as
the BLOCK_USEC constant.) update_latency_range() needs to be modified so
that the maximum latency is never more than max(BLOCK_USEC,
min_latency).

> @@ -1283,6 +1344,7 @@ int pa__init(pa_module*m) {
>  
>      u->block_usec = BLOCK_USEC;
>      pa_sink_set_max_request(u->sink, pa_usec_to_bytes(u->block_usec, &u->sink->sample_spec));
> +    pa_sink_set_latency_range_within_thread(u->sink, u->block_usec, u->block_usec);

It would be very good to set the minimum latency to 0 here (and in
update_latency_range() if there are no outputs), because otherwise we
effectively don't support dynamic latency when there are no outputs.
However, that probably requires some changes to the "null mode"
rendering so that we wake up frequently enough if there's a low-latency
stream connected to the combine-sink.

-- 
Tanu