[3/8] zeroconf-discover: add argument 'one_per_name_type'

Submitted by Yclept Nemo on July 17, 2018, 12:25 a.m.

Details

Message ID 20180717002544.5183-4-orbisvicis@gmail.com
State New
Series "*** Overview ***"
Headers show

Commit Message

Yclept Nemo July 17, 2018, 12:25 a.m.
If 'one_per_name_type' is true, only add one tunnel per avahi service
name and type. In effect, do not add more than one sink or source per
remote pulseaudio server.
---
 src/modules/module-zeroconf-discover.c | 46 +++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/modules/module-zeroconf-discover.c b/src/modules/module-zeroconf-discover.c
index 2c49d13a..1c960983 100644
--- a/src/modules/module-zeroconf-discover.c
+++ b/src/modules/module-zeroconf-discover.c
@@ -53,6 +53,7 @@  PA_MODULE_LOAD_ONCE(true);
 static const char* const valid_modargs[] = {
     "disable_ipv4",
     "disable_ipv6",
+    "one_per_name_type",
     NULL
 };
 
@@ -74,6 +75,14 @@  struct userdata {
     pa_hashmap *tunnels;
 };
 
+static unsigned tunnel_hash_simple(const void *p) {
+    const struct tunnel *t = p;
+
+    return
+        pa_idxset_string_hash_func(t->name) +
+        pa_idxset_string_hash_func(t->type) +
+}
+
 static unsigned tunnel_hash(const void *p) {
     const struct tunnel *t = p;
 
@@ -85,6 +94,18 @@  static unsigned tunnel_hash(const void *p) {
         pa_idxset_string_hash_func(t->domain);
 }
 
+static int tunnel_compare_simple(const void *a, const void *b) {
+    const struct tunnel *ta = a, *tb = b;
+    int r;
+
+    if ((r = strcmp(ta->name, tb->name)))
+        return r;
+    if ((r = strcmp(ta->type, tb->type)))
+        return r;
+
+    return 0;
+}
+
 static int tunnel_compare(const void *a, const void *b) {
     const struct tunnel *ta = a, *tb = b;
     int r;
@@ -148,6 +169,12 @@  static void resolver_cb(
 
     tnl = tunnel_new(interface, protocol, name, type, domain);
 
+    if (pa_hashmap_get(u->tunnels, tnl)) {
+        pa_log_debug("Tunnel [%i,%i,%s,%s,%s] already mapped, skipping.",
+                     interface, protocol, name, type, domain);
+        goto finish;
+    }
+
     if (event != AVAHI_RESOLVER_FOUND)
         pa_log("Resolving of '%s' failed: %s", name, avahi_strerror(avahi_client_errno(u->client)));
     else {
@@ -295,6 +322,8 @@  static void browser_cb(
 
     if (event == AVAHI_BROWSER_NEW) {
 
+        /* Since the resolver is asynchronous and the hashmap may not yet be
+         * updated, this check must be duplicated in the resolver callback. */
         if (!pa_hashmap_get(u->tunnels, t))
             if (!(avahi_service_resolver_new(u->client, interface, protocol, name, type, domain, u->protocol, 0, resolver_cb, u)))
                 pa_log("avahi_service_resolver_new() failed: %s", avahi_strerror(avahi_client_errno(u->client)));
@@ -304,9 +333,11 @@  static void browser_cb(
          * it from the callback */
 
     } else if (event == AVAHI_BROWSER_REMOVE) {
-        struct tunnel *t2;
+        struct tunnel *t2 = pa_hashmap_get(u->tunnels, t);
 
-        if ((t2 = pa_hashmap_get(u->tunnels, t))) {
+        /* A full comparison is required even if 'one_per_name_type' is true.
+         * Yes, this is redundant if it's false. */
+        if (t2 && !tunnel_compare(t2, t)) {
             pa_module_unload_request_by_index(u->core, t2->module_index, true);
             pa_hashmap_remove(u->tunnels, t2);
             tunnel_free(t2);
@@ -417,6 +448,7 @@  int pa__init(pa_module*m) {
     struct userdata *u;
     pa_modargs *ma = NULL;
     bool disable_ipv4, disable_ipv6;
+    bool one_per_name_type;
     AvahiProtocol protocol;
     int error;
 
@@ -435,6 +467,11 @@  int pa__init(pa_module*m) {
         goto fail;
     }
 
+    if (pa_modargs_get_value_boolean(ma, "one_per_name_type", &one_per_name_type) < 0) {
+        pa_log("Failed to parse argument 'one_per_name_type'.");
+        goto fail;
+    }
+
     if (disable_ipv4 && disable_ipv6) {
         pa_log("Given both 'disable_ipv4' and 'disable_ipv6', unloading.");
         goto fail;
@@ -452,7 +489,10 @@  int pa__init(pa_module*m) {
     u->client = u->sink_browser = u->source_browser = NULL;
     u->protocol = protocol;
 
-    u->tunnels = pa_hashmap_new(tunnel_hash, tunnel_compare);
+    if (one_per_name_type)
+        u->tunnels = pa_hashmap_new(tunnel_hash_simple, tunnel_compare_simple);
+    else
+        u->tunnels = pa_hashmap_new(tunnel_hash, tunnel_compare);
 
     u->avahi_poll = pa_avahi_poll_new(m->core->mainloop);
 

Comments

Tanu Kaskinen Aug. 11, 2018, 2:05 p.m.
On Mon, 2018-07-16 at 20:25 -0400, Yclept Nemo wrote:
> If 'one_per_name_type' is true, only add one tunnel per avahi service
> name and type. In effect, do not add more than one sink or source per
> remote pulseaudio server.

Is avahi service name the address? And is service type sink or source?

What's the reason you want this feature?

> ---
>  src/modules/module-zeroconf-discover.c | 46 +++++++++++++++++++++++++++++++---
>  1 file changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/src/modules/module-zeroconf-discover.c b/src/modules/module-zeroconf-discover.c
> index 2c49d13a..1c960983 100644
> --- a/src/modules/module-zeroconf-discover.c
> +++ b/src/modules/module-zeroconf-discover.c
> @@ -53,6 +53,7 @@ PA_MODULE_LOAD_ONCE(true);
>  static const char* const valid_modargs[] = {
>      "disable_ipv4",
>      "disable_ipv6",
> +    "one_per_name_type",
>      NULL
>  };
>  
> @@ -74,6 +75,14 @@ struct userdata {
>      pa_hashmap *tunnels;
>  };
>  
> +static unsigned tunnel_hash_simple(const void *p) {
> +    const struct tunnel *t = p;
> +
> +    return
> +        pa_idxset_string_hash_func(t->name) +
> +        pa_idxset_string_hash_func(t->type) +
> +}

This doesn't build.

> +
>  static unsigned tunnel_hash(const void *p) {
>      const struct tunnel *t = p;
>  
> @@ -85,6 +94,18 @@ static unsigned tunnel_hash(const void *p) {
>          pa_idxset_string_hash_func(t->domain);
>  }
>  
> +static int tunnel_compare_simple(const void *a, const void *b) {
> +    const struct tunnel *ta = a, *tb = b;
> +    int r;
> +
> +    if ((r = strcmp(ta->name, tb->name)))
> +        return r;
> +    if ((r = strcmp(ta->type, tb->type)))
> +        return r;
> +
> +    return 0;
> +}
> +
>  static int tunnel_compare(const void *a, const void *b) {
>      const struct tunnel *ta = a, *tb = b;
>      int r;
> @@ -148,6 +169,12 @@ static void resolver_cb(
>  
>      tnl = tunnel_new(interface, protocol, name, type, domain);
>  
> +    if (pa_hashmap_get(u->tunnels, tnl)) {
> +        pa_log_debug("Tunnel [%i,%i,%s,%s,%s] already mapped, skipping.",
> +                     interface, protocol, name, type, domain);
> +        goto finish;
> +    }
> +
>      if (event != AVAHI_RESOLVER_FOUND)
>          pa_log("Resolving of '%s' failed: %s", name, avahi_strerror(avahi_client_errno(u->client)));
>      else {
> @@ -295,6 +322,8 @@ static void browser_cb(
>  
>      if (event == AVAHI_BROWSER_NEW) {
>  
> +        /* Since the resolver is asynchronous and the hashmap may not yet be
> +         * updated, this check must be duplicated in the resolver callback. */
>          if (!pa_hashmap_get(u->tunnels, t))
>              if (!(avahi_service_resolver_new(u->client, interface, protocol, name, type, domain, u->protocol, 0, resolver_cb, u)))
>                  pa_log("avahi_service_resolver_new() failed: %s", avahi_strerror(avahi_client_errno(u->client)));
> @@ -304,9 +333,11 @@ static void browser_cb(
>           * it from the callback */
>  
>      } else if (event == AVAHI_BROWSER_REMOVE) {
> -        struct tunnel *t2;
> +        struct tunnel *t2 = pa_hashmap_get(u->tunnels, t);
>  
> -        if ((t2 = pa_hashmap_get(u->tunnels, t))) {
> +        /* A full comparison is required even if 'one_per_name_type' is true.
> +         * Yes, this is redundant if it's false. */

This comment is a bit weird. It would make more sense to me if you
dropped the words "even" and "yes". You could also add an explanation
why it's redundant when one_per_name_type is false.

> +        if (t2 && !tunnel_compare(t2, t)) {
>              pa_module_unload_request_by_index(u->core, t2->module_index, true);
>              pa_hashmap_remove(u->tunnels, t2);
>              tunnel_free(t2);