Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
MIME-Version: 1.0
References: <20231013100537.3867-1-claudia.rosu@nxp.com> <20231013100537.3867-2-claudia.rosu@nxp.com>
In-Reply-To: <20231013100537.3867-2-claudia.rosu@nxp.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Fri, 13 Oct 2023 10:29:46 -0700
Message-ID: <CABBYNZL1UJkH8_bUv-qRW9TZSAwhrw29_-R5xjnz9B5b+EO5ng@mail.gmail.com>
Subject: Re: [PATCH BlueZ 1/2] bap: Fix source+sink endpoint registration
To:     Claudia Draghicescu <claudia.rosu@nxp.com>
Cc:     linux-bluetooth@vger.kernel.org, iulia.tanasescu@nxp.com,
        mihai-octavian.urzica@nxp.com, silviu.barbulescu@nxp.com,
        vlad.pruteanu@nxp.com, andrei.istodorescu@nxp.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Claudia,

On Fri, Oct 13, 2023 at 3:06=E2=80=AFAM Claudia Draghicescu
<claudia.rosu@nxp.com> wrote:
>
> Create new endpoint name for the simulated broadcast sink that is
> created when registering a broadcast source endpoint.
> This removes the ambiguity when having registered a local
> broadcast sink and fixes the situation when multiple remote
> endpoints are created when discovering a broadcast source.
>
> ---
>  src/shared/bap.c | 54 +++++++++++++++++++++++++++---------------------
>  src/shared/bap.h |  1 +
>  2 files changed, 32 insertions(+), 23 deletions(-)
>
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index 925501c48..266116235 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -644,7 +644,7 @@ static struct bt_bap_endpoint *bap_endpoint_new_broad=
cast(struct bt_bap_db *bdb,
>         if (type =3D=3D BT_BAP_BCAST_SINK)
>                 ep->dir =3D BT_BAP_BCAST_SOURCE;
>         else
> -               ep->dir =3D BT_BAP_BCAST_SINK;
> +               ep->dir =3D BT_BAP_SIMULATED_BCAST_SINK;
>
>         return ep;
>  }
> @@ -1500,12 +1500,12 @@ static void ep_config_cb(struct bt_bap_stream *st=
ream, int err)
>                 return;
>
>         if (bt_bap_stream_get_type(stream) =3D=3D BT_BAP_STREAM_TYPE_BCAS=
T) {
> -               if (bt_bap_stream_io_dir(stream) =3D=3D BT_BAP_BCAST_SINK=
)
> +               if (bt_bap_stream_io_dir(stream) =3D=3D BT_BAP_SIMULATED_=
BCAST_SINK)
>                         stream_set_state_broadcast(stream,
> -                                               BT_BAP_STREAM_STATE_QOS);
> +                               BT_BAP_STREAM_STATE_QOS);
>                 else if (bt_bap_stream_io_dir(stream) =3D=3D BT_BAP_BCAST=
_SOURCE)
>                         stream_set_state_broadcast(stream,
> -                                               BT_BAP_STREAM_STATE_CONFI=
G);
> +                               BT_BAP_STREAM_STATE_CONFIG);
>                 return;
>         }
>
> @@ -2710,15 +2710,15 @@ struct bt_bap_pac *bt_bap_add_vendor_pac(struct g=
att_db *db,
>                 break;
>         case BT_BAP_BCAST_SOURCE:
>                 bap_add_broadcast_source(pac);
> -               if (queue_isempty(bdb->broadcast_sinks)) {
> -                       /* When adding a local broadcast source, add also=
 a
> -                        * local broadcast sink
> -                        */
> -                       pac_broadcast_sink =3D bap_pac_new(bdb, name,
> -                                       BT_BAP_BCAST_SINK, &codec, qos,
> -                                       data, metadata);
> -                       bap_add_broadcast_sink(pac_broadcast_sink);
> -               }
> +               /* When adding a local broadcast source, add also a
> +                * local broadcast sink
> +                */
> +               pac_broadcast_sink =3D bap_pac_new(bdb, name,
> +                       BT_BAP_SIMULATED_BCAST_SINK, &codec, qos,
> +                       data, metadata);

I'm not really sure why this is needed to begin with, if that is to
have a remote endpoint I'd say we need to change the logic so
broadcast src role is allowed to create streams without a remote
endpoint, anyway we should probably have better documentation around
this logic.

> +               bap_add_broadcast_sink(pac_broadcast_sink);
> +               queue_foreach(sessions, notify_session_pac_added, pac_bro=
adcast_sink);
> +               return pac;
>                 break;
>         case BT_BAP_BCAST_SINK:
>                 bap_add_broadcast_sink(pac);
> @@ -4457,13 +4457,23 @@ static void bap_foreach_pac(struct queue *l, stru=
ct queue *r,
>                 for (er =3D queue_get_entries(r); er; er =3D er->next) {
>                         struct bt_bap_pac *rpac =3D er->data;
>
> +                       if ((lpac->type =3D=3D BT_BAP_BCAST_SOURCE)
> +                               && (rpac->type !=3D BT_BAP_SIMULATED_BCAS=
T_SINK))
> +                               continue;
> +                       if ((rpac->type =3D=3D BT_BAP_SIMULATED_BCAST_SIN=
K)
> +                               && (lpac->type =3D=3D BT_BAP_BCAST_SOURCE=
)) {
> +                               func(lpac, rpac, user_data);
> +                               return;
> +                       }
> +
>                         /* Skip checking codec for bcast source,
>                          * it will be checked when BASE info are received
>                          */
>                         if ((rpac->type !=3D BT_BAP_BCAST_SOURCE) &&
>                                 (!bap_codec_equal(&lpac->codec, &rpac->co=
dec)))
>                                 continue;
> -
> +                       if (rpac->type =3D=3D BT_BAP_SIMULATED_BCAST_SINK=
)
> +                               continue;
>                         if (!func(lpac, rpac, user_data))
>                                 return;
>                 }
> @@ -4484,12 +4494,6 @@ void bt_bap_foreach_pac(struct bt_bap *bap, uint8_=
t type,
>                 return bap_foreach_pac(bap->ldb->sinks, bap->rdb->sources=
,
>                                                            func, user_dat=
a);
>         case BT_BAP_BCAST_SOURCE:
> -               if (queue_isempty(bap->rdb->broadcast_sources)
> -                       && queue_isempty(bap->rdb->broadcast_sinks))
> -                       return bap_foreach_pac(bap->ldb->broadcast_source=
s,
> -                                       bap->ldb->broadcast_sinks,
> -                                       func, user_data);
> -
>                 return bap_foreach_pac(bap->ldb->broadcast_sinks,
>                                         bap->rdb->broadcast_sources,
>                                         func, user_data);
> @@ -4497,6 +4501,10 @@ void bt_bap_foreach_pac(struct bt_bap *bap, uint8_=
t type,
>                 return bap_foreach_pac(bap->ldb->broadcast_sinks,
>                                         bap->rdb->broadcast_sources,
>                                         func, user_data);
> +       case BT_BAP_SIMULATED_BCAST_SINK:
> +               return bap_foreach_pac(bap->ldb->broadcast_sources,
> +                                       bap->ldb->broadcast_sinks,
> +                                       func, user_data);
>         }
>  }
>
> @@ -4927,12 +4935,12 @@ unsigned int bt_bap_stream_enable(struct bt_bap_s=
tream *stream,
>                 queue_foreach(stream->links, bap_stream_enable_link, meta=
data);
>                 break;
>         case BT_BAP_STREAM_TYPE_BCAST:
> -               if (bt_bap_stream_io_dir(stream) =3D=3D BT_BAP_BCAST_SINK=
)
> +               if (bt_bap_stream_io_dir(stream) =3D=3D BT_BAP_SIMULATED_=
BCAST_SINK)
>                         stream_set_state_broadcast(stream,
> -                                               BT_BAP_STREAM_STATE_CONFI=
G);
> +                               BT_BAP_STREAM_STATE_CONFIG);
>                 else if (bt_bap_stream_io_dir(stream) =3D=3D BT_BAP_BCAST=
_SOURCE)
>                         stream_set_state_broadcast(stream,
> -                                               BT_BAP_STREAM_STATE_STREA=
MING);
> +                                BT_BAP_STREAM_STATE_STREAMING);
>
>                 return 1;
>         }
> diff --git a/src/shared/bap.h b/src/shared/bap.h
> index ebe4dbf7d..af3b6be71 100644
> --- a/src/shared/bap.h
> +++ b/src/shared/bap.h
> @@ -19,6 +19,7 @@
>  #define        BT_BAP_SOURCE                   0x02
>  #define        BT_BAP_BCAST_SOURCE             0x03
>  #define        BT_BAP_BCAST_SINK               0x04
> +#define BT_BAP_SIMULATED_BCAST_SINK    0x05
>
>  #define BT_BAP_STREAM_TYPE_UCAST       0x01
>  #define        BT_BAP_STREAM_TYPE_BCAST        0x02
> --
> 2.39.2
>


--=20
Luiz Augusto von Dentz