diff --git a/common.c b/common.c index 0a8eaa04..81e882ad 100644 --- a/common.c +++ b/common.c @@ -565,7 +565,7 @@ void command_set_volume(double volume) { inform("Couldn't allocate memory for set_volume argument string"); } else { memset(command_buffer, 0, command_buffer_size); - sprintf(command_buffer, "%s%f", config.cmd_set_volume, volume); + snprintf(command_buffer, command_buffer_size, "%s%f", config.cmd_set_volume, volume); // debug(1,"command_buffer is \"%s\".",command_buffer); int argC; char **argV; @@ -1004,12 +1004,18 @@ void sps_nanosleep(const time_t sec, const long nanosec) { debug(1, "Error in sps_nanosleep of %d sec and %ld nanoseconds: %d.", sec, nanosec, errno); } +char *previous_debug_message = NULL; + int sps_pthread_mutex_timedlock(pthread_mutex_t *mutex, useconds_t dally_time, const char *debugmessage, int debuglevel) { + if (previous_debug_message) + free(previous_debug_message); + previous_debug_message = strdup(debugmessage); + useconds_t time_to_wait = dally_time; int r = pthread_mutex_trylock(mutex); - while ((r) && (time_to_wait > 0)) { + while ((r == EBUSY) && (time_to_wait > 0)) { useconds_t st = time_to_wait; if (st > 20000) st = 20000; @@ -1019,8 +1025,14 @@ int sps_pthread_mutex_timedlock(pthread_mutex_t *mutex, useconds_t dally_time, } if (r != 0) { char errstr[1000]; - debug(debuglevel, "error %d: \"%s\" waiting for a mutex: \"%s\".", r, - strerror_r(r, errstr, sizeof(errstr)), debugmessage); + if (r == EBUSY) + debug(debuglevel, + "error EBUSY waiting for a mutex: \"%s\". Previous debug message was: \"%s\".", + debugmessage, previous_debug_message); + else + debug(debuglevel, + "error %d: \"%s\" waiting for a mutex: \"%s\". Previous debug message was: \"%s\".", r, + strerror_r(r, errstr, sizeof(errstr)), debugmessage, previous_debug_message); } return r; } diff --git a/dacp.c b/dacp.c index b0afb184..8f9f8fdc 100644 --- a/dacp.c +++ b/dacp.c @@ -113,8 +113,10 @@ static const struct http_funcs responseFuncs = { response_realloc, response_body, response_header, response_code, }; -static pthread_mutex_t dacp_conversation_lock = PTHREAD_MUTEX_INITIALIZER; -static pthread_mutex_t dacp_server_information_lock = PTHREAD_MUTEX_INITIALIZER; +// static pthread_mutex_t dacp_conversation_lock = PTHREAD_MUTEX_INITIALIZER; +// static pthread_mutex_t dacp_server_information_lock = PTHREAD_MUTEX_INITIALIZER; +static pthread_mutex_t dacp_conversation_lock; +static pthread_mutex_t dacp_server_information_lock; static pthread_cond_t dacp_server_information_cv = PTHREAD_COND_INITIALIZER; int dacp_send_command(const char *command, char **body, ssize_t *bodysize) { @@ -139,14 +141,14 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) { response.size = 0; response.code = 0; - char portstring[10], server[256], message[1024]; + char portstring[10], server[1024], message[1024]; memset(&portstring, 0, sizeof(portstring)); if (dacp_server.connection_family == AF_INET6) { - sprintf(server, "%s%%%u", dacp_server.ip_string, dacp_server.scope_id); + snprintf(server, sizeof(server), "%s%%%u", dacp_server.ip_string, dacp_server.scope_id); } else { strcpy(server, dacp_server.ip_string); } - sprintf(portstring, "%u", dacp_server.port); + snprintf(portstring, sizeof(portstring), "%u", dacp_server.port); // first, load up address structs with getaddrinfo(): @@ -192,9 +194,9 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) { } else { // debug(1,"DACP connect succeeded."); - sprintf(message, - "GET /ctrl-int/1/%s HTTP/1.1\r\nHost: %s:%u\r\nActive-Remote: %u\r\n\r\n", - command, dacp_server.ip_string, dacp_server.port, dacp_server.active_remote_id); + snprintf(message, sizeof(message), + "GET /ctrl-int/1/%s HTTP/1.1\r\nHost: %s:%u\r\nActive-Remote: %u\r\n\r\n", + command, dacp_server.ip_string, dacp_server.port, dacp_server.active_remote_id); // Send command // debug(1,"DACP connect message: \"%s\".",message); @@ -221,7 +223,7 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) { if (setsockopt(sockfd, SOL_SOCKET, SO_RCVTIMEO, (const char *)&tv, sizeof tv) == -1) debug(1, "Error %d setting receive timeout for DACP service.", errno); int ndata = recv(sockfd, buffer, sizeof(buffer), 0); - // debug(1,"Received %d bytes: \"%s\".",ndata,buffer); + debug(3, "Received %d bytes: \"%s\".", ndata, buffer); if (ndata <= 0) { debug(1, "dacp_send_command -- error receiving response for command \"%s\".", command); @@ -302,12 +304,22 @@ void relinquish_dacp_server_information(rtsp_conn_info *conn) { // the conversation number // Thus, we can keep the DACP port that might have previously been discovered void set_dacp_server_information(rtsp_conn_info *conn) { + debug(1, "set_dacp_server_information"); sps_pthread_mutex_timedlock( &dacp_server_information_lock, 500000, "set_dacp_server_information couldn't get DACP server information lock in 0.5 second!.", 2); + debug(1, "got lock"); dacp_server.players_connection_thread_index = conn->connection_number; - if (strcmp(conn->dacp_id, dacp_server.dacp_id) != 0) { - strncpy(dacp_server.dacp_id, conn->dacp_id, sizeof(dacp_server.dacp_id)); + debug(1, "do comparison"); + debug(1, "conn->dacp_id is \"%s\"", conn->dacp_id); + debug(1, "dacp_server.dacp_id is \"%s\"", dacp_server.dacp_id); + + if ((conn->dacp_id == NULL) || (strcmp(conn->dacp_id, dacp_server.dacp_id) != 0)) { + debug(1, "comparison failed"); + if (conn->dacp_id) + strncpy(dacp_server.dacp_id, conn->dacp_id, sizeof(dacp_server.dacp_id)); + else + dacp_server.dacp_id[0] = '\0'; dacp_server.port = 0; dacp_server.scan_enable = 0; dacp_server.connection_family = conn->connection_ip_family; @@ -319,7 +331,8 @@ void set_dacp_server_information(rtsp_conn_info *conn) { 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 + mdns_dacp_monitor(dacp_server.dacp_id); // create a new one for us if a DACP-ID is provided, + // otherwise will return a NULL metadata_hub_modify_prolog(); int ch = metadata_store.dacp_server_active != dacp_server.scan_enable; @@ -334,8 +347,9 @@ void set_dacp_server_information(rtsp_conn_info *conn) { } metadata_hub_modify_epilog(ch); } else { + debug(1, "comparison succeeded"); if (dacp_server.port) { - debug(1, "Enable scanning."); + // debug(1, "Re-enable scanning."); dacp_server.scan_enable = 1; // metadata_hub_modify_prolog(); // int ch = metadata_store.dacp_server_active != dacp_server.scan_enable; @@ -343,12 +357,14 @@ void set_dacp_server_information(rtsp_conn_info *conn) { // metadata_hub_modify_epilog(ch); } } + debug(1, "doing something else"); 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 ".", dacp_server.active_remote_id); pthread_cond_signal(&dacp_server_information_cv); pthread_mutex_unlock(&dacp_server_information_lock); + debug(1, "Done gettin' Ready"); } void dacp_monitor_port_update_callback(char *dacp_id, uint16_t port) { @@ -422,7 +438,7 @@ void *dacp_monitor_thread_code(__attribute__((unused)) void *na) { else idle_scan_count = 0; - debug(3, "Scan Result: %d, Bad Scan Count: %d, Idle Scan Count: %d.", result, bad_result_count, + debug(2, "Scan Result: %d, Bad Scan Count: %d, Idle Scan Count: %d.", result, bad_result_count, idle_scan_count); if ((bad_result_count == config.scan_max_bad_response_count) || @@ -646,7 +662,7 @@ void *dacp_monitor_thread_code(__attribute__((unused)) void *na) { char *pt = st; int it; for (it = 0; it < 16; it++) { - sprintf(pt, "%02X", metadata_store.item_composite_id[it]); + snprintf(pt, 3, "%02X", metadata_store.item_composite_id[it]); pt += 2; } *pt = 0; @@ -761,6 +777,49 @@ void *dacp_monitor_thread_code(__attribute__((unused)) void *na) { } void dacp_monitor_start() { + int rc; + pthread_mutexattr_t mta; + + rc = pthread_mutexattr_init(&mta); + if (rc) + debug(1, "Error creating the DACP Conversation Lock Mutex Att Init"); + + rc = pthread_mutexattr_settype(&mta, PTHREAD_MUTEX_ERRORCHECK); + if (rc) + debug(1, "Error creating the DACP Conversation Lock Mutex Errorcheck"); + + // rc = pthread_mutexattr_setname_np(&mta, "DACP Conversation Lock"); + // if (rc) + // debug(1,"Error creating the DACP Conversation Lock Mutex Set Name"); + + rc = pthread_mutex_init(&dacp_conversation_lock, &mta); + if (rc) + debug(1, "Error creating the DACP Conversation Lock Mutex Init"); + + rc = pthread_mutexattr_destroy(&mta); + if (rc) + debug(1, "Error creating the DACP Conversation Lock Attr Destroy"); + + rc = pthread_mutexattr_init(&mta); + if (rc) + debug(1, "Error creating the DACP Server Information Lock Mutex Att Init"); + + rc = pthread_mutexattr_settype(&mta, PTHREAD_MUTEX_ERRORCHECK); + if (rc) + debug(1, "Error creating the DACP Server Information Lock Mutex Errorcheck"); + + // rc = pthread_mutexattr_setname_np(&mta, "DACP Conversation Lock"); + // if (rc) + // debug(1,"Error creating the DACP Server Information Lock Mutex Set Name"); + + rc = pthread_mutex_init(&dacp_server_information_lock, &mta); + if (rc) + debug(1, "Error creating the DACP Server Information Lock Mutex Init"); + + rc = pthread_mutexattr_destroy(&mta); + if (rc) + 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); } @@ -821,8 +880,9 @@ int dacp_set_include_speaker_volume(int64_t machine_number, int32_t vo) { debug(2, "dacp_set_include_speaker_volume to %" PRId32 ".", vo); char message[1000]; memset(message, 0, sizeof(message)); - sprintf(message, "setproperty?include-speaker-id=%" PRId64 "&dmcp.volume=%" PRId32 "", - machine_number, vo); + snprintf(message, sizeof(message), + "setproperty?include-speaker-id=%" PRId64 "&dmcp.volume=%" PRId32 "", machine_number, + vo); debug(2, "sending \"%s\"", message); return send_simple_dacp_command(message); // should return 204 @@ -831,8 +891,8 @@ int dacp_set_include_speaker_volume(int64_t machine_number, int32_t vo) { int dacp_set_speaker_volume(int64_t machine_number, int32_t vo) { char message[1000]; memset(message, 0, sizeof(message)); - sprintf(message, "setproperty?speaker-id=%" PRId64 "&dmcp.volume=%" PRId32 "", machine_number, - vo); + snprintf(message, sizeof(message), "setproperty?speaker-id=%" PRId64 "&dmcp.volume=%" PRId32 "", + machine_number, vo); debug(2, "sending \"%s\"", message); return send_simple_dacp_command(message); // should return 204 diff --git a/dbus-service.c b/dbus-service.c index 26834e58..20150abd 100644 --- a/dbus-service.c +++ b/dbus-service.c @@ -141,8 +141,7 @@ void dbus_metadata_watcher(struct metadata_bundle *argc, __attribute__((unused)) // Make up the artwork URI if we have one if (argc->cover_art_pathname) { char artURIstring[1024]; - sprintf(artURIstring, "file://%s", argc->cover_art_pathname); - // sprintf(artURIstring,""); + snprintf(artURIstring, sizeof(artURIstring), "file://%s", argc->cover_art_pathname); // debug(1,"artURI String: \"%s\".",artURIstring); GVariant *artUrl = g_variant_new("s", artURIstring); g_variant_builder_add(dict_builder, "{sv}", "mpris:artUrl", artUrl); @@ -152,7 +151,7 @@ void dbus_metadata_watcher(struct metadata_bundle *argc, __attribute__((unused)) if ((argc->track_metadata) && (argc->track_metadata->item_id)) { char trackidstring[128]; // debug(1, "Set ID using mper ID: \"%u\".",argc->item_id); - sprintf(trackidstring, "/org/gnome/ShairportSync/mper_%u", argc->track_metadata->item_id); + snprintf(trackidstring, sizeof(trackidstring), "/org/gnome/ShairportSync/mper_%u", argc->track_metadata->item_id); GVariant *trackid = g_variant_new("o", trackidstring); g_variant_builder_add(dict_builder, "{sv}", "mpris:trackid", trackid); } @@ -733,7 +732,7 @@ static void on_dbus_name_lost(__attribute__((unused)) GDBusConnection *connectio // name, (config.dbus_service_bus_type == DBT_session) ? "session" : "system"); pid_t pid = getpid(); char interface_name[256] = ""; - sprintf(interface_name, "org.gnome.ShairportSync.i%d", pid); + snprintf(interface_name, sizeof(interface_name), "org.gnome.ShairportSync.i%d", pid); GBusType dbus_bus_type = G_BUS_TYPE_SYSTEM; if (config.dbus_service_bus_type == DBT_session) dbus_bus_type = G_BUS_TYPE_SESSION; diff --git a/mdns.c b/mdns.c index 6634d1d0..855f3c3e 100644 --- a/mdns.c +++ b/mdns.c @@ -63,7 +63,7 @@ void mdns_register(void) { char *p = mdns_service_name; int i; for (i = 0; i < 6; i++) { - sprintf(p, "%02X", config.hw_addr[i]); + snprintf(p, 3, "%02X", config.hw_addr[i]); p += 2; } *p++ = '@'; @@ -106,6 +106,9 @@ void mdns_unregister(void) { void *mdns_dacp_monitor(char *dacp_id) { void *reply = NULL; + if ((dacp_id==NULL) || (*dacp_id=='\0')) + debug(1, "A null or zero-length DACP-ID string was provided for monitoring -- nothing done."); + else { if ((config.mdns) && (config.mdns->mdns_dacp_monitor)) { reply = config.mdns->mdns_dacp_monitor(dacp_id); if (reply == NULL) { @@ -113,6 +116,7 @@ void *mdns_dacp_monitor(char *dacp_id) { } } else debug(1, "Can't start a DACP monitor -- none registered."); + } return reply; } diff --git a/mdns_avahi.c b/mdns_avahi.c index 2f948bff..f231eecd 100644 --- a/mdns_avahi.c +++ b/mdns_avahi.c @@ -95,7 +95,7 @@ static void resolve_callback(AvahiServiceResolver *r, AVAHI_GCC_UNUSED AvahiIfIn #ifdef CONFIG_METADATA char portstring[20]; memset(portstring, 0, sizeof(portstring)); - sprintf(portstring, "%u", port); + snprintf(portstring, sizeof(portstring), "%u", port); send_ssnc_metadata('dapo', strdup(portstring), strlen(portstring), 0); #endif } diff --git a/mdns_external.c b/mdns_external.c index 6f4e10f4..e3a241fd 100644 --- a/mdns_external.c +++ b/mdns_external.c @@ -85,7 +85,7 @@ static int fork_execvp(const char *file, char *const argv[]) { static int mdns_external_avahi_register(char *apname, __attribute__((unused)) int port) { char mdns_port[6]; - sprintf(mdns_port, "%d", config.port); + snprintf(mdns_port, sizeof(mdns_port), "%d", config.port); char *argvwithoutmetadata[] = { NULL, apname, config.regtype, mdns_port, MDNS_RECORD_WITHOUT_METADATA, NULL}; @@ -124,7 +124,7 @@ static int mdns_external_avahi_register(char *apname, __attribute__((unused)) in static int mdns_external_dns_sd_register(char *apname, __attribute__((unused)) int port) { char mdns_port[6]; - sprintf(mdns_port, "%d", config.port); + snprintf(mdns_port, sizeof(mdns_port), "%d", config.port); char *argvwithoutmetadata[] = { NULL, apname, config.regtype, mdns_port, MDNS_RECORD_WITHOUT_METADATA, NULL}; diff --git a/metadata_hub.c b/metadata_hub.c index 04c31b2a..ff7eef46 100644 --- a/metadata_hub.c +++ b/metadata_hub.c @@ -72,6 +72,7 @@ void metadata_hub_release_track_metadata(struct track_metadata_bundle *track_met if (track_metadata) { release_char_string(&track_metadata->track_name); release_char_string(&track_metadata->artist_name); + release_char_string(&track_metadata->album_artist_name); release_char_string(&track_metadata->album_name); release_char_string(&track_metadata->genre); release_char_string(&track_metadata->comment); @@ -79,7 +80,10 @@ void metadata_hub_release_track_metadata(struct track_metadata_bundle *track_met release_char_string(&track_metadata->file_kind); release_char_string(&track_metadata->song_description); release_char_string(&track_metadata->song_album_artist); - release_char_string(&track_metadata->sort_as); + release_char_string(&track_metadata->sort_name); + release_char_string(&track_metadata->sort_artist); + release_char_string(&track_metadata->sort_album); + release_char_string(&track_metadata->sort_composer); free((char *)track_metadata); } else { debug(3, "Asked to release non-existent track metadata"); @@ -213,7 +217,7 @@ char *metadata_write_image_file(const char *buf, int len) { char jpg[] = "jpg"; int i; for (i = 0; i < 16; i++) - sprintf(&img_md5_str[i * 2], "%02x", (uint8_t)img_md5[i]); + snprintf(&img_md5_str[i * 2], 3, "%02x", (uint8_t)img_md5[i]); // see if the file is a jpeg or a png if (strncmp(buf, "\xFF\xD8\xFF", 3) == 0) ext = jpg; @@ -327,6 +331,14 @@ void metadata_hub_process_metadata(uint32_t type, uint32_t code, char *data, uin debug(1, "No track metadata memory allocated when artist name received!"); } break; + case 'assl': + if (track_metadata) { + track_metadata->album_artist_name = strndup(data, length); + debug(2, "MH Album Artist name set to: \"%s\"", track_metadata->album_artist_name); + } else { + debug(1, "No track metadata memory allocated when album artist name received!"); + } + break; case 'ascm': if (track_metadata) { track_metadata->comment = strndup(data, length); @@ -372,21 +384,48 @@ void metadata_hub_process_metadata(uint32_t type, uint32_t code, char *data, uin track_metadata->song_album_artist = strndup(data, length); debug(2, "MH Song Album Artist set to: \"%s\"", track_metadata->song_album_artist); } else { - debug(1, "No track metadata memory allocated when song description received!"); + debug(1, "No track metadata memory allocated when song artist received!"); } break; case 'assn': if (track_metadata) { - track_metadata->sort_as = strndup(data, length); - debug(2, "MH Sort As set to: \"%s\"", track_metadata->sort_as); + track_metadata->sort_name = strndup(data, length); + debug(2, "MH Sort Name set to: \"%s\"", track_metadata->sort_name); } else { debug(1, - "No track metadata memory allocated when sort as (sort name) description received!"); + "No track metadata memory allocated when sort name description received!"); + } + break; + case 'assa': + if (track_metadata) { + track_metadata->sort_artist = strndup(data, length); + debug(2, "MH Sort Artist set to: \"%s\"", track_metadata->sort_artist); + } else { + debug(1, + "No track metadata memory allocated when sort artist description received!"); + } + break; + case 'assu': + if (track_metadata) { + track_metadata->sort_album = strndup(data, length); + debug(2, "MH Sort Album set to: \"%s\"", track_metadata->sort_album); + } else { + debug(1, + "No track metadata memory allocated when sort album description received!"); + } + break; + case 'assc': + if (track_metadata) { + track_metadata->sort_composer = strndup(data, length); + debug(2, "MH Sort Composer set to: \"%s\"", track_metadata->sort_composer); + } else { + debug(1, + "No track metadata memory allocated when sort composer description received!"); } break; - // default: - /* + default: + /* { char typestring[5]; *(uint32_t *)typestring = htonl(type); @@ -403,7 +442,9 @@ void metadata_hub_process_metadata(uint32_t type, uint32_t code, char *data, uin if (payload) free(payload); } - */ + */ + break; + } } else if (type == 'ssnc') { switch (code) { diff --git a/metadata_hub.h b/metadata_hub.h index 3ffc0dc1..29825555 100644 --- a/metadata_hub.h +++ b/metadata_hub.h @@ -32,6 +32,7 @@ typedef struct track_metadata_bundle { item_composite_id[16]; // seems to be nowplaying 4 ids: dbid, plid, playlistItem, itemid char *track_name; // a malloced string -- if non-zero, free it before replacing it char *artist_name; // a malloced string -- if non-zero, free it before replacing it + char *album_artist_name; // a malloced string -- if non-zero, free it before replacing it char *album_name; // a malloced string -- if non-zero, free it before replacing it char *genre; // a malloced string -- if non-zero, free it before replacing it char *comment; // a malloced string -- if non-zero, free it before replacing it @@ -39,7 +40,10 @@ typedef struct track_metadata_bundle { char *file_kind; // a malloced string -- if non-zero, free it before replacing it char *song_description; // a malloced string -- if non-zero, free it before replacing it char *song_album_artist; // a malloced string -- if non-zero, free it before replacing it - char *sort_as; // a malloced string -- if non-zero, free it before replacing it + char *sort_name; // a malloced string -- if non-zero, free it before replacing it + char *sort_artist; // a malloced string -- if non-zero, free it before replacing it + char *sort_album; // a malloced string -- if non-zero, free it before replacing it + char *sort_composer; // a malloced string -- if non-zero, free it before replacing it uint32_t songtime_in_milliseconds; } track_metadata_bundle; diff --git a/mpris-service.c b/mpris-service.c index 30a09e01..3d6cf96b 100644 --- a/mpris-service.c +++ b/mpris-service.c @@ -91,8 +91,7 @@ void mpris_metadata_watcher(struct metadata_bundle *argc, __attribute__((unused) // Make up the artwork URI if we have one if (argc->cover_art_pathname) { char artURIstring[1024]; - sprintf(artURIstring, "file://%s", argc->cover_art_pathname); - // sprintf(artURIstring,""); + snprintf(artURIstring, sizeof(artURIstring), "file://%s", argc->cover_art_pathname); // debug(1,"artURI String: \"%s\".",artURIstring); GVariant *artUrl = g_variant_new("s", artURIstring); g_variant_builder_add(dict_builder, "{sv}", "mpris:artUrl", artUrl); @@ -109,7 +108,7 @@ void mpris_metadata_watcher(struct metadata_bundle *argc, __attribute__((unused) for (it = 0; it < 16; it++) { if (argc->track_metadata->item_composite_id[it]) non_zero = 1; - sprintf(pt, "%02X", argc->track_metadata->item_composite_id[it]); + snprintf(pt, 3, "%02X", argc->track_metadata->item_composite_id[it]); pt += 2; } } @@ -118,13 +117,13 @@ void mpris_metadata_watcher(struct metadata_bundle *argc, __attribute__((unused) if (non_zero) { // debug(1, "Set ID using composite ID: \"0x%s\".", st); char trackidstring[1024]; - sprintf(trackidstring, "/org/gnome/ShairportSync/%s", st); + snprintf(trackidstring, sizeof(trackidstring), "/org/gnome/ShairportSync/%s", st); GVariant *trackid = g_variant_new("o", trackidstring); g_variant_builder_add(dict_builder, "{sv}", "mpris:trackid", trackid); } else if ((argc->track_metadata) && (argc->track_metadata->item_id)) { char trackidstring[128]; // debug(1, "Set ID using mper ID: \"%u\".",argc->item_id); - sprintf(trackidstring, "/org/gnome/ShairportSync/mper_%u", argc->track_metadata->item_id); + snprintf(trackidstring, sizeof(trackidstring), "/org/gnome/ShairportSync/mper_%u", argc->track_metadata->item_id); GVariant *trackid = g_variant_new("o", trackidstring); g_variant_builder_add(dict_builder, "{sv}", "mpris:trackid", trackid); } @@ -292,7 +291,7 @@ static void on_mpris_name_lost(__attribute__((unused)) GDBusConnection *connecti // name,(mpris_bus_type==G_BUS_TYPE_SESSION) ? "session" : "system"); pid_t pid = getpid(); char interface_name[256] = ""; - sprintf(interface_name, "org.mpris.MediaPlayer2.ShairportSync.i%d", pid); + snprintf(interface_name, sizeof(interface_name), "org.mpris.MediaPlayer2.ShairportSync.i%d", pid); GBusType mpris_bus_type = G_BUS_TYPE_SYSTEM; if (config.mpris_service_bus_type == DBT_session) mpris_bus_type = G_BUS_TYPE_SESSION; diff --git a/player.c b/player.c index 7560bfb4..c3b74267 100644 --- a/player.c +++ b/player.c @@ -1659,8 +1659,6 @@ static void *player_thread_func(void *arg) { player_volume(config.airplay_volume, conn); int64_t frames_to_drop = 0; // debug(1, "Play begin"); - if (play_number % 100 == 0) - debug(3, "Play frame %d.", play_number); while (!conn->player_thread_please_stop) { abuf_t *inframe = buffer_get_frame(conn); if (inframe) { @@ -1668,9 +1666,11 @@ static void *player_thread_func(void *arg) { inbuflength = inframe->length; if (inbuf) { play_number++; + if (play_number % 100 == 0) + debug(3, "Play frame %d.", play_number); conn->play_number_after_flush++; if (inframe->timestamp == 0) { - // debug(1,"Player has a supplied silent frame."); + debug(3, "Player has a supplied silent frame."); conn->last_seqno_read = (SUCCESSOR(conn->last_seqno_read) & 0xffff); // manage the packet out of sequence minder config.output->play(silence, conn->max_frames_per_packet * conn->output_sample_ratio); @@ -1685,7 +1685,9 @@ static void *player_thread_func(void *arg) { */ config.output->play(silence, conn->max_frames_per_packet * conn->output_sample_ratio); } else if (frames_to_drop) { - // debug(2,"%lld frames to drop.",frames_to_drop); + if (frames_to_drop > 3 * config.output_rate) + warn("Very large number of frames to drop: %" PRId64 "."); + debug(3, "%" PRId64 " frames to drop.", frames_to_drop); frames_to_drop -= inframe->length; if (frames_to_drop < 0) frames_to_drop = 0; @@ -1885,6 +1887,9 @@ static void *player_thread_func(void *arg) { if ((config.no_sync == 0) && (inframe->timestamp != 0) && (!conn->player_thread_please_stop) && (config.resyncthreshold > 0.0) && (abs_sync_error > config.resyncthreshold * config.output_rate)) { + if (abs_sync_error > 3 * config.output_rate) { + warn("The sync error is very large: %" PRId64 " frames."); + } sync_error_out_of_bounds++; } else { sync_error_out_of_bounds = 0; diff --git a/player.h b/player.h index 4ceabf9c..6f1a8b5b 100644 --- a/player.h +++ b/player.h @@ -141,6 +141,12 @@ typedef struct { int control_socket; // our local [server] control socket int timing_socket; // local timing socket + uint16_t remote_control_port; + uint16_t remote_timing_port; + uint16_t local_audio_port; + uint16_t local_control_port; + uint16_t local_timing_port; + int64_t latency_delayed_timestamp; // this is for debugging only... int64_t reference_timestamp; uint64_t reference_timestamp_time; diff --git a/rtp.c b/rtp.c index 25d2198a..8168de14 100644 --- a/rtp.c +++ b/rtp.c @@ -216,7 +216,7 @@ void *rtp_control_receiver(void *arg) { char *obfp = obf; int obfc; for (obfc = 0; obfc < plen; obfc++) { - sprintf(obfp, "%02X", packet[obfc]); + snprintf(obfp, 3, "%02X", packet[obfc]); obfp += 2; }; *obfp = 0; @@ -500,7 +500,7 @@ void *rtp_timing_receiver(void *arg) { char *obfp = obf; int obfc; for (obfc=0;obfcrtp_running) - die("rtp_setup called with active stream!"); + warn("rtp_setup has been called with al already-active stream -- ignored. Possible duplicate " + "SETUP call?"); + else { - debug(2, "rtp_setup: cport=%d tport=%d.", cport, tport); + debug(2, "rtp_setup: cport=%d tport=%d.", cport, tport); - // print out what we know about the client - void *client_addr = NULL, *self_addr = NULL; - // int client_port, self_port; - // char client_port_str[64]; - // char self_addr_str[64]; + // print out what we know about the client + void *client_addr = NULL, *self_addr = NULL; + // int client_port, self_port; + // char client_port_str[64]; + // char self_addr_str[64]; - conn->connection_ip_family = - remote->SAFAMILY; // keep information about the kind of ip of the client + conn->connection_ip_family = + remote->SAFAMILY; // keep information about the kind of ip of the client #ifdef AF_INET6 - if (conn->connection_ip_family == AF_INET6) { - struct sockaddr_in6 *sa6 = (struct sockaddr_in6 *)remote; - client_addr = &(sa6->sin6_addr); - // client_port = ntohs(sa6->sin6_port); - sa6 = (struct sockaddr_in6 *)local; - self_addr = &(sa6->sin6_addr); - // self_port = ntohs(sa6->sin6_port); - conn->self_scope_id = sa6->sin6_scope_id; - } + if (conn->connection_ip_family == AF_INET6) { + struct sockaddr_in6 *sa6 = (struct sockaddr_in6 *)remote; + client_addr = &(sa6->sin6_addr); + // client_port = ntohs(sa6->sin6_port); + sa6 = (struct sockaddr_in6 *)local; + self_addr = &(sa6->sin6_addr); + // self_port = ntohs(sa6->sin6_port); + conn->self_scope_id = sa6->sin6_scope_id; + } #endif - if (conn->connection_ip_family == AF_INET) { - struct sockaddr_in *sa4 = (struct sockaddr_in *)remote; - client_addr = &(sa4->sin_addr); - // client_port = ntohs(sa4->sin_port); - sa4 = (struct sockaddr_in *)local; - self_addr = &(sa4->sin_addr); - // self_port = ntohs(sa4->sin_port); - } + if (conn->connection_ip_family == AF_INET) { + struct sockaddr_in *sa4 = (struct sockaddr_in *)remote; + client_addr = &(sa4->sin_addr); + // client_port = ntohs(sa4->sin_port); + sa4 = (struct sockaddr_in *)local; + self_addr = &(sa4->sin_addr); + // self_port = ntohs(sa4->sin_port); + } - inet_ntop(conn->connection_ip_family, client_addr, conn->client_ip_string, - sizeof(conn->client_ip_string)); - inet_ntop(conn->connection_ip_family, self_addr, conn->self_ip_string, - sizeof(conn->self_ip_string)); + inet_ntop(conn->connection_ip_family, client_addr, conn->client_ip_string, + sizeof(conn->client_ip_string)); + inet_ntop(conn->connection_ip_family, self_addr, conn->self_ip_string, + sizeof(conn->self_ip_string)); - debug(2, "Set up play connection from %s to self at %s on RTSP conversation thread %d.", - conn->client_ip_string, conn->self_ip_string, conn->connection_number); + debug(2, "Set up play connection from %s to self at %s on RTSP conversation thread %d.", + conn->client_ip_string, conn->self_ip_string, conn->connection_number); - // set up a the record of the remote's control socket - struct addrinfo hints; - struct addrinfo *servinfo; + // set up a the record of the remote's control socket + struct addrinfo hints; + struct addrinfo *servinfo; - memset(&conn->rtp_client_control_socket, 0, sizeof(conn->rtp_client_control_socket)); - memset(&hints, 0, sizeof hints); - hints.ai_family = conn->connection_ip_family; - hints.ai_socktype = SOCK_DGRAM; - char portstr[20]; - snprintf(portstr, 20, "%d", cport); - if (getaddrinfo(conn->client_ip_string, portstr, &hints, &servinfo) != 0) - die("Can't get address of client's control port"); + memset(&conn->rtp_client_control_socket, 0, sizeof(conn->rtp_client_control_socket)); + memset(&hints, 0, sizeof hints); + hints.ai_family = conn->connection_ip_family; + hints.ai_socktype = SOCK_DGRAM; + char portstr[20]; + snprintf(portstr, 20, "%d", cport); + if (getaddrinfo(conn->client_ip_string, portstr, &hints, &servinfo) != 0) + die("Can't get address of client's control port"); #ifdef AF_INET6 - if (servinfo->ai_family == AF_INET6) { - memcpy(&conn->rtp_client_control_socket, servinfo->ai_addr, sizeof(struct sockaddr_in6)); - // ensure the scope id matches that of remote. this is needed for link-local addresses. - struct sockaddr_in6 *sa6 = (struct sockaddr_in6 *)&conn->rtp_client_control_socket; - sa6->sin6_scope_id = conn->self_scope_id; - } else + if (servinfo->ai_family == AF_INET6) { + memcpy(&conn->rtp_client_control_socket, servinfo->ai_addr, sizeof(struct sockaddr_in6)); + // ensure the scope id matches that of remote. this is needed for link-local addresses. + struct sockaddr_in6 *sa6 = (struct sockaddr_in6 *)&conn->rtp_client_control_socket; + sa6->sin6_scope_id = conn->self_scope_id; + } else #endif - memcpy(&conn->rtp_client_control_socket, servinfo->ai_addr, sizeof(struct sockaddr_in)); - freeaddrinfo(servinfo); + memcpy(&conn->rtp_client_control_socket, servinfo->ai_addr, sizeof(struct sockaddr_in)); + freeaddrinfo(servinfo); - // set up a the record of the remote's timing socket - memset(&conn->rtp_client_timing_socket, 0, sizeof(conn->rtp_client_timing_socket)); - memset(&hints, 0, sizeof hints); - hints.ai_family = conn->connection_ip_family; - hints.ai_socktype = SOCK_DGRAM; - snprintf(portstr, 20, "%d", tport); - if (getaddrinfo(conn->client_ip_string, portstr, &hints, &servinfo) != 0) - die("Can't get address of client's timing port"); + // set up a the record of the remote's timing socket + memset(&conn->rtp_client_timing_socket, 0, sizeof(conn->rtp_client_timing_socket)); + memset(&hints, 0, sizeof hints); + hints.ai_family = conn->connection_ip_family; + hints.ai_socktype = SOCK_DGRAM; + snprintf(portstr, 20, "%d", tport); + if (getaddrinfo(conn->client_ip_string, portstr, &hints, &servinfo) != 0) + die("Can't get address of client's timing port"); #ifdef AF_INET6 - if (servinfo->ai_family == AF_INET6) { - memcpy(&conn->rtp_client_timing_socket, servinfo->ai_addr, sizeof(struct sockaddr_in6)); - // ensure the scope id matches that of remote. this is needed for link-local addresses. - struct sockaddr_in6 *sa6 = (struct sockaddr_in6 *)&conn->rtp_client_timing_socket; - sa6->sin6_scope_id = conn->self_scope_id; - } else + if (servinfo->ai_family == AF_INET6) { + memcpy(&conn->rtp_client_timing_socket, servinfo->ai_addr, sizeof(struct sockaddr_in6)); + // ensure the scope id matches that of remote. this is needed for link-local addresses. + struct sockaddr_in6 *sa6 = (struct sockaddr_in6 *)&conn->rtp_client_timing_socket; + sa6->sin6_scope_id = conn->self_scope_id; + } else #endif - memcpy(&conn->rtp_client_timing_socket, servinfo->ai_addr, sizeof(struct sockaddr_in)); - freeaddrinfo(servinfo); + memcpy(&conn->rtp_client_timing_socket, servinfo->ai_addr, sizeof(struct sockaddr_in)); + freeaddrinfo(servinfo); - // now, we open three sockets -- one for the audio stream, one for the timing and one for the - // control + // now, we open three sockets -- one for the audio stream, one for the timing and one for the + // control + conn->remote_control_port = cport; + conn->remote_timing_port = tport; - *lsport = bind_port(conn->connection_ip_family, conn->self_ip_string, conn->self_scope_id, - &conn->audio_socket); - *lcport = bind_port(conn->connection_ip_family, conn->self_ip_string, conn->self_scope_id, - &conn->control_socket); - *ltport = bind_port(conn->connection_ip_family, conn->self_ip_string, conn->self_scope_id, - &conn->timing_socket); + conn->local_audio_port = bind_port(conn->connection_ip_family, conn->self_ip_string, + conn->self_scope_id, &conn->audio_socket); + conn->local_control_port = bind_port(conn->connection_ip_family, conn->self_ip_string, + conn->self_scope_id, &conn->control_socket); + conn->local_timing_port = bind_port(conn->connection_ip_family, conn->self_ip_string, + conn->self_scope_id, &conn->timing_socket); - debug(2, "listening for audio, control and timing on ports %d, %d, %d.", *lsport, *lcport, - *ltport); + debug(2, "listening for audio, control and timing on ports %d, %d, %d.", conn->local_audio_port, + conn->local_control_port, conn->local_timing_port); - conn->reference_timestamp = 0; - // pthread_create(&rtp_audio_thread, NULL, &rtp_audio_receiver, NULL); - // pthread_create(&rtp_control_thread, NULL, &rtp_control_receiver, NULL); - // pthread_create(&rtp_timing_thread, NULL, &rtp_timing_receiver, NULL); + conn->reference_timestamp = 0; + // pthread_create(&rtp_audio_thread, NULL, &rtp_audio_receiver, NULL); + // pthread_create(&rtp_control_thread, NULL, &rtp_control_receiver, NULL); + // pthread_create(&rtp_timing_thread, NULL, &rtp_timing_receiver, NULL); - conn->request_sent = 0; - conn->rtp_running = 1; + conn->request_sent = 0; + conn->rtp_running = 1; #ifdef CONFIG_METADATA - send_ssnc_metadata('clip', strdup(conn->client_ip_string), strlen(conn->client_ip_string), 1); - send_ssnc_metadata('svip', strdup(conn->self_ip_string), strlen(conn->self_ip_string), 1); + send_ssnc_metadata('clip', strdup(conn->client_ip_string), strlen(conn->client_ip_string), 1); + send_ssnc_metadata('svip', strdup(conn->self_ip_string), strlen(conn->self_ip_string), 1); #endif + } } void get_reference_timestamp_stuff(int64_t *timestamp, uint64_t *timestamp_time, diff --git a/rtp.h b/rtp.h index c89f7e56..743cff52 100644 --- a/rtp.h +++ b/rtp.h @@ -12,8 +12,7 @@ void *rtp_audio_receiver(void *arg); void *rtp_control_receiver(void *arg); void *rtp_timing_receiver(void *arg); -void rtp_setup(SOCKADDR *local, SOCKADDR *remote, int controlport, int timingport, - int *local_server_port, int *local_control_port, int *local_timing_port, +void rtp_setup(SOCKADDR *local, SOCKADDR *remote, uint16_t controlport, uint16_t timingport, rtsp_conn_info *conn); void rtp_request_resend(seq_t first, uint32_t count, rtsp_conn_info *conn); void rtp_request_client_pause(rtsp_conn_info *conn); // ask the client to pause diff --git a/rtsp.c b/rtsp.c index 7bc1334e..3fb6ba39 100644 --- a/rtsp.c +++ b/rtsp.c @@ -378,7 +378,7 @@ static void debug_print_msg_content(int level, rtsp_message *msg) { char *obfp = obf; int obfc; for (obfc = 0; obfc < msg->contentlength; obfc++) { - sprintf(obfp, "%02X", msg->content[obfc]); + snprintf(obfp, 3, "%02X", msg->content[obfc]); obfp += 2; }; *obfp = 0; @@ -668,7 +668,10 @@ static void msg_write_response(int fd, rtsp_message *resp) { static void handle_record(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { debug(2, "Connection %d: RECORD", conn->connection_number); - player_play(conn); // the thread better be 0 + if (conn->player_thread) + warn("Duplicate RECORD message -- ignored"); + else + player_play(conn); // the thread better be 0 resp->respcode = 200; // I think this is for telling the client what the absolute minimum latency @@ -700,7 +703,6 @@ static void handle_record(rtsp_conn_info *conn, rtsp_message *req, rtsp_message } } } - // usleep(500000); } static void handle_options(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req, @@ -724,11 +726,9 @@ static void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_m debug(3, "TEARDOWN: synchronously terminating the player thread of RTSP conversation thread %d (2).", conn->connection_number); - // if (rtsp_playing()) { player_stop(conn); debug(3, "TEARDOWN: successful termination of playing thread of RTSP conversation thread %d.", conn->connection_number); - //} } static void handle_flush(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { @@ -763,8 +763,7 @@ static void handle_flush(rtsp_conn_info *conn, rtsp_message *req, rtsp_message * static void handle_setup(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { debug(2, "Connection %d: SETUP", conn->connection_number); - int cport, tport; - int lsport, lcport, ltport; + uint16_t cport, tport; char *ar = msg_get_header(req, "Active-Remote"); if (ar) { @@ -775,61 +774,74 @@ static void handle_setup(rtsp_conn_info *conn, rtsp_message *req, rtsp_message * #ifdef CONFIG_METADATA send_metadata('ssnc', 'acre', ar, strlen(ar), req, 1); #endif + } else { + debug(2, "Note: no Active-Remote information the SETUP Record."); + conn->dacp_active_remote = 0; } ar = msg_get_header(req, "DACP-ID"); if (ar) { debug(2, "DACP-ID string seen: \"%s\".", ar); + if (conn->dacp_id) // this is in case SETUP was previously called + free(conn->dacp_id); conn->dacp_id = strdup(ar); #ifdef CONFIG_METADATA send_metadata('ssnc', 'daid', ar, strlen(ar), req, 1); #endif + } else { + debug(2, "Note: no DACP-ID string information in the SETUP Record."); + if (conn->dacp_id) // this is in case SETUP was previously called + free(conn->dacp_id); + conn->dacp_id = NULL; } char *hdr = msg_get_header(req, "Transport"); - if (!hdr) + if (!hdr) { + debug(1, "SETUP doesn't contain a Transport header."); goto error; + } char *p; p = strstr(hdr, "control_port="); - if (!p) + if (!p) { + debug(1, "SETUP doesn't specify a control_port."); goto error; + } p = strchr(p, '=') + 1; cport = atoi(p); p = strstr(hdr, "timing_port="); - if (!p) + if (!p) { + debug(1, "SETUP doesn't specify a timing_port."); goto error; + } p = strchr(p, '=') + 1; tport = atoi(p); - // rtsp_take_player(); - rtp_setup(&conn->local, &conn->remote, cport, tport, &lsport, &lcport, <port, conn); - if (!lsport) - goto error; - char *q; - p = strstr(hdr, "control_port="); - if (p) { - q = strchr(p, ';'); // get past the control port entry - *p++ = 0; - if (q++) - strcat(hdr, q); // should unsplice the control port entry + if (conn->rtp_running) { + if ((conn->remote_control_port != cport) || (conn->remote_timing_port != tport)) { + warn("Duplicate SETUP message with different control (old %u, new %u) or timing (old %u, new " + "%u) ports! This is probably fatal!", + conn->remote_control_port, cport, conn->remote_timing_port, tport); + } else { + warn("Duplicate SETUP message with the same control (%u) and timing (%u) ports. This is " + "probably not fatal.", + conn->remote_control_port, conn->remote_timing_port); + } + } else { + rtp_setup(&conn->local, &conn->remote, cport, tport, conn); } - p = strstr(hdr, "timing_port="); - if (p) { - q = strchr(p, ';'); // get past the timing port entry - *p++ = 0; - if (q++) - strcat(hdr, q); // should unsplice the timing port entry + if (conn->local_audio_port == 0) { + debug(1, "SETUP seems to specify a null audio port."); + goto error; } - char *resphdr = alloca(200); - *resphdr = 0; - sprintf(resphdr, "RTP/AVP/" - "UDP;unicast;interleaved=0-1;mode=record;control_port=%d;" - "timing_port=%d;server_" - "port=%d", - lcport, ltport, lsport); + char resphdr[256] = ""; + snprintf(resphdr, sizeof(resphdr), "RTP/AVP/" + "UDP;unicast;interleaved=0-1;mode=record;control_port=%d;" + "timing_port=%d;server_" + "port=%d", + conn->local_control_port, conn->local_timing_port, conn->local_audio_port); msg_add_header(resp, "Transport", resphdr); @@ -1328,7 +1340,7 @@ static void handle_get_parameter(__attribute__((unused)) rtsp_conn_info *conn, r char *p = malloc(128); // will be automatically deallocated with the response is deleted if (p) { resp->content = p; - resp->contentlength = sprintf(p, "\r\nvolume: %.6f\r\n", config.airplay_volume); + resp->contentlength = snprintf(p, 128, "\r\nvolume: %.6f\r\n", config.airplay_volume); } else { debug(1, "Couldn't allocate space for a response."); } @@ -1436,7 +1448,7 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag have_the_player = 1; } else if ((playing_conn) && (playing_conn->connection_number == conn->connection_number)) { // duplicate ANNOUNCE - debug(1, "Duplicate ANNOUNCE, by the look of it!"); + warn("Duplicate ANNOUNCE, by the look of it!"); have_the_player = 1; } else { int should_wait = 0; @@ -1779,7 +1791,7 @@ static int rtsp_auth(char **nonce, rtsp_message *req, rtsp_message *resp) { int i; unsigned char buf[33]; for (i = 0; i < 16; i++) - sprintf((char *)buf + 2 * i, "%02x", digest_urp[i]); + snprintf((char *)buf + 2 * i, 3, "%02x", digest_urp[i]); #ifdef HAVE_LIBSSL MD5_Init(&ctx); @@ -1788,7 +1800,7 @@ static int rtsp_auth(char **nonce, rtsp_message *req, rtsp_message *resp) { MD5_Update(&ctx, *nonce, strlen(*nonce)); MD5_Update(&ctx, ":", 1); for (i = 0; i < 16; i++) - sprintf((char *)buf + 2 * i, "%02x", digest_mu[i]); + snprintf((char *)buf + 2 * i, 3, "%02x", digest_mu[i]); MD5_Update(&ctx, buf, 32); MD5_Final(digest_total, &ctx); #endif @@ -1800,7 +1812,7 @@ static int rtsp_auth(char **nonce, rtsp_message *req, rtsp_message *resp) { mbedtls_md5_update(&tctx, (const unsigned char *)*nonce, strlen(*nonce)); mbedtls_md5_update(&tctx, (unsigned char *)":", 1); for (i = 0; i < 16; i++) - sprintf((char *)buf + 2 * i, "%02x", digest_mu[i]); + snprintf((char *)buf + 2 * i, 3, "%02x", digest_mu[i]); mbedtls_md5_update(&tctx, buf, 32); mbedtls_md5_finish(&tctx, digest_total); #endif @@ -1812,13 +1824,13 @@ static int rtsp_auth(char **nonce, rtsp_message *req, rtsp_message *resp) { md5_update(&tctx, (const unsigned char *)*nonce, strlen(*nonce)); md5_update(&tctx, (unsigned char *)":", 1); for (i = 0; i < 16; i++) - sprintf((char *)buf + 2 * i, "%02x", digest_mu[i]); + snprintf((char *)buf + 2 * i, 3, "%02x", digest_mu[i]); md5_update(&tctx, buf, 32); md5_finish(&tctx, digest_total); #endif for (i = 0; i < 16; i++) - sprintf((char *)buf + 2 * i, "%02x", digest_total[i]); + snprintf((char *)buf + 2 * i, 3, "%02x", digest_total[i]); if (!strcmp(response, (const char *)buf)) return 0;