RE: [PATCH 1/3] aurdio/avrcp: Get player playlist details

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Luiz,

> -----Original Message-----
> From: linux-bluetooth-owner@xxxxxxxxxxxxxxx [mailto:linux-bluetooth-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Luiz Augusto von Dentz
> Sent: Monday, October 19, 2015 4:06 PM
> To: Bharat Bhusan Panda
> Cc: linux-bluetooth@xxxxxxxxxxxxxxx; cpgs@xxxxxxxxxxx
> Subject: Re: [PATCH 1/3] aurdio/avrcp: Get player playlist details
> 
> Hi Bharat,
> 
> On Mon, Oct 19, 2015 at 12:02 PM, Bharat Bhusan Panda
> <bharat.panda@xxxxxxxxxxx> wrote:
> > Ping
> >
> >> -----Original Message-----
> >> From: linux-bluetooth-owner@xxxxxxxxxxxxxxx [mailto:linux-bluetooth-
> >> owner@xxxxxxxxxxxxxxx] On Behalf Of Bharat Panda
> >> Sent: Tuesday, October 06, 2015 5:23 PM
> >> To: linux-bluetooth@xxxxxxxxxxxxxxx
> >> Cc: cpgs@xxxxxxxxxxx; Bharat Panda
> >> Subject: [PATCH 1/3] aurdio/avrcp: Get player playlist details
> >>
> >> Support added to read and cache player playlist details after player
> >> registration completes.
> >> ---
> >>  profiles/audio/media.c | 121
> >> +++++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 121 insertions(+)
> >>
> >> diff --git a/profiles/audio/media.c b/profiles/audio/media.c index
> >> 69070bf..edeb66f 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 */
> >>
> >> @@ -95,6 +96,7 @@ struct media_endpoint {  struct media_player {
> >>       struct media_adapter    *adapter;
> >>       struct avrcp_player     *player;
> >> +     GSList                  *playlists;
> >>       char                    *sender;        /* Player DBus bus id */
> >>       char                    *path;          /* Player object path */
> >>       GHashTable              *settings;      /* Player settings */
> >> @@ -105,8 +107,10 @@ struct media_player {
> >>       char                    *status;
> >>       uint32_t                position;
> >>       uint32_t                duration;
> >> +     uint32_t                total_items;
> >>       uint8_t                 volume;
> >>       GTimer                  *timer;
> >> +     guint                   playlist_id;
> >>       bool                    play;
> >>       bool                    pause;
> >>       bool                    next;
> >> @@ -115,6 +119,13 @@ struct media_player {
> >>       char                    *name;
> >>  };
> >>
> >> +struct media_playlist {
> >> +     char            *name;
> >> +     char            *id;
> >> +     char            *icon;
> >> +     uint8_t         data[0];
> >> +} __attribute__ ((packed));
> >> +
> >>  static GSList *adapters = NULL;
> >>
> >>  static void endpoint_request_free(struct endpoint_request *request)
> >> @@ -
> >> 961,6 +972,9 @@ static void media_player_free(gpointer data)
> >>       if (mp->settings)
> >>               g_hash_table_unref(mp->settings);
> >>
> >> +     if (mp->playlist_id > 0)
> >> +             g_source_remove(mp->playlist_id);
> >> +
> >>       g_timer_destroy(mp->timer);
> >>       g_free(mp->sender);
> >>       g_free(mp->path);
> >> @@ -1271,6 +1285,111 @@ static bool previous(void *user_data)
> >>       return media_player_send(mp, "Previous");  }
> >>
> >> +
> >> +static void playlist_reply(DBusPendingCall *call, void *user_data) {
> >> +     struct media_player *mp = user_data;
> >> +     DBusMessage *reply;
> >> +     DBusMessageIter array_iter, entry, struct_iter;
> >> +     DBusError derr;
> >> +     int ctype;
> >> +
> >> +     reply = dbus_pending_call_steal_reply(call);
> >> +
> >> +     dbus_error_init(&derr);
> >> +
> >> +     if (dbus_set_error_from_message(&derr, reply)) {
> >> +             error("Error: GetPlayLists method %s, %s", derr.name,
> >> +                                     derr.message);
> >> +             dbus_error_free(&derr);
> >> +             goto done;
> >> +     }
> >> +
> >> +     dbus_message_iter_init(reply, &array_iter);
> >> +     ctype = dbus_message_iter_get_arg_type(&array_iter);
> >> +
> >> +     if (ctype != DBUS_TYPE_ARRAY)
> >> +             goto done;
> >> +
> >> +     dbus_message_iter_recurse(&array_iter, &struct_iter);
> >> +
> >> +     while ((ctype = dbus_message_iter_get_arg_type(&struct_iter)) !=
> >> +                             DBUS_TYPE_INVALID) {
> >> +             struct media_playlist *playlist;
> >> +
> >> +             if (ctype != DBUS_TYPE_STRUCT) {
> >> +                     error("Invalid type");
> >> +                     goto done;
> >> +             }
> >> +
> >> +             playlist = g_new0(struct media_playlist, 1);
> >> +
> >> +             dbus_message_iter_recurse(&struct_iter, &entry);
> >> +
> >> +             dbus_message_iter_get_basic(&entry, &playlist->id);
> >> +             DBG("Playlists Id %s", playlist->id);
> >> +
> >> +             dbus_message_iter_next(&entry);
> >> +             dbus_message_iter_get_basic(&entry, &playlist->name);
> >> +             DBG("Playlist Name %s", playlist->name);
> >> +
> >> +             dbus_message_iter_next(&entry);
> >> +             dbus_message_iter_get_basic(&entry, &playlist->icon);
> >> +             DBG("Playlist Icon %s", playlist->icon);
> >> +
> >> +             /* TODO: Create Media folder with playlist information
> >> + */
> >> +
> >> +             mp->playlists = g_slist_append(mp->playlists, playlist);
> >> +             mp->total_items++;
> >> +
> >> +             dbus_message_iter_next(&struct_iter);
> >> +     }
> 
> Id expected you to subscribe to playlist changes once you complete reading
> the initial list then every time it changes you will need to refetch the list in
> case it has changed, but fill free to split it so this patch don't become to big.
> 
> >> +done:
> >> +     dbus_message_unref(reply);
> >> +}
> >> +
> >> +static gboolean get_playlists(gpointer user_data) {
> >> +     struct media_player *mp = user_data;
> >> +     DBusMessage *msg;
> >> +     DBusPendingCall *call;
> >> +     gboolean reverse_order = FALSE;
> >> +     uint32_t start_index = 0x0000;
> >> +     uint32_t end_index = 0xFFFF;
> >> +     char *ordering = "Alphabetical";
> >> +     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);
> >> +
> >> +     mp->total_items = 0;
> >> +
> >> +     if ((dbus_connection_send_with_reply(btd_get_dbus_connection(),
> >> +                                     msg, &call, -1))) {
> >> +             dbus_pending_call_set_notify(call, &playlist_reply, mp,
> >> NULL);
> >> +             dbus_pending_call_unref(call);
> >> +             dbus_message_unref(msg);
> >> +     }
> 
> You need to save the DBusPendingCall call to be able to cancel the request in
> case something happens in the meantime, there is similar code already in the
> same file just look at e.g.
> media_endpoint_async_call. (note that I don't want you to copy it exactly
> but it might be a good idea to have pending request for media players but if
> is just GetPlaylist Im fine to just store it inside media_player structure).
Getplaylist and ActivatePlaylist are the methods I am planning to add for Playlist interface as of now. But Tracklist info are also needed for which there are 4 new methods on Tracklist interface. So it will be better to keep  the pending call and make it somewhat similar to media_endpoint_async_call. But though the parser for these methods reply will be quite different from each other I am planning to put a switch case on the basic of method name to dbus_pending_call_set_notify for callbacks.
I will submit v2 with these changes.
> 
> >> +     mp->playlist_id = 0;
> >> +
> >> +     return FALSE;
> >> +}
> >> +
> >>  static struct avrcp_player_cb player_cb = {
> >>       .list_settings = list_settings,
> >>       .get_setting = get_setting,
> >> @@ -1831,6 +1950,8 @@ static DBusMessage
> >> *register_player(DBusConnection *conn, DBusMessage *msg,
> >>               return btd_error_invalid_args(msg);
> >>       }
> >>
> >> +     mp->playlist_id = g_idle_add(get_playlists, mp);
> 
> Since you won't block anymore the call to get_playlists can be done directly
> here.
> 
> >> +
> >>       return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);  }
> >>
> >> --
> >> 1.9.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> > in
> >> the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo
> >> info at  http://vger.kernel.org/majordomo-info.html
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
> --
> Luiz Augusto von Dentz
> --

Regards,
Bharat

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



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux