diff mbox series

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

Message ID 20220222193855.301757-1-luiz.dentz@gmail.com (mailing list archive)
State Accepted
Commit 2e4627c3c92ed823cb976b0a48d5463c2b187fec
Headers show
Series [v4,BlueZ] player: Fix Track being emitted with empty metadata | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/checkpatch success Checkpatch PASS
tedd_an/gitlint success Gitlint PASS
tedd_an/setupell success Setup ELL PASS
tedd_an/buildprep success Build Prep PASS
tedd_an/build success Build Configuration PASS
tedd_an/makecheck success Make Check PASS
tedd_an/makecheckvalgrind success Make Check PASS
tedd_an/makedistcheck success Make Distcheck PASS
tedd_an/build_extell success Build External ELL PASS
tedd_an/build_extell_make success Build Make with External ELL PASS

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

bluez.test.bot@gmail.com Feb. 22, 2022, 9:38 p.m. UTC | #1 
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
Luiz Augusto von Dentz Feb. 23, 2022, 1:50 a.m. UTC | #2 
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);