diff mbox series

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

Message ID 20220222193855.301757-1-luiz.dentz@gmail.com
State New
Headers show
Series [v4,BlueZ] player: Fix Track being emitted with empty metadata | expand

Commit Message

Luiz Augusto von Dentz Feb. 22, 2022, 7:38 p.m. UTC 
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(-)

Comments

Luiz Augusto von Dentz Feb. 23, 2022, 1:50 a.m. UTC | #1 
Hi,

On Tue, Feb 22, 2022 at 1:39 PM <bluez.test.bot@gmail.com> wrote:
>
> This is automated email and please do not reply to this email!
>
> Dear submitter,
>
> Thank you for submitting the patches to the linux bluetooth mailing list.
> This is a CI test results with your patch series:
> PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=616863
>
> ---Test result---
>
> Test Summary:
> CheckPatch                    PASS      0.70 seconds
> GitLint                       PASS      0.38 seconds
> Prep - Setup ELL              PASS      50.46 seconds
> Build - Prep                  PASS      0.61 seconds
> Build - Configure             PASS      9.83 seconds
> Build - Make                  PASS      1752.78 seconds
> Make Check                    PASS      12.36 seconds
> Make Check w/Valgrind         PASS      533.80 seconds
> Make Distcheck                PASS      276.34 seconds
> Build w/ext ELL - Configure   PASS      10.04 seconds
> Build w/ext ELL - Make        PASS      1725.46 seconds
> Incremental Build with patchesPASS      0.00 seconds
>
>
>
> ---
> Regards,
> Linux Bluetooth
>

Pushed.
diff mbox series

Patch

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