[PATCH v4 BlueZ] player: Fix Track being emitted with empty metadata

* [PATCH v4 BlueZ] player: Fix Track being emitted with empty metadata
@ 2022-02-22 19:38 Luiz Augusto von Dentz
  2022-02-22 21:38 ` [v4,BlueZ] " bluez.test.bot
  0 siblings, 1 reply; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2022-02-22 19:38 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This sometimes causes the Track to be schedule while some metadata are
still pending, also don't remove the Duration from track when updating
its metadata since Duration is typically updated by player status rather
than metadata.

Fixes: https://github.com/bluez/bluez/issues/291
---
 profiles/audio/avrcp.c  |  4 +++
 profiles/audio/player.c | 74 ++++++++++++++++++++++-------------------
 profiles/audio/player.h |  2 ++
 3 files changed, 45 insertions(+), 35 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 5eb182b76..80f34c7a7 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -2402,6 +2402,8 @@ static void avrcp_parse_attribute_list(struct avrcp_player *player,
 	struct media_item *item;
 	int i;
 
+	media_player_clear_metadata(mp);
+
 	item = media_player_set_playlist_item(mp, player->uid);
 
 	for (i = 0; count > 0; count--) {
@@ -2428,6 +2430,8 @@ static void avrcp_parse_attribute_list(struct avrcp_player *player,
 
 		i += len;
 	}
+
+	media_player_metadata_changed(mp);
 }
 
 static gboolean avrcp_get_element_attributes_rsp(struct avctp *conn,
diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index d34b39168..c995697fe 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -85,7 +85,6 @@ struct media_player {
 	char			*status;
 	uint32_t		position;
 	GTimer			*progress;
-	guint			process_id;
 	struct player_callback	*cb;
 	GSList			*pending;
 	GSList			*folders;
@@ -1233,9 +1232,6 @@ void media_player_destroy(struct media_player *mp)
 	if (mp->settings)
 		g_hash_table_unref(mp->settings);
 
-	if (mp->process_id > 0)
-		g_source_remove(mp->process_id);
-
 	if (mp->scope)
 		g_dbus_unregister_interface(btd_get_dbus_connection(),
 						mp->path,
@@ -1308,9 +1304,10 @@ void media_player_set_duration(struct media_player *mp, uint32_t duration)
 
 	g_hash_table_replace(mp->track, g_strdup("Duration"), value);
 
-	g_dbus_emit_property_changed(btd_get_dbus_connection(),
+	g_dbus_emit_property_changed_full(btd_get_dbus_connection(),
 					mp->path, MEDIA_PLAYER_INTERFACE,
-					"Track");
+					"Track",
+					G_DBUS_PROPERTY_CHANGED_FLAG_FLUSH);
 }
 
 void media_player_set_position(struct media_player *mp, uint32_t position)
@@ -1395,26 +1392,20 @@ void media_player_set_status(struct media_player *mp, const char *status)
 	g_timer_start(mp->progress);
 }
 
-static gboolean process_metadata_changed(void *user_data)
+static gboolean remove_metadata(void *key, void *value, void *user_data)
 {
-	struct media_player *mp = user_data;
-	const char *item;
-
-	mp->process_id = 0;
-
-	g_dbus_emit_property_changed(btd_get_dbus_connection(),
-					mp->path, MEDIA_PLAYER_INTERFACE,
-					"Track");
-
-	item = g_hash_table_lookup(mp->track, "Item");
-	if (item == NULL)
+	if (!strcmp(key, "Duration"))
 		return FALSE;
 
-	g_dbus_emit_property_changed(btd_get_dbus_connection(),
-					item, MEDIA_ITEM_INTERFACE,
-					"Metadata");
+	return strcmp(key, "Item") ? TRUE : FALSE;
+}
+
+void media_player_clear_metadata(struct media_player *mp)
+{
+	if (!mp)
+		return;
 
-	return FALSE;
+	g_hash_table_foreach_remove(mp->track, remove_metadata, NULL);
 }
 
 void media_player_set_metadata(struct media_player *mp,
@@ -1433,14 +1424,31 @@ void media_player_set_metadata(struct media_player *mp,
 		return;
 	}
 
-	if (mp->process_id == 0) {
-		g_hash_table_remove_all(mp->track);
-		mp->process_id = g_idle_add(process_metadata_changed, mp);
-	}
-
 	g_hash_table_replace(mp->track, g_strdup(key), value);
 }
 
+void media_player_metadata_changed(struct media_player *mp)
+{
+	char *item;
+
+	if (!mp)
+		return;
+
+	g_dbus_emit_property_changed_full(btd_get_dbus_connection(),
+					mp->path, MEDIA_PLAYER_INTERFACE,
+					"Track",
+					G_DBUS_PROPERTY_CHANGED_FLAG_FLUSH);
+
+	item = g_hash_table_lookup(mp->track, "Item");
+	if (item == NULL)
+		return;
+
+	g_dbus_emit_property_changed_full(btd_get_dbus_connection(),
+					item, MEDIA_ITEM_INTERFACE,
+					"Metadata",
+					G_DBUS_PROPERTY_CHANGED_FLAG_FLUSH);
+}
+
 void media_player_set_type(struct media_player *mp, const char *type)
 {
 	if (g_strcmp0(mp->type, type) == 0)
@@ -1962,6 +1970,7 @@ struct media_item *media_player_set_playlist_item(struct media_player *mp,
 {
 	struct media_folder *folder = mp->playlist;
 	struct media_item *item;
+	char *path;
 
 	DBG("%" PRIu64 "", uid);
 
@@ -1980,16 +1989,11 @@ struct media_item *media_player_set_playlist_item(struct media_player *mp,
 		mp->track = g_hash_table_ref(item->metadata);
 	}
 
-	if (item == g_hash_table_lookup(mp->track, "Item"))
+	path = g_hash_table_lookup(mp->track, "Item");
+	if (path && !strcmp(path, item->path))
 		return item;
 
-	if (mp->process_id == 0) {
-		g_hash_table_remove_all(mp->track);
-		mp->process_id = g_idle_add(process_metadata_changed, mp);
-	}
-
-	g_hash_table_replace(mp->track, g_strdup("Item"),
-						g_strdup(item->path));
+	g_hash_table_replace(mp->track, g_strdup("Item"), g_strdup(item->path));
 
 	return item;
 }
diff --git a/profiles/audio/player.h b/profiles/audio/player.h
index 69f08735b..74fb7d771 100644
--- a/profiles/audio/player.h
+++ b/profiles/audio/player.h
@@ -70,9 +70,11 @@ void media_player_set_setting(struct media_player *mp, const char *key,
 							const char *value);
 const char *media_player_get_status(struct media_player *mp);
 void media_player_set_status(struct media_player *mp, const char *status);
+void media_player_clear_metadata(struct media_player *mp);
 void media_player_set_metadata(struct media_player *mp,
 				struct media_item *item, const char *key,
 				void *data, size_t len);
+void media_player_metadata_changed(struct media_player *mp);
 void media_player_set_type(struct media_player *mp, const char *type);
 void media_player_set_subtype(struct media_player *mp, const char *subtype);
 void media_player_set_name(struct media_player *mp, const char *name);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread