From: Mike Brady Date: Sat, 8 Dec 2018 21:55:49 +0000 (+0000) Subject: Initial shot at tidying up the avahi monitoring for DACP Id port information to make... X-Git-Tag: 3.3RC0~111 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=72079adaf4dcdf77338eb2485af9e8d90e4e9d37;p=thirdparty%2Fshairport-sync.git Initial shot at tidying up the avahi monitoring for DACP Id port information to make it survive abnormal temrination of play sessions. --- diff --git a/dacp.c b/dacp.c index 31497915..4c502cfa 100644 --- a/dacp.c +++ b/dacp.c @@ -60,6 +60,7 @@ typedef struct { pthread_t dacp_monitor_thread; dacp_server_record dacp_server; +void *mdns_dacp_monitor_private_storage_pointer; // HTTP Response data/funcs (See the tinyhttp example.cpp file for more on this.) struct HttpResponse { @@ -356,14 +357,10 @@ void set_dacp_server_information(rtsp_conn_info *conn) { dacp_server.connection_family = conn->connection_ip_family; dacp_server.scope_id = conn->self_scope_id; strncpy(dacp_server.ip_string, conn->client_ip_string, INET6_ADDRSTRLEN); - debug(2, "set_dacp_server_information set IP to \"%s\" and DACP id to \"%s\".", + debug(3, "set_dacp_server_information set IP to \"%s\" and DACP id to \"%s\".", dacp_server.ip_string, dacp_server.dacp_id); - if (dacp_server.port_monitor_private_storage) // if there's is a monitor already active... - mdns_dacp_dont_monitor(dacp_server.port_monitor_private_storage); // let it go. - dacp_server.port_monitor_private_storage = - mdns_dacp_monitor(dacp_server.dacp_id); // create a new one for us if a DACP-ID is provided, - // otherwise will return a NULL + mdns_dacp_monitor_set_id(dacp_server.dacp_id); metadata_hub_modify_prolog(); int ch = metadata_store.dacp_server_active != dacp_server.scan_enable; @@ -389,16 +386,17 @@ void set_dacp_server_information(rtsp_conn_info *conn) { } dacp_server.active_remote_id = conn->dacp_active_remote; // even if the dacp_id remains the same, // the active remote will change. - debug(2, "set_dacp_server_information set active-remote id to %" PRIu32 ".", + debug(3, "set_dacp_server_information set active-remote id to %" PRIu32 ".", dacp_server.active_remote_id); pthread_cond_signal(&dacp_server_information_cv); debug_mutex_unlock(&dacp_server_information_lock, 3); } void dacp_monitor_port_update_callback(char *dacp_id, uint16_t port) { - debug(2, "dacp_monitor_port_update_callback with Remote ID \"%s\" and port number %d.", dacp_id, - port); debug_mutex_lock(&dacp_server_information_lock, 500000, 2); + debug(3, "dacp_monitor_port_update_callback with Remote ID \"%s\", target ID \"%s\" and port " + "number %d.", + dacp_id, dacp_server.dacp_id, port); if (strcmp(dacp_id, dacp_server.dacp_id) == 0) { dacp_server.port = port; if (port == 0) @@ -856,6 +854,7 @@ void dacp_monitor_start() { debug(1, "Error creating the DACP Server Information Lock Attr Destroy"); memset(&dacp_server, 0, sizeof(dacp_server_record)); + pthread_create(&dacp_monitor_thread, NULL, dacp_monitor_thread_code, NULL); } diff --git a/mdns.c b/mdns.c index d25335b1..de703975 100644 --- a/mdns.c +++ b/mdns.c @@ -96,34 +96,38 @@ void mdns_register(void) { if (config.mdns == NULL) die("Could not establish mDNS advertisement!"); + + mdns_dacp_monitor_start(); // create a dacp monitor thread } void mdns_unregister(void) { + mdns_dacp_monitor_stop(); if (config.mdns) { config.mdns->mdns_unregister(); } } -void *mdns_dacp_monitor(char *dacp_id) { - void *reply = NULL; - if ((dacp_id != NULL) && (*dacp_id != '\0')) { - if ((config.mdns) && (config.mdns->mdns_dacp_monitor)) { - reply = config.mdns->mdns_dacp_monitor(dacp_id); - if (reply == NULL) { - debug(1, "Error starting a DACP monitor."); - } - } else - debug(3, "Can't start a DACP monitor -- none registered."); - } - return reply; +void mdns_dacp_monitor_start(void) { + if ((config.mdns) && (config.mdns->mdns_dacp_monitor_start)) { + config.mdns->mdns_dacp_monitor_start(); + } else + debug(3, "Can't start a DACP monitor -- no mdns_dacp_monitor start registered."); } -void mdns_dacp_dont_monitor(void *userdata) { - if ((config.mdns) && (config.mdns->mdns_dacp_dont_monitor)) { - config.mdns->mdns_dacp_dont_monitor(userdata); +void mdns_dacp_monitor_stop() { + if ((config.mdns) && (config.mdns->mdns_dacp_monitor_stop)) { + config.mdns->mdns_dacp_monitor_stop(); } else - debug(3, "Can't stop a DACP monitor -- none registered."); + debug(3, "Can't stop a DACP monitor -- no mdns_dacp_monitor_stop registered."); } + +void mdns_dacp_monitor_set_id(const char *dacp_id) { + if ((config.mdns) && (config.mdns->mdns_dacp_monitor_set_id)) { + config.mdns->mdns_dacp_monitor_set_id(dacp_id); + } else + debug(3, "Can't set dacp_id -- no mdns_dacp_set_id registered."); +} + void mdns_ls_backends(void) { mdns_backend **b = NULL; printf("Available mDNS backends: \n"); diff --git a/mdns.h b/mdns.h index 9f96a9f7..8dc919f2 100644 --- a/mdns.h +++ b/mdns.h @@ -9,8 +9,9 @@ extern int mdns_pid; void mdns_unregister(void); void mdns_register(void); -void *mdns_dacp_monitor(char *dacp_id); -void mdns_dacp_dont_monitor(void *userdata); +void mdns_dacp_monitor_start(); +void mdns_dacp_monitor_stop(void); +void mdns_dacp_monitor_set_id(const char *dacp_id); void mdns_ls_backends(void); @@ -18,8 +19,9 @@ typedef struct { char *name; int (*mdns_register)(char *apname, int port); void (*mdns_unregister)(void); - void *(*mdns_dacp_monitor)(char *); - void (*mdns_dacp_dont_monitor)(void *); + void (*mdns_dacp_monitor_start)(); + void (*mdns_dacp_monitor_set_id)(const char *); + void (*mdns_dacp_monitor_stop)(); } mdns_backend; #ifdef CONFIG_METADATA diff --git a/mdns_avahi.c b/mdns_avahi.c index b739e716..e1abe286 100644 --- a/mdns_avahi.c +++ b/mdns_avahi.c @@ -61,6 +61,8 @@ typedef struct { char *dacp_id; } dacp_browser_struct; +dacp_browser_struct private_dbs; + // static AvahiServiceBrowser *sb = NULL; static AvahiClient *client = NULL; // static AvahiClient *service_client = NULL; @@ -90,24 +92,26 @@ static void resolve_callback(AvahiServiceResolver *r, AVAHI_GCC_UNUSED AvahiIfIn break; case AVAHI_RESOLVER_FOUND: { // char a[AVAHI_ADDRESS_STR_MAX], *t; - // debug(1, "Resolve callback: Service '%s' of type '%s' in domain '%s':", name, type, domain); - char *dacpid = strstr(name, "iTunes_Ctrl_"); - if (dacpid) { - dacpid += strlen("iTunes_Ctrl_"); - if (strcmp(dacpid, dbs->dacp_id) == 0) { - debug(3, "Client's DACP port: %u.", port); + debug(3, "Resolve callback: Service '%s' of type '%s' in domain '%s':", name, type, domain); + if (dbs->dacp_id) { + char *dacpid = strstr(name, "iTunes_Ctrl_"); + if (dacpid) { + dacpid += strlen("iTunes_Ctrl_"); + if (strcmp(dacpid, dbs->dacp_id) == 0) { + debug(1, "Client \"%s\"'s DACP port: %u.", dbs->dacp_id, port); #ifdef CONFIG_DACP_CLIENT - dacp_monitor_port_update_callback(dacpid, port); + dacp_monitor_port_update_callback(dacpid, port); #endif #ifdef CONFIG_METADATA - char portstring[20]; - memset(portstring, 0, sizeof(portstring)); - snprintf(portstring, sizeof(portstring), "%u", port); - send_ssnc_metadata('dapo', strdup(portstring), strlen(portstring), 0); + char portstring[20]; + memset(portstring, 0, sizeof(portstring)); + snprintf(portstring, sizeof(portstring), "%u", port); + send_ssnc_metadata('dapo', strdup(portstring), strlen(portstring), 0); #endif + } + } else { + debug(1, "Resolve callback: Can't see a DACP string in a DACP Record!"); } - } else { - debug(1, "Resolve callback: Can't see a DACP string in a DACP Record!"); } } } @@ -128,7 +132,7 @@ static void browse_callback(AvahiServiceBrowser *b, AvahiIfIndex interface, Avah avahi_threaded_poll_quit(tpoll); break; case AVAHI_BROWSER_NEW: - debug(3, "(Browser) NEW: service '%s' of type '%s' in domain '%s'.", name, type, domain); + // debug(1, "(Browser) NEW: service '%s' of type '%s' in domain '%s'.", name, type, domain); /* We ignore the returned resolver object. In the callback function we free it. If the server is terminated before the callback function is called the server will free @@ -144,7 +148,7 @@ static void browse_callback(AvahiServiceBrowser *b, AvahiIfIndex interface, Avah char *dacpid = strstr(name, "iTunes_Ctrl_"); if (dacpid) { dacpid += strlen("iTunes_Ctrl_"); - if (strcmp(dacpid, dbs->dacp_id) == 0) + if ((dbs->dacp_id) && (strcmp(dacpid, dbs->dacp_id) == 0)) dacp_monitor_port_update_callback(dbs->dacp_id, 0); // say the port is withdrawn } else { debug(1, "Browse callback: Can't see a DACP string in a DACP Record!"); @@ -410,26 +414,15 @@ static void avahi_unregister(void) { service_name = NULL; } -void *avahi_dacp_monitor(char *dacp_id) { - dacp_browser_struct *dbs = (dacp_browser_struct *)malloc(sizeof(dacp_browser_struct)); - - if (dbs == NULL) - die("can not allocate a dacp_browser_struct."); - - char *t = strdup(dacp_id); - if (t) - dbs->dacp_id = t; - else { - die("can not allocate a dacp_id string in dacp_browser_struct."); - } +void avahi_dacp_monitor_start(void) { + dacp_browser_struct *dbs = &private_dbs; + memset((void *)&private_dbs, 0, sizeof(dacp_browser_struct)); // create the threaded poll code int err; if (!(dbs->service_poll = avahi_threaded_poll_new())) { warn("couldn't create avahi threaded service_poll!"); - free(dbs->dacp_id); - free((char *)dbs); - return NULL; + return; } // create the service client @@ -438,63 +431,72 @@ void *avahi_dacp_monitor(char *dacp_id) { service_client_callback, (void *)dbs, &err))) { warn("couldn't create avahi service client: %s!", avahi_strerror(err)); avahi_threaded_poll_free(dbs->service_poll); - free(dbs->dacp_id); - free((char *)dbs); - return NULL; + return; } - /* Create the service browser */ - if (!(dbs->service_browser = - avahi_service_browser_new(dbs->service_client, AVAHI_IF_UNSPEC, AVAHI_PROTO_UNSPEC, - "_dacp._tcp", NULL, 0, browse_callback, (void *)dbs))) { - warn("failed to create avahi service browser: %s\n", - avahi_strerror(avahi_client_errno(dbs->service_client))); - avahi_client_free(dbs->service_client); - avahi_threaded_poll_free(dbs->service_poll); - free(dbs->dacp_id); - free((char *)dbs); - return NULL; - } // start the polling thread if (avahi_threaded_poll_start(dbs->service_poll) < 0) { warn("couldn't start avahi service_poll thread"); avahi_service_browser_free(dbs->service_browser); avahi_client_free(dbs->service_client); avahi_threaded_poll_free(dbs->service_poll); - free(dbs->dacp_id); - free((char *)dbs); - return NULL; + return; } debug(3, "Avahi DACP monitor successfully started"); - return (void *)dbs; + return; } -void avahi_dacp_dont_monitor(void *userdata) { +void avahi_dacp_monitor_set_id(const char *dacp_id) { debug(3, "avahi_dacp_dont_monitor"); - if (userdata) { - dacp_browser_struct *dbs = (dacp_browser_struct *)userdata; - // stop and dispose of everything - if (dbs->service_poll) { - avahi_threaded_poll_stop(dbs->service_poll); + dacp_browser_struct *dbs = &private_dbs; + + if (dbs->dacp_id) + free(dbs->dacp_id); + if (dacp_id == NULL) + dbs->dacp_id = NULL; + else { + char *t = strdup(dacp_id); + if (t) { + dbs->dacp_id = t; avahi_threaded_poll_lock(dbs->service_poll); if (dbs->service_browser) avahi_service_browser_free(dbs->service_browser); - if (dbs->service_client) - avahi_client_free(dbs->service_client); + + if (!(dbs->service_browser = + avahi_service_browser_new(dbs->service_client, AVAHI_IF_UNSPEC, AVAHI_PROTO_UNSPEC, + "_dacp._tcp", NULL, 0, browse_callback, (void *)dbs))) { + warn("failed to create avahi service browser: %s\n", + avahi_strerror(avahi_client_errno(dbs->service_client))); + } avahi_threaded_poll_unlock(dbs->service_poll); - avahi_threaded_poll_free(dbs->service_poll); + } else { + warn("avahi_dacp_set_id: can not allocate a dacp_id string in dacp_browser_struct."); } - free(dbs->dacp_id); - free(userdata); - debug(3, "Avahi DACP monitor successfully stopped"); - } else { - debug(1, "Avahi DACP Monitor is not running."); + debug(3, "Search for DACP ID \"%s\".", t); + } +} + +void avahi_dacp_monitor_stop() { + debug(3, "avahi_dacp_dont_monitor"); + dacp_browser_struct *dbs = &private_dbs; + // stop and dispose of everything + if (dbs->service_poll) { + avahi_threaded_poll_stop(dbs->service_poll); + avahi_threaded_poll_lock(dbs->service_poll); + if (dbs->service_browser) + avahi_service_browser_free(dbs->service_browser); + if (dbs->service_client) + avahi_client_free(dbs->service_client); + avahi_threaded_poll_unlock(dbs->service_poll); + avahi_threaded_poll_free(dbs->service_poll); } - debug(3, "avahi_dacp_dont_monitor exit"); + free(dbs->dacp_id); + debug(3, "Avahi DACP monitor successfully stopped"); } mdns_backend mdns_avahi = {.name = "avahi", .mdns_register = avahi_register, .mdns_unregister = avahi_unregister, - .mdns_dacp_monitor = avahi_dacp_monitor, - .mdns_dacp_dont_monitor = avahi_dacp_dont_monitor}; + .mdns_dacp_monitor_start = avahi_dacp_monitor_start, + .mdns_dacp_monitor_set_id = avahi_dacp_monitor_set_id, + .mdns_dacp_monitor_stop = avahi_dacp_monitor_stop}; diff --git a/mdns_dns_sd.c b/mdns_dns_sd.c index 722aadcf..95bae443 100644 --- a/mdns_dns_sd.c +++ b/mdns_dns_sd.c @@ -93,5 +93,6 @@ static void mdns_dns_sd_unregister(void) { mdns_backend mdns_dns_sd = {.name = "dns-sd", .mdns_register = mdns_dns_sd_register, .mdns_unregister = mdns_dns_sd_unregister, - .mdns_dacp_monitor = NULL, - .mdns_dacp_dont_monitor = NULL}; + .mdns_dacp_monitor_start = NULL, + .mdns_dacp_monitor_set_id = NULL, + .mdns_dacp_monitor_stop = NULL}; diff --git a/mdns_external.c b/mdns_external.c index e3a241fd..d3879ff5 100644 --- a/mdns_external.c +++ b/mdns_external.c @@ -162,11 +162,13 @@ static void kill_mdns_child(void) { mdns_backend mdns_external_avahi = {.name = "external-avahi", .mdns_register = mdns_external_avahi_register, .mdns_unregister = kill_mdns_child, - .mdns_dacp_monitor = NULL, - .mdns_dacp_dont_monitor = NULL}; + .mdns_dacp_monitor_start = NULL, + .mdns_dacp_monitor_set_id = NULL, + .mdns_dacp_monitor_stop = NULL}; mdns_backend mdns_external_dns_sd = {.name = "external-dns-sd", .mdns_register = mdns_external_dns_sd_register, .mdns_unregister = kill_mdns_child, - .mdns_dacp_monitor = NULL, - .mdns_dacp_dont_monitor = NULL}; + .mdns_dacp_monitor_start = NULL, + .mdns_dacp_monitor_set_id = NULL, + .mdns_dacp_monitor_stop = NULL}; diff --git a/mdns_tinysvcmdns.c b/mdns_tinysvcmdns.c index 238b68b2..ad6be65a 100644 --- a/mdns_tinysvcmdns.c +++ b/mdns_tinysvcmdns.c @@ -164,5 +164,6 @@ static void mdns_tinysvcmdns_unregister(void) { mdns_backend mdns_tinysvcmdns = {.name = "tinysvcmdns", .mdns_register = mdns_tinysvcmdns_register, .mdns_unregister = mdns_tinysvcmdns_unregister, - .mdns_dacp_monitor = NULL, - .mdns_dacp_dont_monitor = NULL}; + .mdns_dacp_monitor_start = NULL, + .mdns_dacp_monitor_set_id = NULL, + .mdns_dacp_monitor_stop = NULL}; diff --git a/player.c b/player.c index 429a218d..3d3e8daa 100644 --- a/player.c +++ b/player.c @@ -76,6 +76,7 @@ #endif #include "common.h" +#include "mdns.h" #include "player.h" #include "rtp.h" #include "rtsp.h" @@ -452,7 +453,7 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, in // debug(1,"Flush_rtp_timestamp is %u",flush_rtp_timestamp); // now, if a flush_rtp_timestamp has been defined and the incoming timestamp is "before" it, - // drop it… + // drop itÉ if ((conn->flush_rtp_timestamp != 0) && (actual_timestamp != conn->flush_rtp_timestamp) && (modulo_32_offset(actual_timestamp, conn->flush_rtp_timestamp) < @@ -478,7 +479,7 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, in abuf_t *abuf = 0; if (!conn->ab_synced) { - // if this is the first packet… + // if this is the first packetÉ debug(3, "syncing to seqno %u.", seqno); conn->ab_write = seqno; conn->ab_read = seqno; @@ -500,7 +501,7 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, in } if (conn->ab_write == - seqno) { // if this is the expected packet (which could be the first packet…) + seqno) { // if this is the expected packet (which could be the first packetÉ) uint64_t reception_time = get_absolute_time_in_fp(); if (conn->input_frame_rate_starting_point_is_valid == 0) { if ((conn->packet_count_since_flush >= 500) && (conn->packet_count_since_flush <= 510)) { @@ -1416,19 +1417,10 @@ void player_thread_cleanup_handler(void *arg) { } #ifdef CONFIG_DACP_CLIENT - relinquish_dacp_server_information( conn); // say it doesn't belong to this conversation thread any more... - #else - // stop watching for DACP port number stuff - // this is only used for compatability, if dacp stuff isn't enabled. - if (conn->dapo_private_storage) { - mdns_dacp_dont_monitor(conn->dapo_private_storage); - conn->dapo_private_storage = NULL; - } else { - debug(2, "DACP Monitor already stopped"); - } + mdns_dacp_monitor_set_id(NULL); // say we're not interested in following that DACP id any more #endif debug(2, "Cancelling timing, control and audio threads..."); @@ -1733,20 +1725,11 @@ void *player_thread_func(void *arg) { pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); #ifdef CONFIG_DACP_CLIENT - // debug(1, "Set dacp server info"); - // may have pthread cancellation points in it -- beware - set_dacp_server_information(conn); // this will start scanning when a port is registered by the - // code initiated by the mdns_dacp_monitor + set_dacp_server_information(conn); #else - // this is only used for compatability, if dacp stuff isn't enabled. - // start an mdns/zeroconf thread to look for DACP messages containing our DACP_ID and getting the - // port number - if (conn->dapo_private_storage) - debug(1, "DACP monitor already initialised?"); - else - // almost certainly, this has pthread cancellation points in it -- beware - conn->dapo_private_storage = mdns_dacp_monitor(conn->dacp_id); + mdns_dacp_monitor_set_id(conn->dacp_id); #endif + pthread_setcancelstate(oldState, NULL); // set the default volume to whatever it was before, as stored in the config airplay_volume