2015-10-12 08:41:17

by Bharat Bhusan Panda

[permalink] [raw]
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



2015-10-20 18:18:52

by Luiz Augusto von Dentz

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

Hi Bharat,

On Tue, Oct 20, 2015 at 4:55 PM, Bharat Bhusan Panda
<[email protected]> wrote:
> Hi Simon,
>
>> -----Original Message-----
>> From: [email protected] [mailto:linux-bluetooth-
>> [email protected]] On Behalf Of Simon Fels
>> Sent: Tuesday, October 20, 2015 6:48 PM
>> To: Bharat Bhusan Panda; [email protected]
>> Cc: [email protected]
>> 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: [email protected] [mailto:linux-bluetooth-
>> >> [email protected]] On Behalf Of Bharat Panda
>> >> Sent: Monday, October 12, 2015 2:11 PM
>> >> To: [email protected]
>> >> Cc: [email protected]; 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Luiz Augusto von Dentz

2015-10-20 13:55:31

by Bharat Bhusan Panda

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

Hi Simon,

> -----Original Message-----
> From: [email protected] [mailto:linux-bluetooth-
> [email protected]] On Behalf Of Simon Fels
> Sent: Tuesday, October 20, 2015 6:48 PM
> To: Bharat Bhusan Panda; [email protected]
> Cc: [email protected]
> 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: [email protected] [mailto:linux-bluetooth-
> >> [email protected]] On Behalf Of Bharat Panda
> >> Sent: Monday, October 12, 2015 2:11 PM
> >> To: [email protected]
> >> Cc: [email protected]; 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



2015-10-20 13:18:03

by Simon Fels

[permalink] [raw]
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: [email protected] [mailto:linux-bluetooth-
>> [email protected]] On Behalf Of Bharat Panda
>> Sent: Monday, October 12, 2015 2:11 PM
>> To: [email protected]
>> Cc: [email protected]; 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


2015-10-20 13:05:19

by Bharat Bhusan Panda

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

ping

> -----Original Message-----
> From: [email protected] [mailto:linux-bluetooth-
> [email protected]] On Behalf Of Bharat Panda
> Sent: Monday, October 12, 2015 2:11 PM
> To: [email protected]
> Cc: [email protected]; 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 [email protected] More majordomo
> info at http://vger.kernel.org/majordomo-info.html