2015-10-01 11:48:59

by Bharat Bhusan Panda

[permalink] [raw]
Subject: [PATCH 1/2] audio/avrcp: Add support for Set Browsed Player

Support added to handle SetBrowsedPlayer cmd in AVRCP TG role.
Added a new player callback get_playlist to retrieve available
playlists on org.moris.MediPlayer2.Playlists interface.

btmon:
AVCTP Browsing: Response: type 0x00 label 13 PID 0x110e
AVRCP: SetBrowsedPlayer: len 0x000a
Status: 0x04 (Success)
UIDCounter: 0x0000 (0)
Number of Items: 0x00000007 (7)
CharsetID: 0x006a (UTF-8)
Folder Depth: 0x00 (0)
---
profiles/audio/avrcp.c | 71 ++++++++++++++++++++++++++++++++++++
profiles/audio/avrcp.h | 4 +++
profiles/audio/media.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 173 insertions(+)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 24deac5..b2198e3 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -207,6 +207,15 @@ struct player_item {
char name[0];
} __attribute__ ((packed));

+struct avrcp_set_browsed_player_rsp {
+ uint8_t status;
+ uint16_t uid_counter;
+ uint32_t num_of_items;
+ uint16_t charset_id;
+ uint8_t folder_depth;
+ uint8_t data[0];
+} __attribute__ ((packed));
+
struct avrcp_server {
struct btd_adapter *adapter;
uint32_t tg_record_id;
@@ -1873,6 +1882,67 @@ err_metadata:
return AVRCP_HEADER_LENGTH + 1;
}

+static void avrcp_handle_set_browsed_player(struct avrcp *session,
+ struct avrcp_browsing_header *pdu,
+ uint8_t transaction)
+{
+ struct avrcp_player *player;
+ struct media_player *mp;
+ uint16_t len = ntohs(pdu->param_len);
+ uint16_t player_id = 0;
+ uint8_t status;
+
+ if (len < 1) {
+ status = AVRCP_STATUS_INVALID_PARAM;
+ goto failed;
+ }
+
+ player_id = bt_get_be16(&pdu->params[0]);
+ player = find_tg_player(session, player_id);
+
+ if (player) {
+ struct avrcp_set_browsed_player_rsp *resp;
+ uint32_t num_of_items = 0;
+
+ mp = player->user_data;
+ player->browsed = true;
+
+ resp = (struct avrcp_set_browsed_player_rsp *) pdu->params;
+
+ resp->status = AVRCP_STATUS_SUCCESS;
+ resp->uid_counter = htons(player_get_uid_counter(player));
+ resp->num_of_items = 0;
+ resp->charset_id = htons(AVRCP_CHARSET_UTF8);
+ resp->folder_depth = 0;
+ pdu->param_len = htons(sizeof(*resp));
+
+ if (!player->cb->get_playlists(0x0000, 0xFFFF,
+ "Alphabetical",
+ NULL,
+ &num_of_items, mp)) {
+ status = AVRCP_STATUS_INVALID_PARAM;
+ goto failed;
+ }
+
+ resp->num_of_items = htonl(num_of_items);
+
+ /*
+ * MPRIS player returns a flat hierarchy playlist structure,
+ * where the folder depth will always considered to be 0.
+ */
+ resp->folder_depth = 0;
+ } else {
+ status = AVRCP_STATUS_INVALID_PLAYER_ID;
+ goto failed;
+ }
+
+ return;
+
+failed:
+ pdu->params[0] = status;
+ pdu->param_len = htons(1);
+}
+
static void avrcp_handle_media_player_list(struct avrcp *session,
struct avrcp_browsing_header *pdu,
uint32_t start_item, uint32_t end_item)
@@ -1989,6 +2059,7 @@ static struct browsing_pdu_handler {
uint8_t transaction);
} browsing_handlers[] = {
{ AVRCP_GET_FOLDER_ITEMS, avrcp_handle_get_folder_items },
+ { AVRCP_SET_BROWSED_PLAYER, avrcp_handle_set_browsed_player},
{ },
};

diff --git a/profiles/audio/avrcp.h b/profiles/audio/avrcp.h
index 86d310c..87e591d 100644
--- a/profiles/audio/avrcp.h
+++ b/profiles/audio/avrcp.h
@@ -101,6 +101,10 @@ struct avrcp_player_cb {
bool (*pause) (void *user_data);
bool (*next) (void *user_data);
bool (*previous) (void *user_data);
+ int (*get_playlists) (uint32_t start, uint32_t end,
+ char *ordering, uint8_t *pdu,
+ uint32_t *num_of_items,
+ void *user_data);
};

int avrcp_set_volume(struct btd_device *dev, uint8_t volume, bool notify);
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 69070bf..2835f93 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -58,6 +58,7 @@
#define MEDIA_INTERFACE "org.bluez.Media1"
#define MEDIA_ENDPOINT_INTERFACE "org.bluez.MediaEndpoint1"
#define MEDIA_PLAYER_INTERFACE "org.mpris.MediaPlayer2.Player"
+#define MEDIA_PLAYER_PLAYLIST_INTERFACE "org.mpris.MediaPlayer2.Playlists"

#define REQUEST_TIMEOUT (3 * 1000) /* 3 seconds */

@@ -115,6 +116,14 @@ struct media_player {
char *name;
};

+struct media_playlist {
+ char *name;
+ char *id;
+ char *icon;
+ uint32_t num_of_items;
+ uint8_t data[0];
+} __attribute__ ((packed));
+
static GSList *adapters = NULL;

static void endpoint_request_free(struct endpoint_request *request)
@@ -1270,6 +1279,94 @@ static bool previous(void *user_data)

return media_player_send(mp, "Previous");
}
+static int get_playlists(uint32_t start_index, uint32_t end_index,
+ char *ordering, uint8_t *pdu,
+ uint32_t *num_of_items,
+ void *user_data)
+{
+ struct media_player *mp = user_data;
+ struct media_playlist *playlist;
+ DBusMessage *msg;
+ DBusMessage *reply;
+ DBusMessageIter array_iter;
+ DBusMessageIter value;
+ DBusMessageIter struct_iter;
+ DBusError err;
+ gboolean reverse_order = FALSE;
+ int type;
+ uint32_t total_items = 0;
+ uint32_t max_count = (end_index - start_index);
+
+ msg = dbus_message_new_method_call(mp->sender, mp->path,
+ MEDIA_PLAYER_PLAYLIST_INTERFACE,
+ "GetPlaylists");
+
+ if (msg == NULL) {
+ error("Couldn't allocate D-Bus message");
+ return -ENOMEM;
+ }
+
+ dbus_message_append_args(msg,
+ DBUS_TYPE_UINT32, &start_index,
+ DBUS_TYPE_UINT32, &max_count,
+ DBUS_TYPE_STRING, &ordering,
+ DBUS_TYPE_BOOLEAN, &reverse_order,
+ DBUS_TYPE_INVALID);
+
+ dbus_error_init(&err);
+
+ /* Make Dbus sync call to get available playlists */
+ reply = dbus_connection_send_with_reply_and_block(
+ btd_get_dbus_connection(), msg, -1, &err);
+
+ dbus_message_unref(msg);
+
+ if (!reply) {
+ if (dbus_error_is_set(&err)) {
+ error("%s", err.message);
+ dbus_error_free(&err);
+ }
+ return FALSE;
+ }
+
+ dbus_message_iter_init(reply, &array_iter);
+ type = dbus_message_iter_get_arg_type(&array_iter);
+
+ if (type != DBUS_TYPE_ARRAY)
+ goto done;
+
+ dbus_message_iter_recurse(&array_iter, &struct_iter);
+
+ while ((type = dbus_message_iter_get_arg_type(&struct_iter)) !=
+ DBUS_TYPE_INVALID) {
+ if (type != DBUS_TYPE_STRUCT)
+ goto done;
+
+ dbus_message_iter_recurse(&struct_iter, &value);
+
+ if (pdu != NULL) {
+ playlist = (struct media_playlist *) pdu;
+
+ dbus_message_iter_get_basic(&value, &playlist->id);
+
+ dbus_message_iter_next(&value);
+ dbus_message_iter_get_basic(&value, &playlist->name);
+
+ dbus_message_iter_next(&value);
+ dbus_message_iter_get_basic(&value, &playlist->icon);
+
+ /* TODO: Create Media folder with playlist info */
+ }
+
+ total_items++;
+ *num_of_items = total_items;
+
+ dbus_message_iter_next(&struct_iter);
+ }
+
+done:
+ return TRUE;
+}

static struct avrcp_player_cb player_cb = {
.list_settings = list_settings,
@@ -1288,6 +1385,7 @@ static struct avrcp_player_cb player_cb = {
.pause = pause,
.next = next,
.previous = previous,
+ .get_playlists = get_playlists,
};

static void media_player_exit(DBusConnection *connection, void *user_data)
--
1.9.1



2015-10-07 04:45:36

by Bharat Bhusan Panda

[permalink] [raw]
Subject: RE: [PATCH 1/2] audio/avrcp: Add support for Set Browsed Player

Hi Luiz,

> -----Original Message-----
> From: [email protected] [mailto:linux-bluetooth-
> [email protected]] On Behalf Of Luiz Augusto von Dentz
> Sent: Thursday, October 01, 2015 8:06 PM
> To: Bharat Panda
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH 1/2] audio/avrcp: Add support for Set Browsed Player
>
> Hi Bharat,
>
> On Thu, Oct 1, 2015 at 2:48 PM, Bharat Panda <[email protected]>
> wrote:
> > Support added to handle SetBrowsedPlayer cmd in AVRCP TG role.
> > Added a new player callback get_playlist to retrieve available
> > playlists on org.moris.MediPlayer2.Playlists interface.
> >
> > btmon:
> > AVCTP Browsing: Response: type 0x00 label 13 PID 0x110e
> > AVRCP: SetBrowsedPlayer: len 0x000a
> > Status: 0x04 (Success)
> > UIDCounter: 0x0000 (0)
> > Number of Items: 0x00000007 (7)
> > CharsetID: 0x006a (UTF-8)
> > Folder Depth: 0x00 (0)
> > ---
> > profiles/audio/avrcp.c | 71 ++++++++++++++++++++++++++++++++++++
> > profiles/audio/avrcp.h | 4 +++
> > profiles/audio/media.c | 98
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 173 insertions(+)
> >
> > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c index
> > 24deac5..b2198e3 100644
> > --- a/profiles/audio/avrcp.c
> > +++ b/profiles/audio/avrcp.c
> > @@ -207,6 +207,15 @@ struct player_item {
> > char name[0];
> > } __attribute__ ((packed));
> >
> > +struct avrcp_set_browsed_player_rsp {
> > + uint8_t status;
> > + uint16_t uid_counter;
> > + uint32_t num_of_items;
> > + uint16_t charset_id;
> > + uint8_t folder_depth;
> > + uint8_t data[0];
> > +} __attribute__ ((packed));
> > +
> > struct avrcp_server {
> > struct btd_adapter *adapter;
> > uint32_t tg_record_id;
> > @@ -1873,6 +1882,67 @@ err_metadata:
> > return AVRCP_HEADER_LENGTH + 1; }
> >
> > +static void avrcp_handle_set_browsed_player(struct avrcp *session,
> > + struct avrcp_browsing_header *pdu,
> > + uint8_t transaction) {
> > + struct avrcp_player *player;
> > + struct media_player *mp;
> > + uint16_t len = ntohs(pdu->param_len);
> > + uint16_t player_id = 0;
> > + uint8_t status;
> > +
> > + if (len < 1) {
> > + status = AVRCP_STATUS_INVALID_PARAM;
> > + goto failed;
> > + }
> > +
> > + player_id = bt_get_be16(&pdu->params[0]);
> > + player = find_tg_player(session, player_id);
> > +
> > + if (player) {
> > + struct avrcp_set_browsed_player_rsp *resp;
> > + uint32_t num_of_items = 0;
> > +
> > + mp = player->user_data;
> > + player->browsed = true;
> > +
> > + resp = (struct avrcp_set_browsed_player_rsp *)
> > + pdu->params;
> > +
> > + resp->status = AVRCP_STATUS_SUCCESS;
> > + resp->uid_counter = htons(player_get_uid_counter(player));
> > + resp->num_of_items = 0;
> > + resp->charset_id = htons(AVRCP_CHARSET_UTF8);
> > + resp->folder_depth = 0;
> > + pdu->param_len = htons(sizeof(*resp));
> > +
> > + if (!player->cb->get_playlists(0x0000, 0xFFFF,
> > + "Alphabetical",
> > + NULL,
> > + &num_of_items, mp)) {
>
> Design the callback based in AVRCP interface not MPRIS, it is up to media.c to
> convert this.
>
> > + status = AVRCP_STATUS_INVALID_PARAM;
> > + goto failed;
> > + }
> > +
> > + resp->num_of_items = htonl(num_of_items);
> > +
> > + /*
> > + * MPRIS player returns a flat hierarchy playlist structure,
> > + * where the folder depth will always considered to be 0.
> > + */
> > + resp->folder_depth = 0;
>
> It seems you are assigning this twice, also it probably make sense to split this
> into player_set_browsed and let it return the number of items.
>
> > + } else {
> > + status = AVRCP_STATUS_INVALID_PLAYER_ID;
> > + goto failed;
> > + }
> > +
> > + return;
> > +
> > +failed:
> > + pdu->params[0] = status;
> > + pdu->param_len = htons(1);
> > +}
> > +
> > static void avrcp_handle_media_player_list(struct avrcp *session,
> > struct avrcp_browsing_header *pdu,
> > uint32_t start_item, uint32_t
> > end_item) @@ -1989,6 +2059,7 @@ static struct browsing_pdu_handler {
> > uint8_t
> > transaction); } browsing_handlers[] = {
> > { AVRCP_GET_FOLDER_ITEMS,
> > avrcp_handle_get_folder_items },
> > + { AVRCP_SET_BROWSED_PLAYER,
> > + avrcp_handle_set_browsed_player},
> > { },
> > };
> >
> > diff --git a/profiles/audio/avrcp.h b/profiles/audio/avrcp.h index
> > 86d310c..87e591d 100644
> > --- a/profiles/audio/avrcp.h
> > +++ b/profiles/audio/avrcp.h
> > @@ -101,6 +101,10 @@ struct avrcp_player_cb {
> > bool (*pause) (void *user_data);
> > bool (*next) (void *user_data);
> > bool (*previous) (void *user_data);
> > + int (*get_playlists) (uint32_t start, uint32_t end,
> > + char *ordering, uint8_t *pdu,
> > + uint32_t *num_of_items,
> > + void *user_data);
> > };
> >
> > int avrcp_set_volume(struct btd_device *dev, uint8_t volume, bool
> > notify); diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> > index 69070bf..2835f93 100644
> > --- a/profiles/audio/media.c
> > +++ b/profiles/audio/media.c
> > @@ -58,6 +58,7 @@
> > #define MEDIA_INTERFACE "org.bluez.Media1"
> > #define MEDIA_ENDPOINT_INTERFACE "org.bluez.MediaEndpoint1"
> > #define MEDIA_PLAYER_INTERFACE "org.mpris.MediaPlayer2.Player"
> > +#define MEDIA_PLAYER_PLAYLIST_INTERFACE
> "org.mpris.MediaPlayer2.Playlists"
> >
> > #define REQUEST_TIMEOUT (3 * 1000) /* 3 seconds */
> >
> > @@ -115,6 +116,14 @@ struct media_player {
> > char *name;
> > };
> >
> > +struct media_playlist {
> > + char *name;
> > + char *id;
> > + char *icon;
> > + uint32_t num_of_items;
> > + uint8_t data[0];
> > +} __attribute__ ((packed));
> > +
> > static GSList *adapters = NULL;
> >
> > static void endpoint_request_free(struct endpoint_request *request)
> > @@ -1270,6 +1279,94 @@ static bool previous(void *user_data)
> >
> > return media_player_send(mp, "Previous"); }
> > +static int get_playlists(uint32_t start_index, uint32_t end_index,
> > + char *ordering, uint8_t *pdu,
> > + uint32_t *num_of_items,
> > + void *user_data) {
> > + struct media_player *mp = user_data;
> > + struct media_playlist *playlist;
> > + DBusMessage *msg;
> > + DBusMessage *reply;
> > + DBusMessageIter array_iter;
> > + DBusMessageIter value;
> > + DBusMessageIter struct_iter;
> > + DBusError err;
> > + gboolean reverse_order = FALSE;
> > + int type;
> > + uint32_t total_items = 0;
> > + uint32_t max_count = (end_index - start_index);
> > +
> > + msg = dbus_message_new_method_call(mp->sender, mp->path,
> > + MEDIA_PLAYER_PLAYLIST_INTERFACE,
> > + "GetPlaylists");
> > +
> > + if (msg == NULL) {
> > + error("Couldn't allocate D-Bus message");
> > + return -ENOMEM;
> > + }
> > +
> > + dbus_message_append_args(msg,
> > + DBUS_TYPE_UINT32, &start_index,
> > + DBUS_TYPE_UINT32, &max_count,
> > + DBUS_TYPE_STRING, &ordering,
> > + DBUS_TYPE_BOOLEAN, &reverse_order,
> > + DBUS_TYPE_INVALID);
> > +
> > + dbus_error_init(&err);
> > +
> > + /* Make Dbus sync call to get available playlists */
> > + reply = dbus_connection_send_with_reply_and_block(
> > + btd_get_dbus_connection(), msg, -1,
> > + &err);
>
> You should never do blocking call in the daemon, this would make it
> unresponsive until the player respond. We should either add the
> PlaylistCount and Playlists properties in the registration or try to read them as
> soon the player register, and since they are properties we can probably
> cache it and subscribe to PropertiesChanged interface that way we never
> have to call anything blocking.

Thanks, I have incorporated above review comments and added 3 separate patches for,
- Getting Playlists after player registration.
- Add watch on playlist details changes
- Set Browsed Player response.
TODO: properties changed watch needs to be added, with playlist properties parser.

>
> --
> Luiz Augusto von Dentz
> --

Regards,
Bharat


2015-10-02 07:33:07

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 2/2] tools: Fix Invalid return

Hi Bharat,

On Thu, Oct 1, 2015 at 2:49 PM, Bharat Panda <[email protected]> wrote:
> Return DBUS_HANDLER_RESULT_HANDLED instead of
> DBUS_HANDLER_RESULT_NOT_YET_HANDLED for player message handle.
> ---
> tools/mpris-proxy.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/mpris-proxy.c b/tools/mpris-proxy.c
> index 693055e..2a5cbae 100644
> --- a/tools/mpris-proxy.c
> +++ b/tools/mpris-proxy.c
> @@ -205,6 +205,7 @@ static void append_container(DBusMessageIter *base, DBusMessageIter *iter,
> case DBUS_TYPE_ARRAY:
> case DBUS_TYPE_VARIANT:
> sig = dbus_message_iter_get_signature(&iter_sub);
> + printf("signature: %s\n", sig);
> break;
> default:
> sig = NULL;
> @@ -406,7 +407,7 @@ static DBusHandlerResult player_message(DBusConnection *conn,
> done:
> dbus_message_unref(copy);
>
> - return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
> + return DBUS_HANDLER_RESULT_HANDLED;
> }
>
> static struct player *find_player_by_bus_name(const char *name)
> --
> 1.9.1

I went ahead and applied this one, note that I removed the printf.


--
Luiz Augusto von Dentz

2015-10-01 14:35:46

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/2] audio/avrcp: Add support for Set Browsed Player

Hi Bharat,

On Thu, Oct 1, 2015 at 2:48 PM, Bharat Panda <[email protected]> wrote:
> Support added to handle SetBrowsedPlayer cmd in AVRCP TG role.
> Added a new player callback get_playlist to retrieve available
> playlists on org.moris.MediPlayer2.Playlists interface.
>
> btmon:
> AVCTP Browsing: Response: type 0x00 label 13 PID 0x110e
> AVRCP: SetBrowsedPlayer: len 0x000a
> Status: 0x04 (Success)
> UIDCounter: 0x0000 (0)
> Number of Items: 0x00000007 (7)
> CharsetID: 0x006a (UTF-8)
> Folder Depth: 0x00 (0)
> ---
> profiles/audio/avrcp.c | 71 ++++++++++++++++++++++++++++++++++++
> profiles/audio/avrcp.h | 4 +++
> profiles/audio/media.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 173 insertions(+)
>
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index 24deac5..b2198e3 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -207,6 +207,15 @@ struct player_item {
> char name[0];
> } __attribute__ ((packed));
>
> +struct avrcp_set_browsed_player_rsp {
> + uint8_t status;
> + uint16_t uid_counter;
> + uint32_t num_of_items;
> + uint16_t charset_id;
> + uint8_t folder_depth;
> + uint8_t data[0];
> +} __attribute__ ((packed));
> +
> struct avrcp_server {
> struct btd_adapter *adapter;
> uint32_t tg_record_id;
> @@ -1873,6 +1882,67 @@ err_metadata:
> return AVRCP_HEADER_LENGTH + 1;
> }
>
> +static void avrcp_handle_set_browsed_player(struct avrcp *session,
> + struct avrcp_browsing_header *pdu,
> + uint8_t transaction)
> +{
> + struct avrcp_player *player;
> + struct media_player *mp;
> + uint16_t len = ntohs(pdu->param_len);
> + uint16_t player_id = 0;
> + uint8_t status;
> +
> + if (len < 1) {
> + status = AVRCP_STATUS_INVALID_PARAM;
> + goto failed;
> + }
> +
> + player_id = bt_get_be16(&pdu->params[0]);
> + player = find_tg_player(session, player_id);
> +
> + if (player) {
> + struct avrcp_set_browsed_player_rsp *resp;
> + uint32_t num_of_items = 0;
> +
> + mp = player->user_data;
> + player->browsed = true;
> +
> + resp = (struct avrcp_set_browsed_player_rsp *) pdu->params;
> +
> + resp->status = AVRCP_STATUS_SUCCESS;
> + resp->uid_counter = htons(player_get_uid_counter(player));
> + resp->num_of_items = 0;
> + resp->charset_id = htons(AVRCP_CHARSET_UTF8);
> + resp->folder_depth = 0;
> + pdu->param_len = htons(sizeof(*resp));
> +
> + if (!player->cb->get_playlists(0x0000, 0xFFFF,
> + "Alphabetical",
> + NULL,
> + &num_of_items, mp)) {

Design the callback based in AVRCP interface not MPRIS, it is up to
media.c to convert this.

> + status = AVRCP_STATUS_INVALID_PARAM;
> + goto failed;
> + }
> +
> + resp->num_of_items = htonl(num_of_items);
> +
> + /*
> + * MPRIS player returns a flat hierarchy playlist structure,
> + * where the folder depth will always considered to be 0.
> + */
> + resp->folder_depth = 0;

It seems you are assigning this twice, also it probably make sense to
split this into player_set_browsed and let it return the number of
items.

> + } else {
> + status = AVRCP_STATUS_INVALID_PLAYER_ID;
> + goto failed;
> + }
> +
> + return;
> +
> +failed:
> + pdu->params[0] = status;
> + pdu->param_len = htons(1);
> +}
> +
> static void avrcp_handle_media_player_list(struct avrcp *session,
> struct avrcp_browsing_header *pdu,
> uint32_t start_item, uint32_t end_item)
> @@ -1989,6 +2059,7 @@ static struct browsing_pdu_handler {
> uint8_t transaction);
> } browsing_handlers[] = {
> { AVRCP_GET_FOLDER_ITEMS, avrcp_handle_get_folder_items },
> + { AVRCP_SET_BROWSED_PLAYER, avrcp_handle_set_browsed_player},
> { },
> };
>
> diff --git a/profiles/audio/avrcp.h b/profiles/audio/avrcp.h
> index 86d310c..87e591d 100644
> --- a/profiles/audio/avrcp.h
> +++ b/profiles/audio/avrcp.h
> @@ -101,6 +101,10 @@ struct avrcp_player_cb {
> bool (*pause) (void *user_data);
> bool (*next) (void *user_data);
> bool (*previous) (void *user_data);
> + int (*get_playlists) (uint32_t start, uint32_t end,
> + char *ordering, uint8_t *pdu,
> + uint32_t *num_of_items,
> + void *user_data);
> };
>
> int avrcp_set_volume(struct btd_device *dev, uint8_t volume, bool notify);
> diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> index 69070bf..2835f93 100644
> --- a/profiles/audio/media.c
> +++ b/profiles/audio/media.c
> @@ -58,6 +58,7 @@
> #define MEDIA_INTERFACE "org.bluez.Media1"
> #define MEDIA_ENDPOINT_INTERFACE "org.bluez.MediaEndpoint1"
> #define MEDIA_PLAYER_INTERFACE "org.mpris.MediaPlayer2.Player"
> +#define MEDIA_PLAYER_PLAYLIST_INTERFACE "org.mpris.MediaPlayer2.Playlists"
>
> #define REQUEST_TIMEOUT (3 * 1000) /* 3 seconds */
>
> @@ -115,6 +116,14 @@ struct media_player {
> char *name;
> };
>
> +struct media_playlist {
> + char *name;
> + char *id;
> + char *icon;
> + uint32_t num_of_items;
> + uint8_t data[0];
> +} __attribute__ ((packed));
> +
> static GSList *adapters = NULL;
>
> static void endpoint_request_free(struct endpoint_request *request)
> @@ -1270,6 +1279,94 @@ static bool previous(void *user_data)
>
> return media_player_send(mp, "Previous");
> }
> +static int get_playlists(uint32_t start_index, uint32_t end_index,
> + char *ordering, uint8_t *pdu,
> + uint32_t *num_of_items,
> + void *user_data)
> +{
> + struct media_player *mp = user_data;
> + struct media_playlist *playlist;
> + DBusMessage *msg;
> + DBusMessage *reply;
> + DBusMessageIter array_iter;
> + DBusMessageIter value;
> + DBusMessageIter struct_iter;
> + DBusError err;
> + gboolean reverse_order = FALSE;
> + int type;
> + uint32_t total_items = 0;
> + uint32_t max_count = (end_index - start_index);
> +
> + msg = dbus_message_new_method_call(mp->sender, mp->path,
> + MEDIA_PLAYER_PLAYLIST_INTERFACE,
> + "GetPlaylists");
> +
> + if (msg == NULL) {
> + error("Couldn't allocate D-Bus message");
> + return -ENOMEM;
> + }
> +
> + dbus_message_append_args(msg,
> + DBUS_TYPE_UINT32, &start_index,
> + DBUS_TYPE_UINT32, &max_count,
> + DBUS_TYPE_STRING, &ordering,
> + DBUS_TYPE_BOOLEAN, &reverse_order,
> + DBUS_TYPE_INVALID);
> +
> + dbus_error_init(&err);
> +
> + /* Make Dbus sync call to get available playlists */
> + reply = dbus_connection_send_with_reply_and_block(
> + btd_get_dbus_connection(), msg, -1, &err);

You should never do blocking call in the daemon, this would make it
unresponsive until the player respond. We should either add the
PlaylistCount and Playlists properties in the registration or try to
read them as soon the player register, and since they are properties
we can probably cache it and subscribe to PropertiesChanged interface
that way we never have to call anything blocking.

> +
> + dbus_message_unref(msg);
> +
> + if (!reply) {
> + if (dbus_error_is_set(&err)) {
> + error("%s", err.message);
> + dbus_error_free(&err);
> + }
> + return FALSE;
> + }
> +
> + dbus_message_iter_init(reply, &array_iter);
> + type = dbus_message_iter_get_arg_type(&array_iter);
> +
> + if (type != DBUS_TYPE_ARRAY)
> + goto done;
> +
> + dbus_message_iter_recurse(&array_iter, &struct_iter);
> +
> + while ((type = dbus_message_iter_get_arg_type(&struct_iter)) !=
> + DBUS_TYPE_INVALID) {
> + if (type != DBUS_TYPE_STRUCT)
> + goto done;
> +
> + dbus_message_iter_recurse(&struct_iter, &value);
> +
> + if (pdu != NULL) {
> + playlist = (struct media_playlist *) pdu;
> +
> + dbus_message_iter_get_basic(&value, &playlist->id);
> +
> + dbus_message_iter_next(&value);
> + dbus_message_iter_get_basic(&value, &playlist->name);
> +
> + dbus_message_iter_next(&value);
> + dbus_message_iter_get_basic(&value, &playlist->icon);
> +
> + /* TODO: Create Media folder with playlist info */
> + }
> +
> + total_items++;
> + *num_of_items = total_items;
> +
> + dbus_message_iter_next(&struct_iter);
> + }
> +
> +done:
> + return TRUE;
> +}
>
> static struct avrcp_player_cb player_cb = {
> .list_settings = list_settings,
> @@ -1288,6 +1385,7 @@ static struct avrcp_player_cb player_cb = {
> .pause = pause,
> .next = next,
> .previous = previous,
> + .get_playlists = get_playlists,
> };
>
> static void media_player_exit(DBusConnection *connection, void *user_data)
> --
> 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



--
Luiz Augusto von Dentz

2015-10-01 11:49:00

by Bharat Bhusan Panda

[permalink] [raw]
Subject: [PATCH 2/2] tools: Fix Invalid return

Return DBUS_HANDLER_RESULT_HANDLED instead of
DBUS_HANDLER_RESULT_NOT_YET_HANDLED for player message handle.
---
tools/mpris-proxy.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/mpris-proxy.c b/tools/mpris-proxy.c
index 693055e..2a5cbae 100644
--- a/tools/mpris-proxy.c
+++ b/tools/mpris-proxy.c
@@ -205,6 +205,7 @@ static void append_container(DBusMessageIter *base, DBusMessageIter *iter,
case DBUS_TYPE_ARRAY:
case DBUS_TYPE_VARIANT:
sig = dbus_message_iter_get_signature(&iter_sub);
+ printf("signature: %s\n", sig);
break;
default:
sig = NULL;
@@ -406,7 +407,7 @@ static DBusHandlerResult player_message(DBusConnection *conn,
done:
dbus_message_unref(copy);

- return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+ return DBUS_HANDLER_RESULT_HANDLED;
}

static struct player *find_player_by_bus_name(const char *name)
--
1.9.1