Hi Luiz,
> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>
> Sent: Friday, October 13, 2023 8:30 PM
> To: Claudia Cristina Draghicescu <claudia.rosu@xxxxxxx>
> Cc: linux-bluetooth@xxxxxxxxxxxxxxx; Iulia Tanasescu
> <iulia.tanasescu@xxxxxxx>; Mihai-Octavian Urzica <mihai-
> octavian.urzica@xxxxxxx>; Silviu Florian Barbulescu
> <silviu.barbulescu@xxxxxxx>; Vlad Pruteanu <vlad.pruteanu@xxxxxxx>;
> Andrei Istodorescu <andrei.istodorescu@xxxxxxx>
> Subject: [EXT] Re: [PATCH BlueZ 1/2] bap: Fix source+sink endpoint
> registration
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> Hi Claudia,
>
> On Fri, Oct 13, 2023 at 3:06 AM Claudia Draghicescu <claudia.rosu@xxxxxxx>
> 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_broadcast(struct bt_bap_db *bdb,
> > if (type == BT_BAP_BCAST_SINK)
> > ep->dir = BT_BAP_BCAST_SOURCE;
> > else
> > - ep->dir = BT_BAP_BCAST_SINK;
> > + ep->dir = BT_BAP_SIMULATED_BCAST_SINK;
> >
> > return ep;
> > }
> > @@ -1500,12 +1500,12 @@ static void ep_config_cb(struct bt_bap_stream
> *stream, int err)
> > return;
> >
> > if (bt_bap_stream_get_type(stream) ==
> BT_BAP_STREAM_TYPE_BCAST) {
> > - if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SINK)
> > + if (bt_bap_stream_io_dir(stream) ==
> > + 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) == BT_BAP_BCAST_SOURCE)
> > stream_set_state_broadcast(stream,
> > - BT_BAP_STREAM_STATE_CONFIG);
> > + BT_BAP_STREAM_STATE_CONFIG);
> > return;
> > }
> >
> > @@ -2710,15 +2710,15 @@ struct bt_bap_pac
> *bt_bap_add_vendor_pac(struct gatt_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 = 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 = 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.
I agree that a broadcast source should be allowed to create an endpoint without
a remote pac. We can try this approach and submit a new patch.
Just to be sure, when you say "remote endpoint" do you mean the remote pac or
other bap ep structure?
>
> > + bap_add_broadcast_sink(pac_broadcast_sink);
> > + queue_foreach(sessions, notify_session_pac_added,
> pac_broadcast_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, struct queue *r,
> > for (er = queue_get_entries(r); er; er = er->next) {
> > struct bt_bap_pac *rpac = er->data;
> >
> > + if ((lpac->type == BT_BAP_BCAST_SOURCE)
> > + && (rpac->type != BT_BAP_SIMULATED_BCAST_SINK))
> > + continue;
> > + if ((rpac->type == BT_BAP_SIMULATED_BCAST_SINK)
> > + && (lpac->type == 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 != BT_BAP_BCAST_SOURCE) &&
> > (!bap_codec_equal(&lpac->codec, &rpac->codec)))
> > continue;
> > -
> > + if (rpac->type == 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_data);
> > 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_sources,
> > - 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_stream *stream,
> > queue_foreach(stream->links, bap_stream_enable_link,
> metadata);
> > break;
> > case BT_BAP_STREAM_TYPE_BCAST:
> > - if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SINK)
> > + if (bt_bap_stream_io_dir(stream) ==
> > + BT_BAP_SIMULATED_BCAST_SINK)
> > stream_set_state_broadcast(stream,
> > - BT_BAP_STREAM_STATE_CONFIG);
> > + BT_BAP_STREAM_STATE_CONFIG);
> > else if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SOURCE)
> > stream_set_state_broadcast(stream,
> > - BT_BAP_STREAM_STATE_STREAMING);
> > + 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
> >
>
>
> --
> Luiz Augusto von Dentz
Regards,
Claudia
