[pulseaudio-discuss,6/6] pactl, pacmd, cli-command: Added set-(sink|source)-latency-offset commands.

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

Details

Message ID 1453512698-4055-6-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.
Added cli commands to both utilities to make use of the new
PA_COMMAND_SET_(SINK|SOURCE)_LATENCY_OFFSET protocol commands.

This enables applications to configure the latency of individual sources
and sinks via the command line interface.

The specific motivation is to enable network streaming applications which
interface with PulseAudio by capturing the output of a null sink, to have
PulseAudio correctly report the network latency (known by the application)
as part of the sink's total latency.
---
 man/pactl.1.xml.in               | 12 +++++++
 man/pulse-cli-syntax.5.xml.in    | 10 ++++++
 shell-completion/bash/pulseaudio |  3 +-
 shell-completion/zsh/_pulseaudio |  6 ++++
 src/pulsecore/cli-command.c      | 74 ++++++++++++++++++++++++++++++++++++++++
 src/utils/pacmd.c                |  1 +
 src/utils/pactl.c                | 37 ++++++++++++++++++++
 7 files changed, 142 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/man/pactl.1.xml.in b/man/pactl.1.xml.in
index c2064ca..530fb5d 100644
--- a/man/pactl.1.xml.in
+++ b/man/pactl.1.xml.in
@@ -184,6 +184,18 @@  License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>.
     </option>
 
     <option>
+      <p><opt>set-sink-latency-offset</opt> <arg>SINK</arg> <arg>OFFSET</arg></p>
+      <optdesc><p>Set a latency offset to a specified sink (identified by its symbolic name or numerical index).
+      <arg>OFFSET</arg> is a number which represents the latency offset in microseconds</p></optdesc>
+    </option>
+
+    <option>
+      <p><opt>set-source-latency-offset</opt> <arg>SOURCE</arg> <arg>OFFSET</arg></p>
+      <optdesc><p>Set a latency offset to a specified source (identified by its symbolic name or numerical index).
+      <arg>OFFSET</arg> is a number which represents the latency offset in microseconds</p></optdesc>
+    </option>
+
+    <option>
       <p><opt>set-sink-volume</opt> <arg>SINK</arg> <arg>VOLUME [VOLUME ...]</arg></p>
       <optdesc><p>Set the volume of the specified sink (identified by its symbolic name or numerical index).
       <arg>VOLUME</arg> can be specified as an integer (e.g. 2000, 16384), a linear factor (e.g. 0.4, 1.100), a percentage
diff --git a/man/pulse-cli-syntax.5.xml.in b/man/pulse-cli-syntax.5.xml.in
index 0a0faba..1b976cc 100644
--- a/man/pulse-cli-syntax.5.xml.in
+++ b/man/pulse-cli-syntax.5.xml.in
@@ -163,6 +163,16 @@  License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>.
     </option>
 
     <option>
+      <p><opt>set-sink-latency-offset</opt> <arg>sink-index|sink-name</arg> <arg>offset</arg> </p>
+      <optdesc><p>Change the latency offset of a sink</p></optdesc>
+    </option>
+
+    <option>
+      <p><opt>set-source-latency-offset</opt> <arg>source-index|source-name</arg> <arg>offset</arg> </p>
+      <optdesc><p>Change the latency offset of a source</p></optdesc>
+    </option>
+
+    <option>
       <p><opt>suspend-sink|suspend-source</opt> <arg>index|name</arg> <arg>boolean</arg></p>
       <optdesc><p>Suspend (i.e. disconnect from the underlying hardware) a sink
       (resp. source).</p></optdesc>
diff --git a/shell-completion/bash/pulseaudio b/shell-completion/bash/pulseaudio
index cfcf7ff..2a0dfcc 100644
--- a/shell-completion/bash/pulseaudio
+++ b/shell-completion/bash/pulseaudio
@@ -120,7 +120,8 @@  _pactl() {
                     set-source-port set-sink-volume set-source-volume
                     set-sink-input-volume set-source-output-volume set-sink-mute
                     set-source-mute set-sink-input-mute set-source-output-mute
-                    set-sink-formats set-port-latency-offset subscribe help)
+                    set-sink-formats set-port-latency-offset set-sink-latency-offset
+                    set-source-latency-offset subscribe help)
 
     _init_completion -n = || return
     preprev=${words[$cword-2]}
diff --git a/shell-completion/zsh/_pulseaudio b/shell-completion/zsh/_pulseaudio
index c7d68f5..4746809 100644
--- a/shell-completion/zsh/_pulseaudio
+++ b/shell-completion/zsh/_pulseaudio
@@ -254,6 +254,8 @@  _pactl_completion() {
             'set-sink-port: set the sink port of a sink'
             'set-source-port: set the source port of a source'
             'set-port-latency-offset: set a latency offset on a port'
+            'set-sink-latency-offset: set a latency offset on a sink'
+            'set-source-latency-offset: set a latency offset on a source'
             'set-sink-volume: set the volume of a sink'
             'set-source-volume: set the volume of a source'
             'set-sink-input-volume: set the volume of a stream'
@@ -492,6 +494,8 @@  _pactl_completion() {
             set-source-output-mute)                _set_source_output_mute_parameter;;
             set-sink-formats)                      if ((CURRENT == 2)); then _devices; fi;;
             set-port-latency-offset)               _set_port_latency_offset_parameter;;
+            set-sink-latency-offset)               if ((CURRENT == 2)); then _devices; fi;;
+            set-source-latency-offset)             if ((CURRENT == 2)); then _devices; fi;;
         esac
     }
 
@@ -534,6 +538,8 @@  _pacmd_completion() {
             'set-sink-port: set the sink port of a sink'
             'set-source-port: set the source port of a source'
             'set-port-latency-offset: set a latency offset on a port'
+            'set-sink-latency-offset: set a latency offset on a sink'
+            'set-source-latency-offset: set a latency offset on a source'
             'suspend-sink: suspend or resume a sink'
             'suspend-source: suspend or resume a source'
             'suspend: suspend all sinks and sources'
diff --git a/src/pulsecore/cli-command.c b/src/pulsecore/cli-command.c
index cd88c47..6e694db 100644
--- a/src/pulsecore/cli-command.c
+++ b/src/pulsecore/cli-command.c
@@ -134,6 +134,8 @@  static int pa_cli_command_card_profile(pa_core *c, pa_tokenizer *t, pa_strbuf *b
 static int pa_cli_command_sink_port(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail);
 static int pa_cli_command_source_port(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail);
 static int pa_cli_command_port_offset(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail);
+static int pa_cli_command_sink_offset(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail);
+static int pa_cli_command_source_offset(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail);
 static int pa_cli_command_dump_volumes(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail);
 
 /* A method table for all available commands */
@@ -168,6 +170,8 @@  static const struct command commands[] = {
     { "set-sink-port",           pa_cli_command_sink_port,          "Change the port of a sink (args: index|name, port-name)", 3},
     { "set-source-port",         pa_cli_command_source_port,        "Change the port of a source (args: index|name, port-name)", 3},
     { "set-port-latency-offset", pa_cli_command_port_offset,        "Change the latency of a port (args: card-index|card-name, port-name, latency-offset)", 4},
+    { "set-sink-latency-offset", pa_cli_command_sink_offset,        "Change the latency of a sink (args: sink-index|sink-name, latency-offset)", 3},
+    { "set-source-latency-offset", pa_cli_command_source_offset,    "Change the latency of a source (args: source-index|source-name, latency-offset)", 3},
     { "suspend-sink",            pa_cli_command_suspend_sink,       "Suspend sink (args: index|name, bool)", 3},
     { "suspend-source",          pa_cli_command_suspend_source,     "Suspend source (args: index|name, bool)", 3},
     { "suspend",                 pa_cli_command_suspend,            "Suspend all sinks and all sources (args: bool)", 2},
@@ -1777,6 +1781,76 @@  static int pa_cli_command_port_offset(pa_core *c, pa_tokenizer *t, pa_strbuf *bu
     return 0;
 }
 
+static int pa_cli_command_sink_offset(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail) {
+    const char *n, *l;
+    pa_sink *sink;
+    int64_t offset;
+
+    pa_core_assert_ref(c);
+    pa_assert(t);
+    pa_assert(buf);
+    pa_assert(fail);
+
+    if (!(n = pa_tokenizer_get(t, 1))) {
+           pa_strbuf_puts(buf, "You need to specify a sink either by its name or its index.\n");
+           return -1;
+    }
+
+    if (!(l = pa_tokenizer_get(t, 2))) {
+        pa_strbuf_puts(buf, "You need to specify a latency offset.\n");
+        return -1;
+    }
+
+    if (pa_atol(l, &offset) < 0) {
+        pa_strbuf_puts(buf, "Failed to parse the latency offset.\n");
+        return -1;
+    }
+
+    if (!(sink = pa_namereg_get(c, n, PA_NAMEREG_SINK))) {
+        pa_strbuf_puts(buf, "No sink found by this name or index.\n");
+        return -1;
+    }
+
+    pa_sink_set_latency_offset(sink, offset);
+
+    return 0;
+}
+
+static int pa_cli_command_source_offset(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail) {
+    const char *n, *l;
+    pa_source *source;
+    int64_t offset;
+
+    pa_core_assert_ref(c);
+    pa_assert(t);
+    pa_assert(buf);
+    pa_assert(fail);
+
+    if (!(n = pa_tokenizer_get(t, 1))) {
+           pa_strbuf_puts(buf, "You need to specify a source either by its name or its index.\n");
+           return -1;
+    }
+
+    if (!(l = pa_tokenizer_get(t, 2))) {
+        pa_strbuf_puts(buf, "You need to specify a latency offset.\n");
+        return -1;
+    }
+
+    if (pa_atol(l, &offset) < 0) {
+        pa_strbuf_puts(buf, "Failed to parse the latency offset.\n");
+        return -1;
+    }
+
+    if (!(source = pa_namereg_get(c, n, PA_NAMEREG_SOURCE))) {
+        pa_strbuf_puts(buf, "No source found by this name or index.\n");
+        return -1;
+    }
+
+    pa_source_set_latency_offset(source, offset);
+
+    return 0;
+}
+
 static int pa_cli_command_dump(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail) {
     pa_module *m;
     pa_sink *sink;
diff --git a/src/utils/pacmd.c b/src/utils/pacmd.c
index 616573c..e0ea919 100644
--- a/src/utils/pacmd.c
+++ b/src/utils/pacmd.c
@@ -72,6 +72,7 @@  static void help(const char *argv0) {
     printf("%s %s %s\n", argv0, "set-card-profile", _("CARD PROFILE"));
     printf("%s %s %s\n", argv0, "set-(sink|source)-port", _("NAME|#N PORT"));
     printf("%s %s %s\n", argv0, "set-port-latency-offset", _("CARD-NAME|CARD-#N PORT OFFSET"));
+    printf("%s %s %s\n", argv0, "set-sink-(sink|source)-latency-offset", _("NAME|#N OFFSET"));
     printf("%s %s %s\n", argv0, "set-log-target", _("TARGET"));
     printf("%s %s %s\n", argv0, "set-log-level", _("NUMERIC-LEVEL"));
     printf("%s %s %s\n", argv0, "set-log-meta", _("1|0"));
diff --git a/src/utils/pactl.c b/src/utils/pactl.c
index 8715126..b627971 100644
--- a/src/utils/pactl.c
+++ b/src/utils/pactl.c
@@ -1406,6 +1406,14 @@  static void context_state_callback(pa_context *c, void *userdata) {
                     o = pa_context_set_port_latency_offset(c, card_name, port_name, latency_offset, simple_callback, NULL);
                     break;
 
+                case SET_SINK_LATENCY_OFFSET:
+                    o = pa_context_set_sink_latency_offset(c, sink_name, latency_offset, simple_callback, NULL);
+                    break;
+
+                case SET_SOURCE_LATENCY_OFFSET:
+                    o = pa_context_set_source_latency_offset(c, source_name, latency_offset, simple_callback, NULL);
+                    break;
+
                 case SUBSCRIBE:
                     pa_context_set_subscribe_callback(c, context_subscribe_callback, NULL);
 
@@ -1582,6 +1590,7 @@  static void help(const char *argv0) {
     printf("%s %s %s %s\n", argv0, _("[options]"), "set-(sink-input|source-output)-mute", _("#N 1|0|toggle"));
     printf("%s %s %s %s\n", argv0, _("[options]"), "set-sink-formats", _("#N FORMATS"));
     printf("%s %s %s %s\n", argv0, _("[options]"), "set-port-latency-offset", _("CARD-NAME|CARD-#N PORT OFFSET"));
+    printf("%s %s %s %s\n", argv0, _("[options]"), "set-(sink|source)-latency-offset", _("NAME|#N OFFSET"));
     printf("%s %s %s\n",    argv0, _("[options]"), "subscribe");
     printf(_("\nThe special names @DEFAULT_SINK@, @DEFAULT_SOURCE@ and @DEFAULT_MONITOR@\n"
              "can be used to specify the default sink, source and monitor.\n"));
@@ -2048,6 +2057,34 @@  int main(int argc, char *argv[]) {
                 goto quit;
             }
 
+        } else if (pa_streq(argv[optind], "set-sink-latency-offset")) {
+            action = SET_SINK_LATENCY_OFFSET;
+
+            if (argc != optind+3) {
+                pa_log(_("You have to specify a sink name/index and a latency offset"));
+                goto quit;
+            }
+
+            sink_name = pa_xstrdup(argv[optind+1]);
+            if (pa_atol(argv[optind + 2], &latency_offset) < 0) {
+                pa_log(_("Could not parse latency offset"));
+                goto quit;
+            }
+
+        } else if (pa_streq(argv[optind], "set-source-latency-offset")) {
+            action = SET_SOURCE_LATENCY_OFFSET;
+
+            if (argc != optind+3) {
+                pa_log(_("You have to specify a source name/index and a latency offset"));
+                goto quit;
+            }
+
+            source_name = pa_xstrdup(argv[optind+1]);
+            if (pa_atol(argv[optind + 2], &latency_offset) < 0) {
+                pa_log(_("Could not parse latency offset"));
+                goto quit;
+            }
+
         } else if (pa_streq(argv[optind], "help")) {
             help(bn);
             ret = 0;

Comments

Hi Tanu, Peter,

The preceding patches are for implementing setting latency offsets of
individual sources and sinks, as discussed after the previous patch I made
a few weeks ago.

Two of the patches are trivial bugfixes:

patch 5/6 is using int64s for latency offsets in pactl and pacmd instead of
int32s, as pointed out by Peter.

Patch 1/6 is having sources update the latency offsets they inherit from a
port when the active port changes - previously they didn't do so. I assume
this is a bug, but perhaps there is some reason this was not done that I am
not aware of.

The other four patches are implementing the setting of individual latency
offsets of sinks and sources. This now plays nicely with port latency
offsets. The way I've done this is to rename everything pertaining to the
latency offsets inherited from ports, so that they are all called
sink->port_latency_offset etc. Then I've introduced a new latency offset
for sinks and sources, called just sink->latency_offset etc, which is for
the latency offset set on that sink or source alone. The total latency
reported by the device then includes the sum of both of them.

As suggested by Tanu, shell completion for pacmd and pactl has been
included, and the protocol change is in a separate commit.

The only thing I didn't do was restoring from disk. Tanu, you said this
might be tricky, but I wasn't able to understand module-device-restore
enough to even see the trickiness you were talking about, I wasn't able to
get that far! I'm not very familiar with the codebase - I'm just here
selfishly to try and get my feature in so my application works better :p.
So apologies for that, I hope the changes are worthy without it.

Please advise if there's anything else I ought to do with this, I can
refactor commits if they're split up too much or little, or whatever.

Cheers,

Chris
Hi all,

I was wondering if anyone has had the time to look at my patch?

No rush if there are more important things to be doing!

Regards,

Chris

On Sat, Jan 23, 2016 at 12:49 PM, Chris Billington <
chrisjbillington@gmail.com> wrote:

> Hi Tanu, Peter,
>
> The preceding patches are for implementing setting latency offsets of
> individual sources and sinks, as discussed after the previous patch I made
> a few weeks ago.
>
> Two of the patches are trivial bugfixes:
>
> patch 5/6 is using int64s for latency offsets in pactl and pacmd instead
> of int32s, as pointed out by Peter.
>
> Patch 1/6 is having sources update the latency offsets they inherit from a
> port when the active port changes - previously they didn't do so. I assume
> this is a bug, but perhaps there is some reason this was not done that I am
> not aware of.
>
> The other four patches are implementing the setting of individual latency
> offsets of sinks and sources. This now plays nicely with port latency
> offsets. The way I've done this is to rename everything pertaining to the
> latency offsets inherited from ports, so that they are all called
> sink->port_latency_offset etc. Then I've introduced a new latency offset
> for sinks and sources, called just sink->latency_offset etc, which is for
> the latency offset set on that sink or source alone. The total latency
> reported by the device then includes the sum of both of them.
>
> As suggested by Tanu, shell completion for pacmd and pactl has been
> included, and the protocol change is in a separate commit.
>
> The only thing I didn't do was restoring from disk. Tanu, you said this
> might be tricky, but I wasn't able to understand module-device-restore
> enough to even see the trickiness you were talking about, I wasn't able to
> get that far! I'm not very familiar with the codebase - I'm just here
> selfishly to try and get my feature in so my application works better :p.
> So apologies for that, I hope the changes are worthy without it.
>
> Please advise if there's anything else I ought to do with this, I can
> refactor commits if they're split up too much or little, or whatever.
>
> Cheers,
>
> Chris
>
On Fri, 2016-01-29 at 11:11 +1100, Chris Billington wrote:
> Hi all,
> 
> I was wondering if anyone has had the time to look at my patch?
> 
> No rush if there are more important things to be doing!

I haven't had time yet. I will get to it eventually.

> > The other four patches are implementing the setting of individual latency
> > offsets of sinks and sources. This now plays nicely with port latency
> > offsets. The way I've done this is to rename everything pertaining to the
> > latency offsets inherited from ports, so that they are all called
> > sink->port_latency_offset etc. Then I've introduced a new latency offset
> > for sinks and sources, called just sink->latency_offset etc, which is for
> > the latency offset set on that sink or source alone. The total latency
> > reported by the device then includes the sum of both of them.

Sounds very good.

> > The only thing I didn't do was restoring from disk. Tanu, you said this
> > might be tricky, but I wasn't able to understand module-device-restore
> > enough to even see the trickiness you were talking about, I wasn't able to
> > get that far! I'm not very familiar with the codebase - I'm just here
> > selfishly to try and get my feature in so my application works better :p.
> > So apologies for that, I hope the changes are worthy without it.

I think it's ok to leave the persistence feature out.

-- 
Tanu
Thanks for your enthusiasm,

Let me know when you get around to it!

-Chris

On Fri, Jan 29, 2016 at 8:39 PM, Tanu Kaskinen <tanuk@iki.fi> wrote:

> On Fri, 2016-01-29 at 11:11 +1100, Chris Billington wrote:
> > Hi all,
> >
> > I was wondering if anyone has had the time to look at my patch?
> >
> > No rush if there are more important things to be doing!
>
> I haven't had time yet. I will get to it eventually.
>
> > > The other four patches are implementing the setting of individual
> latency
> > > offsets of sinks and sources. This now plays nicely with port latency
> > > offsets. The way I've done this is to rename everything pertaining to
> the
> > > latency offsets inherited from ports, so that they are all called
> > > sink->port_latency_offset etc. Then I've introduced a new latency
> offset
> > > for sinks and sources, called just sink->latency_offset etc, which is
> for
> > > the latency offset set on that sink or source alone. The total latency
> > > reported by the device then includes the sum of both of them.
>
> Sounds very good.
>
> > > The only thing I didn't do was restoring from disk. Tanu, you said this
> > > might be tricky, but I wasn't able to understand module-device-restore
> > > enough to even see the trickiness you were talking about, I wasn't
> able to
> > > get that far! I'm not very familiar with the codebase - I'm just here
> > > selfishly to try and get my feature in so my application works better
> :p.
> > > So apologies for that, I hope the changes are worthy without it.
>
> I think it's ok to leave the persistence feature out.
>
> --
> Tanu
>