On Sun, 2016-06-05 at 21:05 +0200, Georg Chini wrote:
> In certain situations, the reported sink or source latency may become
> negative.
What are those certain situations?
> This does not indicate that the latency is indeed negative but can be
> considered
> merely an offset error. The current get_latency() calls truncate negative
> latencies
> because they do not make sense from a physical point of view.
> In the context of module loopback, negative source or sink latencies are
> acceptable
> because the overall latency never becomes negative. Truncating negative
> values leads
> to discontinuities in the latency reports, therefore this patch implements
> functions
> in source.c and sink.c which can return the raw value without truncation.
What's the practical problem with the discontinuities? A negative
latency report is always incorrect, so changing it to 0 can only make
the latency report closer to truth. The explanation isn't sufficient
for me to understand why that is such a big problem that it warrants
adding this stuff to the core.
If this stuff makes sense, there should be a comment somewhere
explaining what the "raw latency" means and why we care about it.
> The corresponding PA_SINK_MESSAGE_GET_RAW_LATENCY needs to be handled by the
> driver.
>
> ---
> src/pulsecore/sink.c | 30 ++++++++++++++++++++++++++++++
> src/pulsecore/sink.h | 2 ++
> src/pulsecore/source.c | 30 ++++++++++++++++++++++++++++++
> src/pulsecore/source.h | 2 ++
> 4 files changed, 64 insertions(+)
>
> diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
> index 3f1ef72..2b72279 100644
> --- a/src/pulsecore/sink.c
> +++ b/src/pulsecore/sink.c
> @@ -1550,6 +1550,35 @@ pa_usec_t pa_sink_get_latency_within_thread(pa_sink
> *s) {
> return usec;
> }
>
> +/* Called from IO thread */
> +int64_t pa_sink_get_raw_latency_within_thread(pa_sink *s) {
> + int64_t usec = 0;
> + pa_msgobject *o;
> +
> + pa_sink_assert_ref(s);
> + pa_sink_assert_io_context(s);
> + pa_assert(PA_SINK_IS_LINKED(s->thread_info.state));
> +
> + /* The returned value is supposed to be in the time domain of the sound
> card! */
> +
> + if (s->thread_info.state == PA_SINK_SUSPENDED)
> + return 0;
> +
> + if (!(s->flags & PA_SINK_LATENCY))
> + return 0;
> +
> + o = PA_MSGOBJECT(s);
> +
> + /* FIXME: We probably should make this a proper vtable callback instead
> of going through process_msg() */
> +
> + if (o->process_msg(o, PA_SINK_MESSAGE_GET_RAW_LATENCY, &usec, 0, NULL) <
> 0) {
> + if (o->process_msg(o, PA_SINK_MESSAGE_GET_LATENCY, &usec, 0, NULL) <
> 0)
> + return 0;
This code path ignores the latency offset. I don't think it makes sense
to return here, the failure can just be ignored.
--
Tanu
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
