[PATCH ] audio/player: Fix player name value for MPRIS

[PATCH ] audio/player: Fix player name value for MPRIS

[Bharat Panda]

Added fix for setting media player name when the player
Identity value is invalid.
---
 profiles/audio/media.c | 15 +++++++++++++--
 tools/mpris-proxy.c    |  1 +
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 69070bf..68111e6 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -1805,7 +1805,7 @@ static DBusMessage *register_player(DBusConnection *conn, DBusMessage *msg,
 	struct media_adapter *adapter = data;
 	struct media_player *mp;
 	DBusMessageIter args;
-	const char *sender, *path;
+	const char *sender, *path, *name;
 	int err;
 
 	sender = dbus_message_get_sender(msg);
@@ -1814,6 +1814,8 @@ static DBusMessage *register_player(DBusConnection *conn, DBusMessage *msg,
 
 	dbus_message_iter_get_basic(&args, &path);
 	dbus_message_iter_next(&args);
+	dbus_message_iter_get_basic(&args, &name);
+	dbus_message_iter_next(&args);
 
 	if (media_adapter_find_player(adapter, sender, path) != NULL)
 		return btd_error_already_exists(msg);
@@ -1831,6 +1833,14 @@ static DBusMessage *register_player(DBusConnection *conn, DBusMessage *msg,
 		return btd_error_invalid_args(msg);
 	}
 
+	/*
+	 * The Player Identity value is NULL in MPRIS case, so create
+	 * mpris name on the basis of player bus name and assign
+	 * the value.
+	 */
+	if (mp->name == NULL)
+		mp->name = g_strdup(name);
+
 	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }
 
@@ -1864,7 +1874,8 @@ static const GDBusMethodTable media_methods[] = {
 	{ GDBUS_METHOD("UnregisterEndpoint",
 		GDBUS_ARGS({ "endpoint", "o" }), NULL, unregister_endpoint) },
 	{ GDBUS_METHOD("RegisterPlayer",
-		GDBUS_ARGS({ "player", "o" }, { "properties", "a{sv}" }),
+		GDBUS_ARGS({ "player", "o" }, { "name", "s" },
+						{ "properties", "a{sv}" }),
 		NULL, register_player) },
 	{ GDBUS_METHOD("UnregisterPlayer",
 		GDBUS_ARGS({ "player", "o" }), NULL, unregister_player) },
diff --git a/tools/mpris-proxy.c b/tools/mpris-proxy.c
index bf8148f..cb94f2b 100644
--- a/tools/mpris-proxy.c
+++ b/tools/mpris-proxy.c
@@ -466,6 +466,7 @@ static void add_player(DBusConnection *conn, const char *name,
 	dbus_message_iter_init_append(msg, &iter);
 
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, &path);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &name);
 
 	if (player != NULL) {
 		if (!g_dbus_get_properties(player->conn,
-- 
1.9.1

RE: [PATCH ] audio/player: Fix player name value for MPRIS

[Bharat Bhusan Panda]

ping

> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Bharat Panda
> Sent: Monday, October 12, 2015 2:11 PM
> To: linux-bluetooth@vger.kernel.org
> Cc: cpgs@samsung.com; Bharat Panda
> Subject: [PATCH ] audio/player: Fix player name value for MPRIS
> 
> Added fix for setting media player name when the player Identity value is
> invalid.
> ---
>  profiles/audio/media.c | 15 +++++++++++++--
>  tools/mpris-proxy.c    |  1 +
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/profiles/audio/media.c b/profiles/audio/media.c index
> 69070bf..68111e6 100644
> --- a/profiles/audio/media.c
> +++ b/profiles/audio/media.c
> @@ -1805,7 +1805,7 @@ static DBusMessage
> *register_player(DBusConnection *conn, DBusMessage *msg,
>  	struct media_adapter *adapter = data;
>  	struct media_player *mp;
>  	DBusMessageIter args;
> -	const char *sender, *path;
> +	const char *sender, *path, *name;
>  	int err;
> 
>  	sender = dbus_message_get_sender(msg); @@ -1814,6 +1814,8 @@
> static DBusMessage *register_player(DBusConnection *conn, DBusMessage
> *msg,
> 
>  	dbus_message_iter_get_basic(&args, &path);
>  	dbus_message_iter_next(&args);
> +	dbus_message_iter_get_basic(&args, &name);
> +	dbus_message_iter_next(&args);
> 
>  	if (media_adapter_find_player(adapter, sender, path) != NULL)
>  		return btd_error_already_exists(msg); @@ -1831,6 +1833,14
> @@ static DBusMessage *register_player(DBusConnection *conn,
> DBusMessage *msg,
>  		return btd_error_invalid_args(msg);
>  	}
> 
> +	/*
> +	 * The Player Identity value is NULL in MPRIS case, so create
> +	 * mpris name on the basis of player bus name and assign
> +	 * the value.
> +	 */
> +	if (mp->name == NULL)
> +		mp->name = g_strdup(name);
> +
>  	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);  }
> 
> @@ -1864,7 +1874,8 @@ static const GDBusMethodTable media_methods[]
> = {
>  	{ GDBUS_METHOD("UnregisterEndpoint",
>  		GDBUS_ARGS({ "endpoint", "o" }), NULL,
> unregister_endpoint) },
>  	{ GDBUS_METHOD("RegisterPlayer",
> -		GDBUS_ARGS({ "player", "o" }, { "properties", "a{sv}" }),
> +		GDBUS_ARGS({ "player", "o" }, { "name", "s" },
> +						{ "properties", "a{sv}" }),
>  		NULL, register_player) },
>  	{ GDBUS_METHOD("UnregisterPlayer",
>  		GDBUS_ARGS({ "player", "o" }), NULL, unregister_player) },
> diff --git a/tools/mpris-proxy.c b/tools/mpris-proxy.c index
bf8148f..cb94f2b
> 100644
> --- a/tools/mpris-proxy.c
> +++ b/tools/mpris-proxy.c
> @@ -466,6 +466,7 @@ static void add_player(DBusConnection *conn, const
> char *name,
>  	dbus_message_iter_init_append(msg, &iter);
> 
>  	dbus_message_iter_append_basic(&iter,
> DBUS_TYPE_OBJECT_PATH, &path);
> +	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING,
> &name);
> 
>  	if (player != NULL) {
>  		if (!g_dbus_get_properties(player->conn,
> --
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
in
> the body of a message to majordomo@vger.kernel.org More majordomo
> info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH ] audio/player: Fix player name value for MPRIS

[Simon Fels]

On 20.10.2015 15:05, Bharat Bhusan Panda wrote:
> ping
>
>> -----Original Message-----
>> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
>> owner@vger.kernel.org] On Behalf Of Bharat Panda
>> Sent: Monday, October 12, 2015 2:11 PM
>> To: linux-bluetooth@vger.kernel.org
>> Cc: cpgs@samsung.com; Bharat Panda
>> Subject: [PATCH ] audio/player: Fix player name value for MPRIS
>>
>> Added fix for setting media player name when the player Identity value is
>> invalid.
>> ---
>>   profiles/audio/media.c | 15 +++++++++++++--
>>   tools/mpris-proxy.c    |  1 +
>>   2 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/profiles/audio/media.c b/profiles/audio/media.c index
>> 69070bf..68111e6 100644
>> --- a/profiles/audio/media.c
>> +++ b/profiles/audio/media.c
>> @@ -1805,7 +1805,7 @@ static DBusMessage
>> *register_player(DBusConnection *conn, DBusMessage *msg,
>>   	struct media_adapter *adapter = data;
>>   	struct media_player *mp;
>>   	DBusMessageIter args;
>> -	const char *sender, *path;
>> +	const char *sender, *path, *name;
>>   	int err;
>>
>>   	sender = dbus_message_get_sender(msg); @@ -1814,6 +1814,8 @@
>> static DBusMessage *register_player(DBusConnection *conn, DBusMessage
>> *msg,
>>
>>   	dbus_message_iter_get_basic(&args, &path);
>>   	dbus_message_iter_next(&args);
>> +	dbus_message_iter_get_basic(&args, &name);
>> +	dbus_message_iter_next(&args);
>>
>>   	if (media_adapter_find_player(adapter, sender, path) != NULL)
>>   		return btd_error_already_exists(msg); @@ -1831,6 +1833,14
>> @@ static DBusMessage *register_player(DBusConnection *conn,
>> DBusMessage *msg,
>>   		return btd_error_invalid_args(msg);
>>   	}
>>
>> +	/*
>> +	 * The Player Identity value is NULL in MPRIS case, so create
>> +	 * mpris name on the basis of player bus name and assign
>> +	 * the value.
>> +	 */
>> +	if (mp->name == NULL)
>> +		mp->name = g_strdup(name);
>> +
>>   	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);  }
>>
>> @@ -1864,7 +1874,8 @@ static const GDBusMethodTable media_methods[]
>> = {
>>   	{ GDBUS_METHOD("UnregisterEndpoint",
>>   		GDBUS_ARGS({ "endpoint", "o" }), NULL,
>> unregister_endpoint) },
>>   	{ GDBUS_METHOD("RegisterPlayer",
>> -		GDBUS_ARGS({ "player", "o" }, { "properties", "a{sv}" }),
>> +		GDBUS_ARGS({ "player", "o" }, { "name", "s" },
>> +						{ "properties", "a{sv}" }),

This breaks the API. Can't the name be passed as a member of the 
properties dict?

regards,
Simon

RE: [PATCH ] audio/player: Fix player name value for MPRIS

[Bharat Bhusan Panda]

Hi Simon,

> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Simon Fels
> Sent: Tuesday, October 20, 2015 6:48 PM
> To: Bharat Bhusan Panda; linux-bluetooth@vger.kernel.org
> Cc: cpgs@samsung.com
> Subject: Re: [PATCH ] audio/player: Fix player name value for MPRIS
> 
> On 20.10.2015 15:05, Bharat Bhusan Panda wrote:
> > ping
> >
> >> -----Original Message-----
> >> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> >> owner@vger.kernel.org] On Behalf Of Bharat Panda
> >> Sent: Monday, October 12, 2015 2:11 PM
> >> To: linux-bluetooth@vger.kernel.org
> >> Cc: cpgs@samsung.com; Bharat Panda
> >> Subject: [PATCH ] audio/player: Fix player name value for MPRIS
> >>
> >> Added fix for setting media player name when the player Identity
> >> value is invalid.
> >> ---
> >>   profiles/audio/media.c | 15 +++++++++++++--
> >>   tools/mpris-proxy.c    |  1 +
> >>   2 files changed, 14 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/profiles/audio/media.c b/profiles/audio/media.c index
> >> 69070bf..68111e6 100644
> >> --- a/profiles/audio/media.c
> >> +++ b/profiles/audio/media.c
> >> @@ -1805,7 +1805,7 @@ static DBusMessage
> >> *register_player(DBusConnection *conn, DBusMessage *msg,
> >>   	struct media_adapter *adapter = data;
> >>   	struct media_player *mp;
> >>   	DBusMessageIter args;
> >> -	const char *sender, *path;
> >> +	const char *sender, *path, *name;
> >>   	int err;
> >>
> >>   	sender = dbus_message_get_sender(msg); @@ -1814,6 +1814,8 @@
> >> static DBusMessage *register_player(DBusConnection *conn,
> DBusMessage
> >> *msg,
> >>
> >>   	dbus_message_iter_get_basic(&args, &path);
> >>   	dbus_message_iter_next(&args);
> >> +	dbus_message_iter_get_basic(&args, &name);
> >> +	dbus_message_iter_next(&args);
> >>
> >>   	if (media_adapter_find_player(adapter, sender, path) != NULL)
> >>   		return btd_error_already_exists(msg); @@ -1831,6 +1833,14
> @@
> >> static DBusMessage *register_player(DBusConnection *conn,
> DBusMessage
> >> *msg,
> >>   		return btd_error_invalid_args(msg);
> >>   	}
> >>
> >> +	/*
> >> +	 * The Player Identity value is NULL in MPRIS case, so create
> >> +	 * mpris name on the basis of player bus name and assign
> >> +	 * the value.
> >> +	 */
> >> +	if (mp->name == NULL)
> >> +		mp->name = g_strdup(name);
> >> +
> >>   	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);  }
> >>
> >> @@ -1864,7 +1874,8 @@ static const GDBusMethodTable
> media_methods[] =
> >> {
> >>   	{ GDBUS_METHOD("UnregisterEndpoint",
> >>   		GDBUS_ARGS({ "endpoint", "o" }), NULL,
> >> unregister_endpoint) },
> >>   	{ GDBUS_METHOD("RegisterPlayer",
> >> -		GDBUS_ARGS({ "player", "o" }, { "properties", "a{sv}" }),
> >> +		GDBUS_ARGS({ "player", "o" }, { "name", "s" },
> >> +						{ "properties", "a{sv}" }),
> 
> This breaks the API. Can't the name be passed as a member of the
properties
> dict?
It can be passed in properties dict, but again as per mpris spec there is no
properties defined for name but it can be tweaked at mpris-proxy and passed
to media player.
Not sure which approach will be better to pass the name field.
> 
> regards,
> Simon
> 
Regards,
Bharat 

Re: [PATCH ] audio/player: Fix player name value for MPRIS

[Luiz Augusto von Dentz]

Hi Bharat,

On Tue, Oct 20, 2015 at 4:55 PM, Bharat Bhusan Panda
<bharat.panda@samsung.com> wrote:
> Hi Simon,
>
>> -----Original Message-----
>> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
>> owner@vger.kernel.org] On Behalf Of Simon Fels
>> Sent: Tuesday, October 20, 2015 6:48 PM
>> To: Bharat Bhusan Panda; linux-bluetooth@vger.kernel.org
>> Cc: cpgs@samsung.com
>> Subject: Re: [PATCH ] audio/player: Fix player name value for MPRIS
>>
>> On 20.10.2015 15:05, Bharat Bhusan Panda wrote:
>> > ping
>> >
>> >> -----Original Message-----
>> >> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
>> >> owner@vger.kernel.org] On Behalf Of Bharat Panda
>> >> Sent: Monday, October 12, 2015 2:11 PM
>> >> To: linux-bluetooth@vger.kernel.org
>> >> Cc: cpgs@samsung.com; Bharat Panda
>> >> Subject: [PATCH ] audio/player: Fix player name value for MPRIS
>> >>
>> >> Added fix for setting media player name when the player Identity
>> >> value is invalid.
>> >> ---
>> >>   profiles/audio/media.c | 15 +++++++++++++--
>> >>   tools/mpris-proxy.c    |  1 +
>> >>   2 files changed, 14 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/profiles/audio/media.c b/profiles/audio/media.c index
>> >> 69070bf..68111e6 100644
>> >> --- a/profiles/audio/media.c
>> >> +++ b/profiles/audio/media.c
>> >> @@ -1805,7 +1805,7 @@ static DBusMessage
>> >> *register_player(DBusConnection *conn, DBusMessage *msg,
>> >>    struct media_adapter *adapter = data;
>> >>    struct media_player *mp;
>> >>    DBusMessageIter args;
>> >> -  const char *sender, *path;
>> >> +  const char *sender, *path, *name;
>> >>    int err;
>> >>
>> >>    sender = dbus_message_get_sender(msg); @@ -1814,6 +1814,8 @@
>> >> static DBusMessage *register_player(DBusConnection *conn,
>> DBusMessage
>> >> *msg,
>> >>
>> >>    dbus_message_iter_get_basic(&args, &path);
>> >>    dbus_message_iter_next(&args);
>> >> +  dbus_message_iter_get_basic(&args, &name);
>> >> +  dbus_message_iter_next(&args);
>> >>
>> >>    if (media_adapter_find_player(adapter, sender, path) != NULL)
>> >>            return btd_error_already_exists(msg); @@ -1831,6 +1833,14
>> @@
>> >> static DBusMessage *register_player(DBusConnection *conn,
>> DBusMessage
>> >> *msg,
>> >>            return btd_error_invalid_args(msg);
>> >>    }
>> >>
>> >> +  /*
>> >> +   * The Player Identity value is NULL in MPRIS case, so create
>> >> +   * mpris name on the basis of player bus name and assign
>> >> +   * the value.
>> >> +   */
>> >> +  if (mp->name == NULL)
>> >> +          mp->name = g_strdup(name);
>> >> +
>> >>    return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);  }
>> >>
>> >> @@ -1864,7 +1874,8 @@ static const GDBusMethodTable
>> media_methods[] =
>> >> {
>> >>    { GDBUS_METHOD("UnregisterEndpoint",
>> >>            GDBUS_ARGS({ "endpoint", "o" }), NULL,
>> >> unregister_endpoint) },
>> >>    { GDBUS_METHOD("RegisterPlayer",
>> >> -          GDBUS_ARGS({ "player", "o" }, { "properties", "a{sv}" }),
>> >> +          GDBUS_ARGS({ "player", "o" }, { "name", "s" },
>> >> +                                          { "properties", "a{sv}" }),
>>
>> This breaks the API. Can't the name be passed as a member of the
> properties
>> dict?

We are not going to break the API that is for sure, so if there is no
Identity we should probably come up with a good default.

> It can be passed in properties dict, but again as per mpris spec there is no
> properties defined for name but it can be tweaked at mpris-proxy and passed
> to media player.
> Not sure which approach will be better to pass the name field.

Actually there is, it is the bus name if Im not mistaken.

>> regards,
>> Simon
>>
> Regards,
> Bharat
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz