[BlueZ,v2,0/7] Add BIP for AVRCP covert art OBEX client

If AVRCP Target supports Cover Art download its SDP record contains an
additional access protocol for OBEX with an LCAP PSM on which the OBEX
client session should connect to, with the specific OBEX target header
7163DD54-4A7E-11E2-B47C-0050C2490048.

Once the OBEX session is connected, the AVRCP track metadata will contain
an Image Handle which can be used to get the associated image using
GetImageThumbnail or GetImage with one of the description property found
with GetImageProperties.

v1 -> v2: Parse integer properties as integer instead of string
          Return GetImageProperties values as aa{sv} instead of aa{ss}
          Get properties for GetImage as a{sv} instead of a{ss}
          Set new properties and interface as experimental

Frédéric Danis (7):
  obexd: Add PSM support to session create
  player: Add OBEX PSM port for cover art support
  player: Add image handle support property
  obexd: Add BIP client for AVRCP cover art download.
  obexd: Add GetImageProperties to bip-avrcp
  obexd: Add GetImage to bip-avrcp
  doc: Add description of org.bluez.obex.BipAvrcp

 Makefile.am                     |   9 +-
 Makefile.obexd                  |   2 +
 doc/org.bluez.MediaPlayer.rst   |  11 +
 doc/org.bluez.obex.BipAvrcp.rst |  72 +++
 doc/org.bluez.obex.Client.rst   |   5 +
 doc/org.bluez.obex.Session.rst  |   5 +
 monitor/avctp.c                 |   3 +
 obexd/client/bip-avrcp.c        | 427 +++++++++++++++++
 obexd/client/bip-avrcp.h        |  12 +
 obexd/client/bip-common.c       | 800 ++++++++++++++++++++++++++++++++
 obexd/client/bip-common.h       |  24 +
 obexd/client/manager.c          |  16 +-
 obexd/client/session.c          |  27 +-
 obexd/client/session.h          |   1 +
 obexd/client/transfer.c         |  16 +
 obexd/client/transfer.h         |   2 +
 profiles/audio/avrcp.c          |  64 ++-
 profiles/audio/avrcp.h          |   3 +-
 profiles/audio/player.c         |  30 ++
 profiles/audio/player.h         |   1 +
 tools/parser/avrcp.c            |   3 +
 21 files changed, 1514 insertions(+), 19 deletions(-)
 create mode 100644 doc/org.bluez.obex.BipAvrcp.rst
 create mode 100644 obexd/client/bip-avrcp.c
 create mode 100644 obexd/client/bip-avrcp.h
 create mode 100644 obexd/client/bip-common.c
 create mode 100644 obexd/client/bip-common.h

Hi Frédéric,

On Thu, Sep 12, 2024 at 1:50 PM Frédéric Danis
<frederic.danis@collabora.com> wrote:
>
> The cover art image handle is available in the metadata of the track
> when the OBEX BIP session is connected to the PSM port provided
> in AVRCP SDP record and available as org.bluez.MediaPlayer property.
>
> This service allows to get the thumbnail.
> ---
>  Makefile.obexd           |   1 +
>  obexd/client/bip-avrcp.c | 171 +++++++++++++++++++++++++++++++++++++++
>  obexd/client/bip-avrcp.h |  12 +++

Id got with bit.{c,h} so we can reuse the implementation for BIP.

>  obexd/client/manager.c   |   2 +
>  obexd/client/transfer.c  |  16 ++++
>  obexd/client/transfer.h  |   2 +
>  6 files changed, 204 insertions(+)
>  create mode 100644 obexd/client/bip-avrcp.c
>  create mode 100644 obexd/client/bip-avrcp.h
>
> diff --git a/Makefile.obexd b/Makefile.obexd
> index 4cdce73af..8038d2327 100644
> --- a/Makefile.obexd
> +++ b/Makefile.obexd
> @@ -81,6 +81,7 @@ obexd_src_obexd_SOURCES = $(btio_sources) $(gobex_sources) \
>                         obexd/client/ftp.h obexd/client/ftp.c \
>                         obexd/client/opp.h obexd/client/opp.c \
>                         obexd/client/map.h obexd/client/map.c \
> +                       obexd/client/bip-avrcp.h obexd/client/bip-avrcp.c \
>                         obexd/client/map-event.h obexd/client/map-event.c \
>                         obexd/client/transfer.h obexd/client/transfer.c \
>                         obexd/client/transport.h obexd/client/transport.c \
> diff --git a/obexd/client/bip-avrcp.c b/obexd/client/bip-avrcp.c
> new file mode 100644
> index 000000000..ca702be5a
> --- /dev/null
> +++ b/obexd/client/bip-avrcp.c
> @@ -0,0 +1,171 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + *
> + *  OBEX Client
> + *
> + *  Copyright (C) 2024  Collabora Ltd.
> + *
> + *
> + */
> +
> +#define _GNU_SOURCE
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include "gdbus/gdbus.h"
> +#include "gobex/gobex.h"
> +
> +#include "obexd/src/log.h"
> +#include "transfer.h"
> +#include "session.h"
> +#include "driver.h"
> +#include "bip-avrcp.h"
> +
> +#define OBEX_BIP_AVRCP_UUID \
> +       "\x71\x63\xDD\x54\x4A\x7E\x11\xE2\xB4\x7C\x00\x50\xC2\x49\x00\x48"
> +#define OBEX_BIP_AVRCP_UUID_LEN 16
> +
> +#define BIP_AVRCP_INTERFACE "org.bluez.obex.BipAvrcp1"
> +#define ERROR_INTERFACE "org.bluez.obex.Error"
> +#define BIP_AVRCP_UUID "0000111A-0000-1000-8000-00805f9b34fb"
> +
> +#define IMG_HANDLE_TAG  0x30
> +
> +static DBusConnection *conn;
> +
> +struct bip_avrcp_data {
> +       struct obc_session *session;
> +};
> +
> +static DBusMessage *get_image_thumbnail(DBusConnection *connection,
> +                                       DBusMessage *message, void *user_data)
> +{
> +       struct bip_avrcp_data *bip_avrcp = user_data;
> +       const char *handle = NULL, *image_path = NULL;
> +       struct obc_transfer *transfer;
> +       GObexHeader *header;
> +       DBusMessage *reply = NULL;
> +       GError *err = NULL;
> +
> +       DBG("");
> +
> +       if (dbus_message_get_args(message, NULL,
> +                               DBUS_TYPE_STRING, &image_path,
> +                               DBUS_TYPE_STRING, &handle,
> +                               DBUS_TYPE_INVALID) == FALSE) {
> +               reply = g_dbus_create_error(message,
> +                               ERROR_INTERFACE ".InvalidArguments", NULL);
> +               goto fail;
> +       }
> +
> +       transfer = obc_transfer_get("x-bt/img-thm", NULL, image_path, &err);
> +       if (transfer == NULL)
> +               goto fail;
> +
> +       header = g_obex_header_new_unicode(IMG_HANDLE_TAG, handle);
> +       obc_transfer_add_header(transfer, header);
> +
> +       if (!obc_session_queue(bip_avrcp->session, transfer, NULL, NULL, &err))
> +               goto fail;
> +
> +       return obc_transfer_create_dbus_reply(transfer, message);
> +
> +fail:
> +       reply = g_dbus_create_error(message, ERROR_INTERFACE ".Failed", "%s",
> +                                                               err->message);
> +       g_error_free(err);
> +       return reply;
> +}
> +
> +static const GDBusMethodTable bip_avrcp_methods[] = {
> +       { GDBUS_ASYNC_METHOD("GetImageThumbnail",
> +               GDBUS_ARGS({ "file", "s" }, { "handle", "s"}),
> +               GDBUS_ARGS({ "transfer", "o" }, { "properties", "a{sv}" }),
> +               get_image_thumbnail) },
> +       { }
> +};
> +
> +static void bip_avrcp_free(void *data)
> +{
> +       struct bip_avrcp_data *bip_avrcp = data;
> +
> +       obc_session_unref(bip_avrcp->session);
> +       g_free(bip_avrcp);
> +}
> +
> +static int bip_avrcp_probe(struct obc_session *session)
> +{
> +       struct bip_avrcp_data *bip_avrcp;
> +       const char *path;
> +
> +       path = obc_session_get_path(session);
> +
> +       DBG("%s", path);
> +
> +       bip_avrcp = g_try_new0(struct bip_avrcp_data, 1);
> +       if (!bip_avrcp)
> +               return -ENOMEM;
> +
> +       bip_avrcp->session = obc_session_ref(session);
> +
> +       if (!g_dbus_register_interface(conn, path, BIP_AVRCP_INTERFACE,
> +                                       bip_avrcp_methods,
> +                                       NULL, NULL,
> +                                       bip_avrcp, bip_avrcp_free)) {
> +               bip_avrcp_free(bip_avrcp);
> +               return -ENOMEM;
> +       }
> +
> +       return 0;
> +}
> +
> +static void bip_avrcp_remove(struct obc_session *session)
> +{
> +       const char *path = obc_session_get_path(session);
> +
> +       DBG("%s", path);
> +
> +       g_dbus_unregister_interface(conn, path, BIP_AVRCP_INTERFACE);
> +}
> +
> +static struct obc_driver bip_avrcp = {
> +       .service = "BIP-AVRCP",
> +       .uuid = BIP_AVRCP_UUID,
> +       .target = OBEX_BIP_AVRCP_UUID,
> +       .target_len = OBEX_BIP_AVRCP_UUID_LEN,
> +       .probe = bip_avrcp_probe,
> +       .remove = bip_avrcp_remove
> +};

This can stay as bip_arvcp, since obc_driver is per-uuid, but in the
future if someone wants to add BIP uuid it just registers another
driver.

> +int bip_avrcp_init(void)
> +{
> +       int err;
> +
> +       DBG("");
> +
> +       conn = dbus_bus_get(DBUS_BUS_SESSION, NULL);
> +       if (!conn)
> +               return -EIO;
> +
> +       err = obc_driver_register(&bip_avrcp);
> +       if (err < 0)
> +               goto failed;
> +
> +       return 0;
> +
> +failed:
> +       dbus_connection_unref(conn);
> +       conn = NULL;
> +       return err;
> +}
> +
> +void bip_avrcp_exit(void)
> +{
> +       DBG("");
> +
> +       dbus_connection_unref(conn);
> +       conn = NULL;
> +
> +       obc_driver_unregister(&bip_avrcp);
> +}
> diff --git a/obexd/client/bip-avrcp.h b/obexd/client/bip-avrcp.h
> new file mode 100644
> index 000000000..acd8068f2
> --- /dev/null
> +++ b/obexd/client/bip-avrcp.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + *
> + *  OBEX Client
> + *
> + *  Copyright (C) 2024  Collabora Ltd.
> + *
> + *
> + */
> +
> +int bip_avrcp_init(void);
> +void bip_avrcp_exit(void);
> diff --git a/obexd/client/manager.c b/obexd/client/manager.c
> index 52c00fb0c..5078f56da 100644
> --- a/obexd/client/manager.c
> +++ b/obexd/client/manager.c
> @@ -32,6 +32,7 @@
>  #include "pbap.h"
>  #include "sync.h"
>  #include "map.h"
> +#include "bip-avrcp.h"
>  #include "manager.h"
>
>  #define CLIENT_INTERFACE       "org.bluez.obex.Client1"
> @@ -258,6 +259,7 @@ static const struct obc_module {
>         { "pbap", pbap_init, pbap_exit },
>         { "sync", sync_init, sync_exit },
>         { "map", map_init, map_exit },
> +       { "bip-avrcp", bip_avrcp_init, bip_avrcp_exit },

For the module it shall be just bip, avrcp specific UUID is handled by
a dedicated driver.

>         { }
>  };
>
> diff --git a/obexd/client/transfer.c b/obexd/client/transfer.c
> index a7a85a0c0..fdf32a985 100644
> --- a/obexd/client/transfer.c
> +++ b/obexd/client/transfer.c
> @@ -57,6 +57,7 @@ struct obc_transfer {
>         GObex *obex;
>         uint8_t status;
>         GObexApparam *apparam;
> +       GSList *headers;
>         guint8 op;
>         struct transfer_callback *callback;
>         DBusConnection *conn;
> @@ -820,6 +821,16 @@ static gboolean transfer_start_get(struct obc_transfer *transfer, GError **err)
>                 g_obex_packet_add_bytes(req, G_OBEX_HDR_TYPE, transfer->type,
>                                                 strlen(transfer->type) + 1);
>
> +       if (transfer->headers != NULL) {
> +               GSList *l;
> +
> +               for (l = transfer->headers; l != NULL; l = g_slist_next(l)) {
> +                       GObexHeader *hdr = l->data;
> +
> +                       g_obex_packet_add_header(req, hdr);
> +               }
> +       }

This one seems to be a different change, so Id put it in a different patch.

> +
>         if (transfer->apparam != NULL) {
>                 hdr = g_obex_header_new_apparam(transfer->apparam);
>                 g_obex_packet_add_header(req, hdr);
> @@ -974,3 +985,8 @@ gint64 obc_transfer_get_size(struct obc_transfer *transfer)
>  {
>         return transfer->size;
>  }
> +
> +void obc_transfer_add_header(struct obc_transfer *transfer, void *data)
> +{
> +       transfer->headers = g_slist_append(transfer->headers, data);
> +}
> diff --git a/obexd/client/transfer.h b/obexd/client/transfer.h
> index 323332a62..1ed195984 100644
> --- a/obexd/client/transfer.h
> +++ b/obexd/client/transfer.h
> @@ -47,3 +47,5 @@ gint64 obc_transfer_get_size(struct obc_transfer *transfer);
>
>  DBusMessage *obc_transfer_create_dbus_reply(struct obc_transfer *transfer,
>                                                         DBusMessage *message);
> +
> +void obc_transfer_add_header(struct obc_transfer *transfer, void *data);
> --
> 2.34.1
>
>