Re: [PATCH BlueZ 1/2] profiles: Add remote endpoint path to SelectProperties

Luiz Augusto von Dentz
2022-08-29T21:55:35Z

Hi Frédéric,

On Mon, Aug 29, 2022 at 1:36 PM Frédéric Danis
<frederic.danis@collabora.com> wrote:
>
> ---
> profiles/audio/bap.c | 2 +-
> profiles/audio/media.c | 3 +++
> src/shared/bap.c | 2 ++
> src/shared/bap.h | 2 ++
> 4 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
> index d388afe56..cf27ec0ae 100644
> --- a/profiles/audio/bap.c
> +++ b/profiles/audio/bap.c
> @@ -613,7 +613,7 @@ static bool pac_found(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
>
> /* TODO: Cache LRU? */
> if (btd_service_is_initiator(service))
> - bt_bap_select(lpac, rpac, select_cb, ep);
> + bt_bap_select(lpac, rpac, ep->path, select_cb, ep);
>
> return true;
> }
> diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> index ff3fa197b..8d777eedd 100644
> --- a/profiles/audio/media.c
> +++ b/profiles/audio/media.c
> @@ -891,6 +891,7 @@ done:
>
> static int pac_select(struct bt_bap_pac *pac, struct bt_bap_pac_qos *qos,
> struct iovec *caps, struct iovec *metadata,
> + const char *remote_ep_path,
> bt_bap_pac_select_t cb, void *cb_data, void *user_data)
> {
> struct media_endpoint *endpoint = user_data;
> @@ -917,6 +918,8 @@ static int pac_select(struct bt_bap_pac *pac, struct bt_bap_pac_qos *qos,
>
> dbus_message_iter_init_append(msg, &iter);
>
> + dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, &remote_ep_path);
> +

If the endpoint object is not part of the dictionary then you will
have to fix client/player.c as well since the signature will be
changing, so I think it is better we include it as part of properties
itself.

> dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, "{sv}", &dict);
>
> g_dbus_dict_append_basic_array(&dict, DBUS_TYPE_STRING, &key,
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index 8edc7b72e..691fec2fa 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -4058,6 +4058,7 @@ static bool match_pac(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
> }
>
> int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
> + const char *remote_ep_path,
> bt_bap_pac_select_t func, void *user_data)
> {
> if (!lpac || !rpac || !func)
> @@ -4067,6 +4068,7 @@ int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
> return -EOPNOTSUPP;
>
> lpac->ops->select(lpac, &rpac->qos, rpac->data, rpac->metadata,
> + remote_ep_path,
> func, user_data, lpac->user_data);
>
> return 0;
> diff --git a/src/shared/bap.h b/src/shared/bap.h
> index ff4bac330..da5fe1431 100644
> --- a/src/shared/bap.h
> +++ b/src/shared/bap.h
> @@ -122,6 +122,7 @@ struct bt_bap_pac *bt_bap_add_pac(struct gatt_db *db, const char *name,
> struct bt_bap_pac_ops {
> int (*select) (struct bt_bap_pac *pac, struct bt_bap_pac_qos *qos,
> struct iovec *caps, struct iovec *metadata,
> + const char *remote_ep_path,

Hmm, Id avoid passing D-Bus specific path here since this API is
suppose to be generic and at some point we might want to create a unit
tests that don't involve the daemon, so instead I think it is better
that we pass the rpac and addition to the lpac and then perhaps have a
function which can attach custom user_data to the rpac e.g.: void
bt_bap_pac_set_user_data(void *); void *bt_bap_pac_get_user_data();

> bt_bap_pac_select_t cb, void *cb_data, void *user_data);
> int (*config) (struct bt_bap_stream *stream, struct iovec *cfg,
> struct bt_bap_qos *qos, bt_bap_pac_config_t cb,
> @@ -188,6 +189,7 @@ int bt_bap_pac_get_codec(struct bt_bap_pac *pac, uint8_t *id,
>
> /* Stream related functions */
> int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
> + const char *remote_ep_path,
> bt_bap_pac_select_t func, void *user_data);
>
> struct bt_bap_stream *bt_bap_config(struct bt_bap *bap,
> --
> 2.25.1
>

--
Luiz Augusto von Dentz