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

Submitted by Chris Billington on Jan. 23, 2016, 1:31 a.m.

Details

Message ID 1453512698-4055-3-git-send-email-chrisjbillington@gmail.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in PulseAudio

Not browsing as part of any series.

Commit Message

Chris Billington Jan. 23, 2016, 1:31 a.m.
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.
---
 src/pulsecore/core.h   |  2 ++
 src/pulsecore/sink.c   | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 src/pulsecore/sink.h   | 13 ++++++++++++-
 src/pulsecore/source.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 src/pulsecore/source.h | 13 ++++++++++++-
 5 files changed, 120 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/pulsecore/core.h b/src/pulsecore/core.h
index aefc1eb..864863f 100644
--- a/src/pulsecore/core.h
+++ b/src/pulsecore/core.h
@@ -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,
     PA_CORE_HOOK_DEFAULT_SINK_CHANGED,
     PA_CORE_HOOK_DEFAULT_SOURCE_CHANGED,
     PA_CORE_HOOK_MODULE_NEW,
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 86656f6..f7b5183 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -120,6 +120,11 @@  void pa_sink_new_data_set_volume(pa_sink_new_data *data, const pa_cvolume *volum
         data->volume = *volume;
 }
 
+void pa_sink_new_data_set_latency_offset(pa_sink_new_data *data, int64_t latency_offset) {
+    pa_assert(data);
+    data->latency_offset = latency_offset;
+}
+
 void pa_sink_new_data_set_muted(pa_sink_new_data *data, bool mute) {
     pa_assert(data);
 
@@ -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;
+
     s->save_volume = data->save_volume;
     s->save_muted = data->save_muted;
 
@@ -346,6 +356,7 @@  pa_sink* pa_sink_new(
     s->thread_info.volume_change_safety_margin = core->deferred_volume_safety_margin_usec;
     s->thread_info.volume_change_extra_delay = core->deferred_volume_extra_delay_usec;
     s->thread_info.port_latency_offset = s->port_latency_offset;
+    s->thread_info.latency_offset = s->latency_offset;
 
     /* FIXME: This should probably be moved to pa_sink_put() */
     pa_assert_se(pa_idxset_put(core->sinks, s, &s->index) >= 0);
@@ -1505,6 +1516,8 @@  pa_usec_t pa_sink_get_latency(pa_sink *s) {
 
     pa_assert_se(pa_asyncmsgq_send(s->asyncmsgq, PA_MSGOBJECT(s), PA_SINK_MESSAGE_GET_LATENCY, &usec, 0, NULL) == 0);
 
+    /* Total latency offset is the sum of the port latency offset and the sink latency offset */
+
     /* usec is unsigned, so check that the offset can be added to usec without
      * underflowing. */
     if (-s->port_latency_offset <= (int64_t) usec)
@@ -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;
+
     return usec;
 }
 
@@ -1539,6 +1558,8 @@  pa_usec_t pa_sink_get_latency_within_thread(pa_sink *s) {
     if (o->process_msg(o, PA_SINK_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 */
+
     /* usec is unsigned, so check that the offset can be added to usec without
      * underflowing. */
     if (-s->thread_info.port_latency_offset <= (int64_t) usec)
@@ -1546,6 +1567,12 @@  pa_usec_t pa_sink_get_latency_within_thread(pa_sink *s) {
     else
         usec = 0;
 
+    /* Similarly checking for underflow */
+    if (-s->thread_info.latency_offset <= (int64_t) usec)
+        usec += s->thread_info.latency_offset;
+    else
+        usec = 0;
+
     return usec;
 }
 
@@ -2892,6 +2919,10 @@  int pa_sink_process_msg(pa_msgobject *o, int code, void *userdata, int64_t offse
             s->thread_info.port_latency_offset = offset;
             return 0;
 
+        case PA_SINK_MESSAGE_SET_LATENCY_OFFSET:
+            s->thread_info.latency_offset = offset;
+            return 0;
+
         case PA_SINK_MESSAGE_GET_LATENCY:
         case PA_SINK_MESSAGE_MAX:
             ;
@@ -3300,6 +3331,22 @@  void pa_sink_set_port_latency_offset(pa_sink *s, int64_t offset) {
 }
 
 /* Called from main context */
+void pa_sink_set_latency_offset(pa_sink *s, int64_t offset) {
+    pa_sink_assert_ref(s);
+    pa_assert_ctl_context();
+
+    s->latency_offset = offset;
+
+    if (PA_SINK_IS_LINKED(s->state))
+        pa_assert_se(pa_asyncmsgq_send(s->asyncmsgq, PA_MSGOBJECT(s), PA_SINK_MESSAGE_SET_LATENCY_OFFSET, NULL, offset, NULL) == 0);
+    else
+        s->thread_info.latency_offset = offset;
+
+    pa_subscription_post(s->core, PA_SUBSCRIPTION_EVENT_SINK|PA_SUBSCRIPTION_EVENT_CHANGE, s->index);
+    pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SINK_LATENCY_OFFSET_CHANGED], s);
+}
+
+/* Called from main context */
 size_t pa_sink_get_max_rewind(pa_sink *s) {
     size_t r;
     pa_assert_ctl_context();
diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
index 06a8ee9..a7fa96f 100644
--- a/src/pulsecore/sink.h
+++ b/src/pulsecore/sink.h
@@ -111,9 +111,13 @@  struct pa_sink {
     pa_device_port *active_port;
     pa_atomic_t mixer_dirty;
 
-    /* The latency offset is inherited from the currently active port */
+    /* This latency offset is inherited from the currently active port */
     int64_t port_latency_offset;
 
+    /* This latency offset is specific to the sink and will be summed with the above to give
+     * the total latency offset */
+    int64_t latency_offset;
+
     unsigned priority;
 
     bool set_mute_in_progress;
@@ -284,6 +288,9 @@  struct pa_sink {
         /* This latency offset is a direct copy from s->port_latency_offset */
         int64_t port_latency_offset;
 
+        /* This latency offset is a direct copy from s->latency_offset */
+        int64_t latency_offset;
+
         /* Delayed volume change events are queued here. The events
          * are stored in expiration order. The one expiring next is in
          * the head of the list. */
@@ -332,6 +339,7 @@  typedef enum pa_sink_message {
     PA_SINK_MESSAGE_SET_PORT,
     PA_SINK_MESSAGE_UPDATE_VOLUME_AND_MUTE,
     PA_SINK_MESSAGE_SET_PORT_LATENCY_OFFSET,
+    PA_SINK_MESSAGE_SET_LATENCY_OFFSET,
     PA_SINK_MESSAGE_MAX
 } pa_sink_message_t;
 
@@ -351,6 +359,7 @@  typedef struct pa_sink_new_data {
     pa_sample_spec sample_spec;
     pa_channel_map channel_map;
     uint32_t alternate_sample_rate;
+    int64_t latency_offset;
     pa_cvolume volume;
     bool muted:1;
 
@@ -373,6 +382,7 @@  void pa_sink_new_data_set_sample_spec(pa_sink_new_data *data, const pa_sample_sp
 void pa_sink_new_data_set_channel_map(pa_sink_new_data *data, const pa_channel_map *map);
 void pa_sink_new_data_set_alternate_sample_rate(pa_sink_new_data *data, const uint32_t alternate_sample_rate);
 void pa_sink_new_data_set_volume(pa_sink_new_data *data, const pa_cvolume *volume);
+void pa_sink_new_data_set_latency_offset(pa_sink_new_data *data, int64_t latency_offset);
 void pa_sink_new_data_set_muted(pa_sink_new_data *data, bool mute);
 void pa_sink_new_data_set_port(pa_sink_new_data *data, const char *port);
 void pa_sink_new_data_done(pa_sink_new_data *data);
@@ -418,6 +428,7 @@  unsigned pa_device_init_priority(pa_proplist *p);
 
 int pa_sink_update_rate(pa_sink *s, uint32_t rate, bool passthrough);
 void pa_sink_set_port_latency_offset(pa_sink *s, int64_t offset);
+void pa_sink_set_latency_offset(pa_sink *s, int64_t offset);
 
 /* The returned value is supposed to be in the time domain of the sound card! */
 pa_usec_t pa_sink_get_latency(pa_sink *s);
diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index da64e6b..21bb4f8 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -111,6 +111,11 @@  void pa_source_new_data_set_volume(pa_source_new_data *data, const pa_cvolume *v
         data->volume = *volume;
 }
 
+void pa_source_new_data_set_latency_offset(pa_source_new_data *data, int64_t latency_offset) {
+    pa_assert(data);
+    data->latency_offset = latency_offset;
+}
+
 void pa_source_new_data_set_muted(pa_source_new_data *data, bool mute) {
     pa_assert(data);
 
@@ -302,6 +307,11 @@  pa_source* pa_source_new(
     else
         s->port_latency_offset = 0;
 
+    if (data->latency_offset)
+            s->latency_offset = data->latency_offset;
+        else
+            s->latency_offset = 0;
+
     s->save_volume = data->save_volume;
     s->save_muted = data->save_muted;
 
@@ -331,6 +341,7 @@  pa_source* pa_source_new(
     s->thread_info.volume_change_safety_margin = core->deferred_volume_safety_margin_usec;
     s->thread_info.volume_change_extra_delay = core->deferred_volume_extra_delay_usec;
     s->thread_info.port_latency_offset = s->port_latency_offset;
+    s->thread_info.latency_offset = s->latency_offset;
 
     /* FIXME: This should probably be moved to pa_source_put() */
     pa_assert_se(pa_idxset_put(core->sources, s, &s->index) >= 0);
@@ -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 */
+
     /* usec is unsigned, so check that the offset can be added to usec without
      * underflowing. */
     if (-s->port_latency_offset <= (int64_t) usec)
@@ -1103,6 +1116,12 @@  pa_usec_t pa_source_get_latency(pa_source *s) {
     else
         usec = 0;
 
+    /* Similarly checking for underflow */
+    if (-s->latency_offset <= (int64_t) usec)
+        usec += s->latency_offset;
+    else
+        usec = 0;
+
     return usec;
 }
 
@@ -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 */
+
     /* usec is unsigned, so check that the offset can be added to usec without
      * underflowing. */
     if (-s->thread_info.port_latency_offset <= (int64_t) usec)
@@ -1137,6 +1158,12 @@  pa_usec_t pa_source_get_latency_within_thread(pa_source *s) {
     else
         usec = 0;
 
+    /* Similarly checking for underflow */
+    if (-s->thread_info.latency_offset <= (int64_t) usec)
+        usec += s->thread_info.latency_offset;
+    else
+        usec = 0;
+
     return usec;
 }
 
@@ -2243,6 +2270,10 @@  int pa_source_process_msg(pa_msgobject *object, int code, void *userdata, int64_
             s->thread_info.port_latency_offset = offset;
             return 0;
 
+        case PA_SOURCE_MESSAGE_SET_LATENCY_OFFSET:
+            s->thread_info.latency_offset = offset;
+            return 0;
+
         case PA_SOURCE_MESSAGE_MAX:
             ;
     }
@@ -2579,6 +2610,22 @@  void pa_source_set_port_latency_offset(pa_source *s, int64_t offset) {
 }
 
 /* Called from main thread */
+void pa_source_set_latency_offset(pa_source *s, int64_t offset) {
+    pa_source_assert_ref(s);
+    pa_assert_ctl_context();
+
+    s->latency_offset = offset;
+
+    if (PA_SOURCE_IS_LINKED(s->state))
+        pa_assert_se(pa_asyncmsgq_send(s->asyncmsgq, PA_MSGOBJECT(s), PA_SOURCE_MESSAGE_SET_LATENCY_OFFSET, NULL, offset, NULL) == 0);
+    else
+        s->thread_info.latency_offset = offset;
+
+    pa_subscription_post(s->core, PA_SUBSCRIPTION_EVENT_SOURCE|PA_SUBSCRIPTION_EVENT_CHANGE, s->index);
+    pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SOURCE_LATENCY_OFFSET_CHANGED], s);
+}
+
+/* Called from main thread */
 size_t pa_source_get_max_rewind(pa_source *s) {
     size_t r;
     pa_assert_ctl_context();
diff --git a/src/pulsecore/source.h b/src/pulsecore/source.h
index 31cd158..42819c2 100644
--- a/src/pulsecore/source.h
+++ b/src/pulsecore/source.h
@@ -112,9 +112,13 @@  struct pa_source {
     pa_device_port *active_port;
     pa_atomic_t mixer_dirty;
 
-    /* The latency offset is inherited from the currently active port */
+    /* This latency offset is inherited from the currently active port */
     int64_t port_latency_offset;
 
+    /* This latency offset is specific to the sink and will be summed with the above to give
+     * the total latency offset */
+    int64_t latency_offset;
+
     unsigned priority;
 
     bool set_mute_in_progress;
@@ -226,6 +230,9 @@  struct pa_source {
         /* This latency offset is a direct copy from s->port_latency_offset */
         int64_t port_latency_offset;
 
+        /* This latency offset is a direct copy from s->latency_offset */
+        int64_t latency_offset;
+
         /* Delayed volume change events are queued here. The events
          * are stored in expiration order. The one expiring next is in
          * the head of the list. */
@@ -270,6 +277,7 @@  typedef enum pa_source_message {
     PA_SOURCE_MESSAGE_SET_PORT,
     PA_SOURCE_MESSAGE_UPDATE_VOLUME_AND_MUTE,
     PA_SOURCE_MESSAGE_SET_PORT_LATENCY_OFFSET,
+    PA_SOURCE_MESSAGE_SET_LATENCY_OFFSET,
     PA_SOURCE_MESSAGE_MAX
 } pa_source_message_t;
 
@@ -289,6 +297,7 @@  typedef struct pa_source_new_data {
     pa_sample_spec sample_spec;
     pa_channel_map channel_map;
     uint32_t alternate_sample_rate;
+    int64_t latency_offset;
     pa_cvolume volume;
     bool muted:1;
 
@@ -311,6 +320,7 @@  void pa_source_new_data_set_sample_spec(pa_source_new_data *data, const pa_sampl
 void pa_source_new_data_set_channel_map(pa_source_new_data *data, const pa_channel_map *map);
 void pa_source_new_data_set_alternate_sample_rate(pa_source_new_data *data, const uint32_t alternate_sample_rate);
 void pa_source_new_data_set_volume(pa_source_new_data *data, const pa_cvolume *volume);
+void pa_source_new_data_set_latency_offset(pa_source_new_data *data, int64_t latency_offset);
 void pa_source_new_data_set_muted(pa_source_new_data *data, bool mute);
 void pa_source_new_data_set_port(pa_source_new_data *data, const char *port);
 void pa_source_new_data_done(pa_source_new_data *data);
@@ -351,6 +361,7 @@  void pa_source_update_flags(pa_source *s, pa_source_flags_t mask, pa_source_flag
 /*** May be called by everyone, from main context */
 
 void pa_source_set_port_latency_offset(pa_source *s, int64_t offset);
+void pa_source_set_latency_offset(pa_source *s, int64_t offset);
 
 /* The returned value is supposed to be in the time domain of the sound card! */
 pa_usec_t pa_source_get_latency(pa_source *s);

Comments

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
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@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
>