Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220829203122.51343-1-frederic.danis@collabora.com> <20220829203122.51343-2-frederic.danis@collabora.com>
In-Reply-To: <20220829203122.51343-2-frederic.danis@collabora.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Mon, 29 Aug 2022 14:55:18 -0700
Message-ID: <CABBYNZ+JrmO6T5u+06XjiP9UZ-0Smi436-5jfPQ8fFOOSJMQ4g@mail.gmail.com>
Subject: Re: [PATCH BlueZ 1/2] profiles: Add remote endpoint path to SelectProperties
To:     =?UTF-8?B?RnLDqWTDqXJpYyBEYW5pcw==?= <frederic.danis@collabora.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Fr=C3=A9d=C3=A9ric,

On Mon, Aug 29, 2022 at 1:36 PM Fr=C3=A9d=C3=A9ric 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 =3D 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, &rem=
ote_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, stru=
ct 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 b=
t_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, uint=
8_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
>


--=20
Luiz Augusto von Dentz