[PATCH BlueZ 1/3] audio/media: Fix notifying settings changed incorrectly

[PATCH BlueZ 1/3] audio/media: Fix notifying settings changed incorrectly

[Luiz Augusto von Dentz]

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

We should notify only the setting that has changed not all of them.
---
 profiles/audio/avrcp.c | 30 +++++++++++++-----------------
 profiles/audio/avrcp.h |  3 ++-
 profiles/audio/media.c |  7 +------
 3 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 730f061..f0554fe 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -612,13 +612,15 @@ static int play_status_to_val(const char *status)
 	return -EINVAL;
 }
 
-void avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data)
+void avrcp_player_event(struct avrcp_player *player, uint8_t id,
+							const void *data)
 {
 	uint8_t buf[AVRCP_HEADER_LENGTH + 9];
 	struct avrcp_header *pdu = (void *) buf;
 	uint16_t size;
 	GSList *l;
-	GList *settings;
+	int attr;
+	int val;
 
 	if (player->sessions == NULL)
 		return;
@@ -649,24 +651,18 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data)
 		break;
 	case AVRCP_EVENT_SETTINGS_CHANGED:
 		size = 2;
-		settings = data;
-		pdu->params[1] = g_list_length(settings);
-		for (; settings; settings = settings->next) {
-			const char *key = settings->data;
-			int attr;
-			int val;
+		pdu->params[1] = 1;
 
-			attr = attr_to_val(key);
-			if (attr < 0)
-				continue;
+		attr = attr_to_val(data);
+		if (attr < 0)
+			return;
 
-			val = player_get_setting(player, attr);
-			if (val < 0)
-				continue;
+		val = player_get_setting(player, attr);
+		if (val < 0)
+			return;
 
-			pdu->params[++size] = attr;
-			pdu->params[++size] = val;
-		}
+		pdu->params[++size] = attr;
+		pdu->params[++size] = val;
 		break;
 	default:
 		error("Unknown event %u", id);
diff --git a/profiles/audio/avrcp.h b/profiles/audio/avrcp.h
index 3b1963f..6a435e6 100644
--- a/profiles/audio/avrcp.h
+++ b/profiles/audio/avrcp.h
@@ -115,7 +115,8 @@ struct avrcp_player *avrcp_register_player(struct btd_adapter *adapter,
 						GDestroyNotify destroy);
 void avrcp_unregister_player(struct avrcp_player *player);
 
-void avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data);
+void avrcp_player_event(struct avrcp_player *player, uint8_t id,
+							const void *data);
 
 
 size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands);
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index eb5ea81..45dfe53 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -1470,7 +1470,6 @@ static gboolean set_property(struct media_player *mp, const char *key,
 							const char *value)
 {
 	const char *curval;
-	GList *settings;
 
 	curval = g_hash_table_lookup(mp->settings, key);
 	if (g_strcmp0(curval, value) == 0)
@@ -1480,11 +1479,7 @@ static gboolean set_property(struct media_player *mp, const char *key,
 
 	g_hash_table_replace(mp->settings, g_strdup(key), g_strdup(value));
 
-	settings = list_settings(mp);
-
-	avrcp_player_event(mp->player, AVRCP_EVENT_SETTINGS_CHANGED, settings);
-
-	g_list_free(settings);
+	avrcp_player_event(mp->player, AVRCP_EVENT_SETTINGS_CHANGED, key);
 
 	return TRUE;
 }
-- 
1.8.1.4

[PATCH BlueZ 2/3] audio/AVRCP: Fix invalid response to RegisterNotification

[Luiz Augusto von Dentz]

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

The response to RegisterNotification for event settings changed was
not setting the initial length properly which cause the code to send
malformed/invalid PDUs.
---
 profiles/audio/avrcp.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index f0554fe..f028da9 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1450,20 +1450,25 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
 		len = 1;
 		break;
 	case AVRCP_EVENT_SETTINGS_CHANGED:
+		len = 1;
 		settings = player_list_settings(player);
 
-		pdu->params[++len] = g_list_length(settings);
+		pdu->params[len++] = g_list_length(settings);
 		for (; settings; settings = settings->next) {
 			const char *key = settings->data;
-			uint8_t attr = attr_to_val(key);
+			int attr;
 			int val;
 
+			attr = attr_to_val(key);
+			if (attr < 0)
+				continue;
+
 			val = player_get_setting(player, attr);
 			if (val < 0)
 				continue;
 
-			pdu->params[++len] = attr;
-			pdu->params[++len] = val;
+			pdu->params[len++] = attr;
+			pdu->params[len++] = val;
 		}
 
 		break;
-- 
1.8.1.4

[PATCH BlueZ 3/3] audio/media: Fix setting player settings

[Luiz Augusto von Dentz]

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

The value has to be converted to MPRIS setting otherwise the player won't
recognize it and will probably discard the change.
---
 profiles/audio/avrcp.c |  4 ++--
 profiles/audio/media.c | 54 +++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index f028da9..ffc6415 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -661,8 +661,8 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
 		if (val < 0)
 			return;
 
-		pdu->params[++size] = attr;
-		pdu->params[++size] = val;
+		pdu->params[size++] = attr;
+		pdu->params[size++] = val;
 		break;
 	default:
 		error("Unknown event %u", id);
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 45dfe53..d4d82cf 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -1007,12 +1007,54 @@ static const char *get_setting(const char *key, void *user_data)
 	return g_hash_table_lookup(mp->settings, key);
 }
 
+static void set_shuffle_setting(DBusMessageIter *iter, const char *value)
+{
+	const char *key = "Shuffle";
+	dbus_bool_t val;
+	DBusMessageIter var;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &key);
+	dbus_message_iter_open_container(iter, DBUS_TYPE_VARIANT,
+						DBUS_TYPE_BOOLEAN_AS_STRING,
+						&var);
+	val = strcasecmp(value, "off") != 0;
+	dbus_message_iter_append_basic(&var, DBUS_TYPE_BOOLEAN, &val);
+	dbus_message_iter_close_container(iter, &var);
+}
+
+static const char *repeat_to_loop_status(const char *value)
+{
+	if (strcasecmp(value, "off") == 0)
+		return "None";
+	else if (strcasecmp(value, "singletrack") == 0)
+		return "Track";
+	else if (strcasecmp(value, "alltracks") == 0)
+		return "Playlist";
+
+	return NULL;
+}
+
+static void set_repeat_setting(DBusMessageIter *iter, const char *value)
+{
+	const char *key = "LoopStatus";
+	const char *val;
+	DBusMessageIter var;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &key);
+	dbus_message_iter_open_container(iter, DBUS_TYPE_VARIANT,
+						DBUS_TYPE_STRING_AS_STRING,
+						&var);
+	val = repeat_to_loop_status(value);
+	dbus_message_iter_append_basic(&var, DBUS_TYPE_STRING, &val);
+	dbus_message_iter_close_container(iter, &var);
+}
+
 static int set_setting(const char *key, const char *value, void *user_data)
 {
 	struct media_player *mp = user_data;
 	const char *iface = MEDIA_PLAYER_INTERFACE;
 	DBusMessage *msg;
-	DBusMessageIter iter, var;
+	DBusMessageIter iter;
 
 	DBG("%s = %s", key, value);
 
@@ -1028,13 +1070,11 @@ static int set_setting(const char *key, const char *value, void *user_data)
 
 	dbus_message_iter_init_append(msg, &iter);
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &iface);
-	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &key);
 
-	dbus_message_iter_open_container(&iter, DBUS_TYPE_VARIANT,
-						DBUS_TYPE_STRING_AS_STRING,
-						&var);
-	dbus_message_iter_append_basic(&var, DBUS_TYPE_STRING, &value);
-	dbus_message_iter_close_container(&iter, &var);
+	if (strcasecmp(key, "Shuffle") == 0)
+		set_shuffle_setting(&iter, value);
+	else if (strcasecmp(key, "Repeat") == 0)
+		set_repeat_setting(&iter, value);
 
 	g_dbus_send_message(btd_get_dbus_connection(), msg);
 
-- 
1.8.1.4

Re: [PATCH BlueZ 1/3] audio/media: Fix notifying settings changed incorrectly

[Johan Hedberg]

Hi Luiz,

On Tue, Jun 18, 2013, Luiz Augusto von Dentz wrote:
> We should notify only the setting that has changed not all of them.
> ---
>  profiles/audio/avrcp.c | 30 +++++++++++++-----------------
>  profiles/audio/avrcp.h |  3 ++-
>  profiles/audio/media.c |  7 +------
>  3 files changed, 16 insertions(+), 24 deletions(-)

All three patches have been applied. Thanks.

Johan