On Fri, 2012-06-22 at 20:55 +0200, poljar (Damir Jelic) wrote:
> From: poljar <poljarinho at gmail.com>
>
> pacmd was extended so it can handle the new latency offset.
>
> A new function was added so we can set the latency also the list
> commands were extended to print the latency offset on the ports.
Thanks, I've done a few changes (see below), and I'll push this soon.
> @@ -168,6 +169,7 @@ static const struct command commands[] = {
> { "set-card-profile", pa_cli_command_card_profile, "Change the profile of a card (args: index|name, profile-name)", 3},
> { "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},
The description string is misaligned.
> { "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},
> @@ -1723,6 +1725,52 @@ static int pa_cli_command_source_port(pa_core *c, pa_tokenizer *t, pa_strbuf *bu
> return 0;
> }
>
> +static int pa_cli_command_port_offset(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, pa_bool_t *fail) {
> + const char *n, *p, *l;
> + pa_device_port *port;
> + pa_card *card;
> + double 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 card either by its name or its index.\n");
> + return -1;
> + }
> +
> + if (!(p = pa_tokenizer_get(t, 2))) {
> + pa_strbuf_puts(buf, "You need to specify a port by its name.\n");
> + return -1;
> + }
> +
> + if (!(l = pa_tokenizer_get(t, 3))) {
> + pa_strbuf_puts(buf, "You need to specify a latency offset.\n");
> + return -1;
> + }
> +
> + if (pa_atod(l, &offset) < 0) {
> + pa_strbuf_puts(buf, "Failed to parse latency.\n");
> + return -1;
> + }
I guess you use pa_atod() because there's no pa_atoi64(), and therefore
the full range of pa_usec_t is not available if you just use pa_atoi().
But the range is still roughly from -2000 seconds to 2000 seconds, which
should be enough for everybody. In my opinion it's better to accept a
reduced range than to accept floating point numbers. If you accept
floating point numbers, you should check also that the offset is between
INT64_MIN and INT64_MAX.
In conclusion: I have changed the offset parsing to use pa_atoi().
> +
> + if (!(card = pa_namereg_get(c, n, PA_NAMEREG_CARD))) {
> + pa_strbuf_puts(buf, "No card found by this name or index.\n");
> + return -1;
> + }
> +
> + if (!(port = pa_hashmap_get(card->ports, p))) {
> + pa_strbuf_puts(buf, "No port found by this name.\n");
> + return -1;
> + }
> +
> + pa_device_port_set_latency_offset(port, (pa_usec_t) offset);
> +
> + return 0;
> +}
> +
> static int pa_cli_command_dump(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, pa_bool_t *fail) {
> pa_module *m;
> pa_sink *sink;
> diff --git a/src/pulsecore/cli-text.c b/src/pulsecore/cli-text.c
> index d234b96..d4c6660 100644
> --- a/src/pulsecore/cli-text.c
> +++ b/src/pulsecore/cli-text.c
> @@ -126,8 +126,9 @@ static void append_port_list(pa_strbuf *s, pa_hashmap *ports)
> pa_strbuf_puts(s, "\tports:\n");
> PA_HASHMAP_FOREACH(p, ports, state) {
> char *t = pa_proplist_to_string_sep(p->proplist, "\n\t\t\t\t");
> - pa_strbuf_printf(s, "\t\t%s: %s (priority %u, available: %s)\n",
> - p->name, p->description, p->priority, port_available_to_string(p->available));
> + pa_strbuf_printf(s, "\t\t%s: %s (priority %u, offset %.2f usec, available: %s)\n",
> + p->name, p->description, p->priority, (double) p->latency_offset,
> + port_available_to_string(p->available));
Why cast the offset to floating point? I'd understand it if you'd change
the units to milliseconds or something, but if you don't change the
units, what's the point?
Also, I think it's better to write "latency offset" than "offset" in the
output, since I think it's not obvious to the user what just "offset"
means in this context.
--
Tanu
