Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-1799-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240212153222.8191-1-silviu.barbulescu@nxp.com> <20240212153222.8191-3-silviu.barbulescu@nxp.com>
In-Reply-To: <20240212153222.8191-3-silviu.barbulescu@nxp.com>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Mon, 12 Feb 2024 17:17:52 -0500
Message-ID: <CABBYNZ+EsY+P=sbZ-QDeWzzgNk-Q0A_8THxJDg-hONjyv-LXVg@mail.gmail.com>
Subject: Re: [PATCH BlueZ v2 2/7] shared/bap: Create multiple streams for the
 same pac
To: Silviu Florian Barbulescu <silviu.barbulescu@nxp.com>
Cc: linux-bluetooth@vger.kernel.org, mihai-octavian.urzica@nxp.com, 
	vlad.pruteanu@nxp.com, andrei.istodorescu@nxp.com, iulia.tanasescu@nxp.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Silviu,

On Mon, Feb 12, 2024 at 10:32=E2=80=AFAM Silviu Florian Barbulescu
<silviu.barbulescu@nxp.com> wrote:
>
> Broadcast source requires to create multiple streams for one pac.
> This is required for multiple BISes support.
> ---
>  src/shared/bap.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index e26dbf944..f0ffdebfe 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -5156,7 +5156,7 @@ struct bt_bap_stream *bt_bap_stream_new(struct bt_b=
ap *bap,
>                                         struct iovec *data)
>  {
>         struct bt_bap_stream *stream;
> -       struct bt_bap_endpoint *ep;
> +       struct bt_bap_endpoint *ep =3D NULL;
>         struct match_pac match;
>
>         if (!bap)
> @@ -5212,12 +5212,32 @@ struct bt_bap_stream *bt_bap_stream_new(struct bt=
_bap *bap,
>         match.lpac =3D lpac;
>         match.rpac =3D rpac;
>
> -       /* Check for existing stream */
> -       ep =3D queue_find(bap->remote_eps, find_ep_pacs, &match);
> +       /* Broadcast source supports multiple endpoints (multiple BISes)
> +        * for one pac so allow it to register a new endpoint even if
> +        * others already exist.
> +        */
> +       if (lpac->type !=3D BT_BAP_BCAST_SOURCE) {
> +               /* Check for existing stream */
> +               ep =3D queue_find(bap->remote_eps, find_ep_pacs, &match);
> +       }
> +
>         if (!ep) {
>                 /* Check for unused ASE */
>                 ep =3D queue_find(bap->remote_eps, find_ep_unused, &match=
);
> -               if (!ep) {
> +               if (!ep && lpac->type =3D=3D BT_BAP_BCAST_SOURCE) {
> +                       /* Push a new remote endpoint with direction
> +                        * broadcast source
> +                        */
> +                       ep =3D bap_endpoint_new_broadcast(bap->rdb,
> +                                       BT_BAP_BCAST_SOURCE);
> +
> +                       if (ep)
> +                               queue_push_tail(bap->remote_eps, ep);
> +                       else {
> +                               DBG(bap, "Unable to create endpoint");
> +                               return NULL;
> +                       }
> +               } else if (!ep) {

Perhaps we need a dedicated callback from creating streams to avoid
having to add such checks everywhere, that said I don't quite
understand why we have Broadcast Source to find a remote endpoint, it
shall use a local one instead and perhaps have the ep struct support
multiple streams to be attached so we don't have to create dummy
endpoints just to be able to create new streams.

>                         DBG(bap, "Unable to find unused ASE");
>                         return NULL;
>                 }
> --
> 2.39.2
>


--=20
Luiz Augusto von Dentz