[pulseaudio-discuss] [PATCH 3/6] source, sink, core: Setting latency offsets of individual sources and sinks.

[Chris Billington]

Thanks Tanu,

I am still interested in this and will fix my mistakes, thanks for finding
them!

I'll get to this sometime in the next week.

Regards,

Chris

On Mon, Jun 6, 2016 at 4:21 AM, Tanu Kaskinen <tanuk at iki.fi> wrote:

> On Sat, 2016-01-23 at 12:31 +1100, Chris Billington wrote:
>
> > Some sinks and sources do not have ports associated with them, such as a
> null
> > sink. Nonetheless sources and sinks with no ports can be used for sound
> > input/output with an associated latency, for example by capturing output
> from
> > a null sink and directing over a network, which is how some
> implementations of
> > network audio streaming protocols interact with PulseAudio.
> >
> > To keep graphics in sync with audio in these cases, the
> application/plugin must
> > be able to report the latency due to network transport/buffering to
> PulseAudio
> > and have it included in the total latency for that source or sink.
> >
> > Previously, only the latency offset of a whole port could be set.
> > You could call pa_sink_set_latency_offset(), but the value you passed in
> would
> > be overwritten on change to a new port - that value was really just a
> cached
> > value of the port's latency offset. So those variables and functions
> have since
> > (in the previous commit) been renamed pa_sink_set_port_latency_offset()
> etc, to
> > distinguish them from the ones introduced in this commit.
> >
> > This patch adds set_sink_latency_offset() and set_source_latency
> offset(),
> > and the corresponding members of the sink and source structs.
> >
> > It also adds corresponding hooks PA_CORE_HOOK_SINK_LATENCY_OFFSET_CHANGED
> > and PA_CORE_HOOK_SOURCE_LATENCY_OFFSET_CHANGED.
> >
> > The total latency reported by a source or sink now includes both the port
> > latency offset (if any) as well as the source or sink's own latency
> offset.
>
> Thanks, looks mostly good. I'll point out a few small issues below.
>
> Since it's been already many months since you submitted these patches,
> please let me know if during that time you have lost interest to finish
> off this work (it would be understandable).
>
> > @@ -125,6 +125,8 @@ typedef enum pa_core_hook {
> >      PA_CORE_HOOK_CARD_PROFILE_AVAILABLE_CHANGED,
> >      PA_CORE_HOOK_PORT_AVAILABLE_CHANGED,
> >      PA_CORE_HOOK_PORT_LATENCY_OFFSET_CHANGED,
> > +    PA_CORE_HOOK_SINK_LATENCY_OFFSET_CHANGED,
> > +    PA_CORE_HOOK_SOURCE_LATENCY_OFFSET_CHANGED,
>
> A cosmetic note about the hook list ordering: I prefer different
> grouping - e.g. all sink hooks should be together.
>
> > @@ -314,6 +319,11 @@ pa_sink* pa_sink_new(
> >      else
> >          s->port_latency_offset = 0;
> >
> > +    if (data->latency_offset)
> > +        s->latency_offset = data->latency_offset;
> > +    else
> > +        s->latency_offset = 0;
>
> This can be replaced with just
>
>     s->latency_offset = data->latency_offset;
>
> without any changes in behaviour. The same applies to the code in
> source.c.
>
> > @@ -1512,6 +1525,12 @@ pa_usec_t pa_sink_get_latency(pa_sink *s) {
> >      else
> >          usec = 0;
> >
> > +    /* Similarly checking for underflow */
> > +    if (-s->latency_offset <= (int64_t) usec)
> > +        usec += s->latency_offset;
> > +    else
> > +        usec = 0;
>
> The two offsets should be summed together and then added to usec,
> rather than adding them to usec one by one. The code in the patch
> doesn't work correctly if, for example, the initial usec is 1000, the
> port latency offset -2000 and the sink latency offset is 2000. The
> final usec should be unchanged (1000), because the two offsets undo
> each other, but this code results in the final usec being 2000.
>
> This applies also to pa_sink_get_latency_within_thread() and the
> corresponding functions in source.c.
>
> > @@ -1096,6 +1107,8 @@ pa_usec_t pa_source_get_latency(pa_source *s) {
> >
> >      pa_assert_se(pa_asyncmsgq_send(s->asyncmsgq, PA_MSGOBJECT(s),
> PA_SOURCE_MESSAGE_GET_LATENCY, &usec, 0, NULL) == 0);
> >
> > +    /* Total latency offset is the sum of the port latency offset and
> the sink latency offset */
>
> Copy-paste mistake: s/sink/source/
>
> > @@ -1130,6 +1149,8 @@ pa_usec_t
> pa_source_get_latency_within_thread(pa_source *s) {
> >      if (o->process_msg(o, PA_SOURCE_MESSAGE_GET_LATENCY, &usec, 0,
> NULL) < 0)
> >          return -1;
> >
> > +    /* Total latency offset is the sum of the port latency offset and
> the sink latency offset */
>
> The same copy-paste error here too.
>
> --
> Tanu
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/pulseaudio-discuss/attachments/20160608/a58e2739/attachment.html>