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
