Hi Frédéric,
On Wed, Sep 4, 2024 at 10:33 AM Frédéric Danis
<frederic.danis@xxxxxxxxxxxxx> wrote:
>
> This parse the AVRCP Target SDP record for the L2CAP PSM to use with
> the OBEX session to get the cover art.
> ---
> doc/org.bluez.MediaPlayer.rst | 6 +++++
> profiles/audio/avrcp.c | 51 +++++++++++++++++++++++++++++++----
> profiles/audio/player.c | 29 ++++++++++++++++++++
> profiles/audio/player.h | 1 +
> 4 files changed, 82 insertions(+), 5 deletions(-)
>
> diff --git a/doc/org.bluez.MediaPlayer.rst b/doc/org.bluez.MediaPlayer.rst
> index 60bd679bb..f1e999bdf 100644
> --- a/doc/org.bluez.MediaPlayer.rst
> +++ b/doc/org.bluez.MediaPlayer.rst
> @@ -313,3 +313,9 @@ object Playlist
> ```````````````
>
> Playlist object path.
> +
> +uint16 ObexPort [readonly]
> +`````````````````````````````
> +
> + If present indicates the player can get cover art using BIP over OBEX
> + on this PSM port.
Id pass this via as part of URL e.g. obex://<address>/handle:[port]
That said, like I mentioned in the other change, we could perhaps
attempt to load it directly from bluetoothd since obexd registered its
profile with it the daemon will know that BIP has been registered and
we could allow bluetoothd to create sessions/transfer via that
connection rather than using the session D-Bus, it is a little more
work compared to doing bluetoothctl but then we don't have to
reimplement as part of upper layers or force the system to run
bluetoothctl which is more like a testing tool to validate our API's,
etc.
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index 752e55be3..61558e492 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -118,8 +118,14 @@
> #define AVRCP_FEATURE_CATEGORY_2 0x0002
> #define AVRCP_FEATURE_CATEGORY_3 0x0004
> #define AVRCP_FEATURE_CATEGORY_4 0x0008
> -#define AVRCP_FEATURE_PLAYER_SETTINGS 0x0010
> -#define AVRCP_FEATURE_BROWSING 0x0040
> +#define AVRCP_FEATURE_TG_PLAYER_SETTINGS 0x0010
> +#define AVRCP_FEATURE_TG_GROUP_NAVIGATION 0x0020
> +#define AVRCP_FEATURE_BROWSING 0x0040
> +#define AVRCP_FEATURE_TG_MULTIPLE_PLAYER 0x0080
> +#define AVRCP_FEATURE_TG_COVERT_ART 0x0100
> +#define AVRCP_FEATURE_CT_GET_IMAGE_PROP 0x0080
> +#define AVRCP_FEATURE_CT_GET_IMAGE 0x0100
> +#define AVRCP_FEATURE_CT_GET_THUMBNAIL 0x0200
>
> #define AVRCP_BATTERY_STATUS_NORMAL 0
> #define AVRCP_BATTERY_STATUS_WARNING 1
> @@ -254,6 +260,7 @@ struct avrcp_data {
> struct avrcp_player *player;
> uint16_t version;
> int features;
> + uint16_t obex_port;
> GSList *players;
> };
>
> @@ -487,7 +494,7 @@ static sdp_record_t *avrcp_tg_record(bool browsing)
> AVRCP_FEATURE_CATEGORY_2 |
> AVRCP_FEATURE_CATEGORY_3 |
> AVRCP_FEATURE_CATEGORY_4 |
> - AVRCP_FEATURE_PLAYER_SETTINGS );
> + AVRCP_FEATURE_TG_PLAYER_SETTINGS);
>
> record = sdp_record_alloc();
> if (!record)
> @@ -3522,6 +3529,7 @@ static struct avrcp_player *create_ct_player(struct avrcp *session,
> return NULL;
> }
>
> + media_player_set_obex_port(mp, session->controller->obex_port);
> media_player_set_callbacks(mp, &ct_cbs, player);
> player->user_data = mp;
> player->destroy = (GDestroyNotify) media_player_destroy;
> @@ -4006,7 +4014,8 @@ static gboolean avrcp_get_capabilities_resp(struct avctp *conn, uint8_t code,
> if (events == (1 << AVRCP_EVENT_VOLUME_CHANGED))
> return FALSE;
>
> - if ((session->controller->features & AVRCP_FEATURE_PLAYER_SETTINGS) &&
> + if ((session->controller->features &
> + AVRCP_FEATURE_TG_PLAYER_SETTINGS) &&
> !(events & (1 << AVRCP_EVENT_SETTINGS_CHANGED)))
> avrcp_list_player_attributes(session);
>
> @@ -4075,8 +4084,9 @@ static struct avrcp_data *data_init(struct avrcp *session, const char *uuid)
> {
> struct avrcp_data *data;
> const sdp_record_t *rec;
> - sdp_list_t *list;
> + sdp_list_t *list, *protos;
> sdp_profile_desc_t *desc;
> + int port = 0;
>
> data = g_new0(struct avrcp_data, 1);
>
> @@ -4092,6 +4102,35 @@ static struct avrcp_data *data_init(struct avrcp *session, const char *uuid)
> sdp_get_int_attr(rec, SDP_ATTR_SUPPORTED_FEATURES, &data->features);
> sdp_list_free(list, free);
>
> + if ((g_strcmp0(uuid, AVRCP_TARGET_UUID) != 0) ||
> + !(data->features & AVRCP_FEATURE_TG_COVERT_ART) ||
> + (sdp_get_add_access_protos(rec, &protos) != 0))
> + return data;
> +
> + /* Get the PSM port from the Additional Protocol Descriptor list
> + * entry containing OBEX UUID
> + */
> + for (list = protos; list; list = list->next) {
> + sdp_list_t *p;
> +
> + for (p = list->data; p; p = p->next) {
> + sdp_data_t *seq = p->data;
> +
> + if ((sdp_uuid_to_proto(&seq->val.uuid) == OBEX_UUID) &&
> + SDP_IS_UUID(seq->dtd)) {
> + port = sdp_get_proto_port(list, L2CAP_UUID);
> + goto done;
> + }
> + }
> + }
> +
> +done:
> + if (port > 0)
> + data->obex_port = port;
> +
> + sdp_list_foreach(protos, (sdp_list_func_t) sdp_list_free, NULL);
> + sdp_list_free(protos, NULL);
> +
> return data;
> }
>
> @@ -4189,6 +4228,8 @@ static void controller_init(struct avrcp *session)
> session->controller = controller;
>
> DBG("%p version 0x%04x", controller, controller->version);
> + if (controller->obex_port)
> + DBG("%p OBEX PSM 0x%04x", controller, controller->obex_port);
>
> service = btd_device_get_service(session->dev, AVRCP_TARGET_UUID);
> btd_service_connecting_complete(service, 0);
> diff --git a/profiles/audio/player.c b/profiles/audio/player.c
> index c995697fe..4e3de8047 100644
> --- a/profiles/audio/player.c
> +++ b/profiles/audio/player.c
> @@ -88,6 +88,7 @@ struct media_player {
> struct player_callback *cb;
> GSList *pending;
> GSList *folders;
> + uint16_t obex_port;
> };
>
> static void append_track(void *key, void *value, void *user_data)
> @@ -437,6 +438,28 @@ static gboolean get_playlist(const GDBusPropertyTable *property,
> return TRUE;
> }
>
> +static gboolean obexport_exists(const GDBusPropertyTable *property,
> + void *data)
> +{
> + struct media_player *mp = data;
> +
> + return mp->obex_port != 0;
> +}
> +
> +static gboolean get_obexport(const GDBusPropertyTable *property,
> + DBusMessageIter *iter, void *data)
> +{
> + struct media_player *mp = data;
> +
> + if (mp->obex_port == 0)
> + return FALSE;
> +
> + dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16,
> + &mp->obex_port);
> +
> + return TRUE;
> +}
> +
> static DBusMessage *media_player_play(DBusConnection *conn, DBusMessage *msg,
> void *data)
> {
> @@ -778,6 +801,7 @@ static const GDBusPropertyTable media_player_properties[] = {
> { "Browsable", "b", get_browsable, NULL, browsable_exists },
> { "Searchable", "b", get_searchable, NULL, searchable_exists },
> { "Playlist", "o", get_playlist, NULL, playlist_exists },
> + { "ObexPort", "q", get_obexport, NULL, obexport_exists },
> { }
> };
>
> @@ -1997,3 +2021,8 @@ struct media_item *media_player_set_playlist_item(struct media_player *mp,
>
> return item;
> }
> +
> +void media_player_set_obex_port(struct media_player *mp, uint16_t port)
> +{
> + mp->obex_port = port;
> +}
> diff --git a/profiles/audio/player.h b/profiles/audio/player.h
> index 74fb7d771..0076c4641 100644
> --- a/profiles/audio/player.h
> +++ b/profiles/audio/player.h
> @@ -86,6 +86,7 @@ void media_player_set_folder(struct media_player *mp, const char *path,
> void media_player_set_playlist(struct media_player *mp, const char *name);
> struct media_item *media_player_set_playlist_item(struct media_player *mp,
> uint64_t uid);
> +void media_player_set_obex_port(struct media_player *mp, uint16_t port);
>
> struct media_item *media_player_create_folder(struct media_player *mp,
> const char *name,
> --
> 2.34.1
>
>
--
Luiz Augusto von Dentz
