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

[Tanu Kaskinen]

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);

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk