From: Mike Brady Date: Sun, 1 Apr 2018 15:09:53 +0000 (+0100) Subject: Take in track metadata but discard it if it's invalid (i.e. has a given mper ID of 0. X-Git-Tag: 3.2RC1~7^2~18 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cb69c1dd2e163a39d3a17519e51f670a9e4dbbf4;p=thirdparty%2Fshairport-sync.git Take in track metadata but discard it if it's invalid (i.e. has a given mper ID of 0. --- diff --git a/dacp.c b/dacp.c index 53aa47d7..63bfdfe8 100644 --- a/dacp.c +++ b/dacp.c @@ -187,7 +187,7 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) { // connect! // debug(1, "DACP socket created."); if (connect(sockfd, res->ai_addr, res->ai_addrlen) < 0) { - debug(3, "DACP connect failed with errno %d.",errno); + debug(3, "DACP connect failed with errno %d.", errno); response.code = 496; // Can't connect to the DACP server } else { // debug(1,"DACP connect succeeded."); @@ -367,7 +367,6 @@ void *dacp_monitor_thread_code(__attribute__((unused)) void *na) { int32_t revision_number = 1; while (1) { int result; - int changed = 0; sps_pthread_mutex_timedlock( &dacp_server_information_lock, 500000, "dacp_monitor_thread_code couldn't get DACP server information lock in 0.5 second!.", 1); @@ -461,21 +460,18 @@ void *dacp_monitor_thread_code(__attribute__((unused)) void *na) { if (metadata_store.play_status != PS_STOPPED) { metadata_store.play_status = PS_STOPPED; debug(1, "Play status is \"stopped\"."); - metadata_store.changed = 1; } break; case 3: if (metadata_store.play_status != PS_PAUSED) { metadata_store.play_status = PS_PAUSED; debug(1, "Play status is \"paused\"."); - metadata_store.changed = 1; } break; case 4: if (metadata_store.play_status != PS_PLAYING) { metadata_store.play_status = PS_PLAYING; debug(1, "Play status changed to \"playing\"."); - metadata_store.changed = 1; } break; default: @@ -491,14 +487,12 @@ void *dacp_monitor_thread_code(__attribute__((unused)) void *na) { if (metadata_store.shuffle_status != SS_OFF) { metadata_store.shuffle_status = SS_OFF; debug(1, "Shuffle status is \"off\"."); - metadata_store.changed = 1; } break; case 1: if (metadata_store.shuffle_status != SS_ON) { metadata_store.shuffle_status = SS_ON; debug(1, "Shuffle status is \"on\"."); - metadata_store.changed = 1; } break; default: @@ -514,21 +508,18 @@ void *dacp_monitor_thread_code(__attribute__((unused)) void *na) { if (metadata_store.repeat_status != RS_OFF) { metadata_store.repeat_status = RS_OFF; debug(1, "Repeat status is \"none\"."); - metadata_store.changed = 1; } break; case 1: if (metadata_store.repeat_status != RS_ONE) { metadata_store.repeat_status = RS_ONE; debug(1, "Repeat status is \"one\"."); - metadata_store.changed = 1; } break; case 2: if (metadata_store.repeat_status != RS_ALL) { metadata_store.repeat_status = RS_ALL; debug(1, "Repeat status is \"all\"."); - metadata_store.changed = 1; } break; default: @@ -610,8 +601,8 @@ void *dacp_monitor_thread_code(__attribute__((unused)) void *na) { case 'astm': t = sp - item_size; r = ntohl(*(uint32_t *)(t)); - metadata_store.songtime_in_milliseconds = ntohl(*(uint32_t *)(t)); - metadata_store.changed = 1; + if (metadata_store.track_metadata) + metadata_store.track_metadata->songtime_in_milliseconds = ntohl(*(uint32_t *)(t)); break; /* diff --git a/dbus-service.c b/dbus-service.c index cbd8c9e1..a6476027 100644 --- a/dbus-service.c +++ b/dbus-service.c @@ -21,7 +21,7 @@ ShairportSyncAdvancedRemoteControl *shairportSyncAdvancedRemoteControlSkeleton = void dbus_metadata_watcher(struct metadata_bundle *argc, __attribute__((unused)) void *userdata) { char response[100]; - char *th; + const char *th; shairport_sync_advanced_remote_control_set_volume(shairportSyncAdvancedRemoteControlSkeleton, argc->speaker_volume); @@ -149,44 +149,44 @@ void dbus_metadata_watcher(struct metadata_bundle *argc, __attribute__((unused)) } // Add the TrackID if we have one - if (argc->item_id) { + 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->item_id); + sprintf(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); } // Add the track name if there is one - if (argc->track_name) { + if ((argc->track_metadata) && (argc->track_metadata->track_name)) { // debug(1, "Track name set to \"%s\".", argc->track_name); - GVariant *trackname = g_variant_new("s", argc->track_name); + GVariant *trackname = g_variant_new("s", argc->track_metadata->track_name); g_variant_builder_add(dict_builder, "{sv}", "xesam:title", trackname); } // Add the album name if there is one - if (argc->album_name) { + if ((argc->track_metadata) && (argc->track_metadata->album_name)) { // debug(1, "Album name set to \"%s\".", argc->album_name); - GVariant *albumname = g_variant_new("s", argc->album_name); + GVariant *albumname = g_variant_new("s", argc->track_metadata->album_name); g_variant_builder_add(dict_builder, "{sv}", "xesam:album", albumname); } // Add the artists if there are any (actually there will be at most one, but put it in an array) - if (argc->artist_name) { + if ((argc->track_metadata) && (argc->track_metadata->artist_name)) { /* Build the artists array */ // debug(1,"Build artist array"); aa = g_variant_builder_new(G_VARIANT_TYPE("as")); - g_variant_builder_add(aa, "s", argc->artist_name); + g_variant_builder_add(aa, "s", argc->track_metadata->artist_name); GVariant *artists = g_variant_builder_end(aa); g_variant_builder_unref(aa); g_variant_builder_add(dict_builder, "{sv}", "xesam:artist", artists); } // Add the genres if there are any (actually there will be at most one, but put it in an array) - if (argc->genre) { + if ((argc->track_metadata) && (argc->track_metadata->genre)) { // debug(1,"Build genre"); aa = g_variant_builder_new(G_VARIANT_TYPE("as")); - g_variant_builder_add(aa, "s", argc->genre); + g_variant_builder_add(aa, "s", argc->track_metadata->genre); GVariant *genres = g_variant_builder_end(aa); g_variant_builder_unref(aa); g_variant_builder_add(dict_builder, "{sv}", "xesam:genre", genres); diff --git a/metadata_hub.c b/metadata_hub.c index 049ec0fa..b627d9f9 100644 --- a/metadata_hub.c +++ b/metadata_hub.c @@ -58,6 +58,7 @@ #endif pthread_rwlock_t metadata_hub_re_lock = PTHREAD_RWLOCK_INITIALIZER; +struct track_metadata_bundle *track_metadata; // used for a temporary track metadata store void release_char_string(char **str) { if (*str) { @@ -66,9 +67,29 @@ void release_char_string(char **str) { } } +void metadata_hub_release_track_metadata(struct track_metadata_bundle *track_metadata) { + // debug(1,"release track metadata"); + if (track_metadata) { + release_char_string(&track_metadata->track_name); + release_char_string(&track_metadata->artist_name); + release_char_string(&track_metadata->album_name); + release_char_string(&track_metadata->genre); + release_char_string(&track_metadata->comment); + release_char_string(&track_metadata->composer); + 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); + free((char *)track_metadata); + } else { + debug(1, "Releasing non-existent track metadata"); + } +} + void metadata_hub_init(void) { // debug(1, "Metadata bundle initialisation."); memset(&metadata_store, 0, sizeof(metadata_store)); + track_metadata = NULL; } void add_metadata_watcher(metadata_watcher fn, void *userdata) { @@ -98,20 +119,6 @@ void metadata_hub_release_track_artwork(void) { release_char_string(&metadata_store.cover_art_pathname); } -void metadata_hub_reset_track_metadata(void) { - // debug(1,"release track metadata"); - release_char_string(&metadata_store.track_name); - release_char_string(&metadata_store.artist_name); - release_char_string(&metadata_store.album_name); - release_char_string(&metadata_store.genre); - release_char_string(&metadata_store.comment); - release_char_string(&metadata_store.composer); - release_char_string(&metadata_store.file_kind); - release_char_string(&metadata_store.sort_as); - metadata_store.item_id = 0; - metadata_store.songtime_in_milliseconds = 0; -} - void run_metadata_watchers(void) { int i; // debug(1, "locking metadata hub for reading"); @@ -279,105 +286,85 @@ void metadata_hub_process_metadata(uint32_t type, uint32_t code, char *data, uin if (type == 'core') { switch (code) { case 'mper': - metadata_store.item_id = ntohl(*(uint32_t *)data); - debug(2, "MH Item ID set to: \"%u\"", metadata_store.item_id); + if (track_metadata) { + track_metadata->item_id = ntohl(*(uint32_t *)data); + track_metadata->item_id_received = 1; + debug(2, "MH Item ID set to: \"%u\"", track_metadata->item_id); + } else { + debug(1, "No track metadata memory allocated when item id received!"); + } break; case 'asal': - if ((metadata_store.album_name == NULL) || - (strncmp(metadata_store.album_name, data, length) != 0)) { - if (metadata_store.album_name) - free(metadata_store.album_name); - metadata_store.album_name = strndup(data, length); - debug(2, "MH Album name set to: \"%s\"", metadata_store.album_name); - metadata_store.album_name_changed = 1; - metadata_store.changed = 1; + if (track_metadata) { + track_metadata->album_name = strndup(data, length); + debug(2, "MH Album name set to: \"%s\"", track_metadata->album_name); + } else { + debug(1, "No track metadata memory allocated when album name received!"); } break; case 'asar': - if ((metadata_store.artist_name == NULL) || - (strncmp(metadata_store.artist_name, data, length) != 0)) { - if (metadata_store.artist_name) - free(metadata_store.artist_name); - metadata_store.artist_name = strndup(data, length); - debug(2, "MH Artist name set to: \"%s\"", metadata_store.artist_name); - metadata_store.artist_name_changed = 1; - metadata_store.changed = 1; + if (track_metadata) { + track_metadata->artist_name = strndup(data, length); + debug(2, "MH Artist name set to: \"%s\"", track_metadata->artist_name); + } else { + debug(1, "No track metadata memory allocated when artist name received!"); } break; case 'ascm': - if ((metadata_store.comment == NULL) || - (strncmp(metadata_store.comment, data, length) != 0)) { - if (metadata_store.comment) - free(metadata_store.comment); - metadata_store.comment = strndup(data, length); - debug(2, "MH Comment set to: \"%s\"", metadata_store.comment); - metadata_store.comment_changed = 1; - metadata_store.changed = 1; + if (track_metadata) { + track_metadata->comment = strndup(data, length); + debug(2, "MH Comment set to: \"%s\"", track_metadata->comment); + } else { + debug(1, "No track metadata memory allocated when comment received!"); } break; case 'asgn': - if ((metadata_store.genre == NULL) || (strncmp(metadata_store.genre, data, length) != 0)) { - if (metadata_store.genre) - free(metadata_store.genre); - metadata_store.genre = strndup(data, length); - debug(2, "MH Genre set to: \"%s\"", metadata_store.genre); - metadata_store.genre_changed = 1; - metadata_store.changed = 1; + if (track_metadata) { + track_metadata->genre = strndup(data, length); + debug(2, "MH Genre set to: \"%s\"", track_metadata->genre); + } else { + debug(1, "No track metadata memory allocated when genre received!"); } break; case 'minm': - if ((metadata_store.track_name == NULL) || - (strncmp(metadata_store.track_name, data, length) != 0)) { - if (metadata_store.track_name) - free(metadata_store.track_name); - metadata_store.track_name = strndup(data, length); - debug(2, "MH Track name set to: \"%s\"", metadata_store.track_name); - metadata_store.track_name_changed = 1; - metadata_store.changed = 1; + if (track_metadata) { + track_metadata->track_name = strndup(data, length); + debug(2, "MH Track name set to: \"%s\"", track_metadata->track_name); + } else { + debug(1, "No track metadata memory allocated when track name received!"); } break; case 'ascp': - if ((metadata_store.composer == NULL) || - (strncmp(metadata_store.composer, data, length) != 0)) { - if (metadata_store.composer) - free(metadata_store.composer); - metadata_store.composer = strndup(data, length); - debug(2, "MH Composer set to: \"%s\"", metadata_store.composer); - metadata_store.composer_changed = 1; - metadata_store.changed = 1; + if (track_metadata) { + track_metadata->composer = strndup(data, length); + debug(2, "MH Composer set to: \"%s\"", track_metadata->composer); + } else { + debug(1, "No track metadata memory allocated when track name received!"); } break; case 'asdt': - if ((metadata_store.file_kind == NULL) || - (strncmp(metadata_store.file_kind, data, length) != 0)) { - if (metadata_store.file_kind) - free(metadata_store.file_kind); - metadata_store.file_kind = strndup(data, length); - debug(2, "MH Song Description set to: \"%s\"", metadata_store.file_kind); - metadata_store.file_kind_changed = 1; - metadata_store.changed = 1; + if (track_metadata) { + track_metadata->song_description = strndup(data, length); + debug(2, "MH Song Description set to: \"%s\"", track_metadata->song_description); + } else { + debug(1, "No track metadata memory allocated when song description received!"); } break; case 'asaa': - if ((metadata_store.file_kind == NULL) || - (strncmp(metadata_store.file_kind, data, length) != 0)) { - if (metadata_store.file_kind) - free(metadata_store.file_kind); - metadata_store.file_kind = strndup(data, length); - debug(2, "MH File Kind set to: \"%s\"", metadata_store.file_kind); - metadata_store.file_kind_changed = 1; - metadata_store.changed = 1; + if (track_metadata) { + 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!"); } break; case 'assn': - if ((metadata_store.sort_as == NULL) || - (strncmp(metadata_store.sort_as, data, length) != 0)) { - if (metadata_store.sort_as) - free(metadata_store.sort_as); - metadata_store.sort_as = strndup(data, length); - debug(2, "MH Sort As set to: \"%s\"", metadata_store.sort_as); - metadata_store.sort_as_changed = 1; - metadata_store.changed = 1; + if (track_metadata) { + track_metadata->sort_as = strndup(data, length); + debug(2, "MH Sort As set to: \"%s\"", track_metadata->sort_as); + } else { + debug(1, + "No track metadata memory allocated when sort as (sort name) description received!"); } break; @@ -411,20 +398,42 @@ void metadata_hub_process_metadata(uint32_t type, uint32_t code, char *data, uin case 'mdst': debug(2, "MH Metadata stream processing start."); - metadata_hub_modify_prolog(); - metadata_hub_reset_track_metadata(); - metadata_hub_release_track_artwork(); + if (track_metadata) { + debug(1, "This track metadata bundle still seems to exist -- releasing it"); + metadata_hub_release_track_metadata(track_metadata); + } + track_metadata = (struct track_metadata_bundle *)malloc(sizeof(struct track_metadata_bundle)); + if (track_metadata == NULL) + die("Could not allocate memory for track metadata."); + memset(track_metadata, 0, sizeof(struct track_metadata_bundle)); // now we have a valid track + // metadata space, but the + // metadata itself + // might turin out to be invalid. Specifically, YouTube on iOS can generate a sequence of + // track metadata that is invalid. + // it is distinguished by having an item_id of zero. break; case 'mden': - metadata_hub_modify_epilog(1); + if (track_metadata) { + if ((track_metadata->item_id_received == 0) || (track_metadata->item_id != 0)) { + // i.e. it's only invalid if it has definitely been given an item_id of zero + metadata_hub_modify_prolog(); + metadata_hub_release_track_metadata(metadata_store.track_metadata); + metadata_store.track_metadata = track_metadata; + track_metadata = NULL; + metadata_hub_modify_epilog(1); + } else { + debug(1, "The track information received is invalid -- dropping it"); + metadata_hub_release_track_metadata(track_metadata); + track_metadata = NULL; + } + } debug(2, "MH Metadata stream processing end."); break; case 'PICT': if (length > 16) { metadata_hub_modify_prolog(); debug(2, "MH Picture received, length %u bytes.", length); - if (metadata_store.cover_art_pathname) - free(metadata_store.cover_art_pathname); + release_char_string(&metadata_store.cover_art_pathname); metadata_store.cover_art_pathname = metadata_write_image_file(data, length); metadata_hub_modify_epilog(1); } @@ -448,8 +457,7 @@ void metadata_hub_process_metadata(uint32_t type, uint32_t code, char *data, uin if ((metadata_store.server_ip == NULL) || (strncmp(metadata_store.server_ip, data, length) != 0)) { metadata_hub_modify_prolog(); - if (metadata_store.server_ip) - free(metadata_store.server_ip); + release_char_string(&metadata_store.server_ip); metadata_store.server_ip = strndup(data, length); // debug(1, "MH Server IP set to: \"%s\"", metadata_store.server_ip); metadata_hub_modify_epilog(1); @@ -467,7 +475,8 @@ void metadata_hub_process_metadata(uint32_t type, uint32_t code, char *data, uin metadata_hub_modify_prolog(); metadata_store.player_state = PS_STOPPED; // debug(1,"player_stop release track metadata and artwork"); - metadata_hub_reset_track_metadata(); + metadata_hub_release_track_metadata(metadata_store.track_metadata); + metadata_store.track_metadata = NULL; metadata_hub_release_track_artwork(); metadata_hub_modify_epilog(1); } break; diff --git a/metadata_hub.h b/metadata_hub.h index ce0505d8..583a012f 100644 --- a/metadata_hub.h +++ b/metadata_hub.h @@ -25,81 +25,52 @@ enum repeat_status_type { RS_ALL, } repeat_status_type; +typedef struct track_metadata_bundle { + uint32_t item_id; // seems to be a track ID -- see itemid in DACP.c + int item_id_received; // important for deciding if the track information should be ignored. + unsigned char + 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_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 + char *composer; // a malloced string -- if non-zero, free it before replacing it + 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 + uint32_t songtime_in_milliseconds; +} track_metadata_bundle; + struct metadata_bundle; typedef void (*metadata_watcher)(struct metadata_bundle *argc, void *userdata); typedef struct metadata_bundle { + char *client_ip; // IP number used by the audio source (i.e. the "client"), which is also the DACP + // server + char *server_ip; // IP number used by Shairport Sync + int dacp_server_active; // true if there's a reachable DACP server (assumed to be the Airplay // client) ; false otherwise int advanced_dacp_server_active; // true if there's a reachable DACP server with iTunes // capabilitiues // ; false otherwise - - int changed; // normally 0, nonzero if a field has been changed - int playerstatusupdates_are_received; // false if it's "traditional" metadata - - int player_thread_active; // true if there is a player threrad; false otherwise - enum play_status_type play_status; - int play_status_changed; - enum shuffle_status_type shuffle_status; - int shuffle_status_changed; - enum repeat_status_type repeat_status; - int repeat_status_changed; - - char *track_name; // a malloced string -- if non-zero, free it before replacing it - int track_name_changed; - - char *artist_name; // a malloced string -- if non-zero, free it before replacing it - int artist_name_changed; - - char *album_name; // a malloced string -- if non-zero, free it before replacing it - int album_name_changed; - - char *genre; // a malloced string -- if non-zero, free it before replacing it - int genre_changed; - char *comment; // a malloced string -- if non-zero, free it before replacing it - int comment_changed; - - char *composer; // a malloced string -- if non-zero, free it before replacing it - int composer_changed; - - char *file_kind; // a malloced string -- if non-zero, free it before replacing it - int file_kind_changed; - - char *sort_as; // a malloced string -- if non-zero, free it before replacing it - int sort_as_changed; - - char *client_ip; // IP number used by the audio source (i.e. the "client"), which is also the DACP - // server - int client_ip_changed; - - char *server_ip; // IP number used by Shairport Sync - - uint32_t item_id; // seems to be a track ID -- see itemid in DACP.c - int item_id_changed; - - uint32_t songtime_in_milliseconds; - - unsigned char - item_composite_id[16]; // seems to be nowplaying 4 ids: dbid, plid, playlistItem, itemid + struct track_metadata_bundle *track_metadata; char *cover_art_pathname; // if non-zero, it will have been assigned with malloc. - // - enum play_status_type player_state; // this is the state of the actual player itself, which can be a bit noisy. int speaker_volume; // this is the actual speaker volume, allowing for the main volume and the // speaker volume control - // int previous_speaker_volume; // this is needed to prevent a loop - int airplay_volume; metadata_watcher watchers[number_of_watchers]; // functions to call if the metadata is changed.