Hi Pauli,
On Mon, Jan 30, 2023 at 12:04 PM Pauli Virtanen <pav@xxxxxx> wrote:
>
> Hi Luiz,
>
> ma, 2023-01-30 kello 11:39 -0800, Luiz Augusto von Dentz kirjoitti:
> > On Mon, Jan 30, 2023 at 11:06 AM Pauli Virtanen <pav@xxxxxx> wrote:
> > >
> > > Support central-only case, where no local endpoints are created, and
> > > nothing is registered in the local GATT database.
> > >
> > > In this case, we keep track of the local PACs ("sinks" and "sources"),
> > > but there are no PACS and ASCS registrations or endpoints.
> >
> > I'm not really following how that would work without local endpoints
> > since the endpoints are the interface which is used to setup the
> > stream with the likes of pipewire/pulseaudio we can't really remove
> > them, what Id would suggest doing is to have a flag indicating if the
> > instance is client only or not, that way bt_bap can identify if it
> > needs to register services or not.
>
> What I tried to say here is that in the central-only (or client in
> other words) mode, no ASEs or PACs are registered in the local GATT DB.
> The local endpoints do still exist in the Media1 API. In shared/bap.c
> they are always associated with the PACs (not ASEs), so the client-only
> mode does not need ASEs. So what is done is to my understanding what
> you write above --- maybe I am missing something?
Make sense.
> I've tested playback on this patchset for pipewire -> pipewire between
> two machines, with the CIS Central flag forced to false on one end, and
> CIS Peripheral false on the other end. Also playback to non-BlueZ
> device works with CIS Peripheral flag disabled.
Alright, make sure it also works with the likes of bluetoothctl -e as
well, since that is likely how the CI will be testing this in the
future.
> >
> > > ---
> > > src/shared/bap.c | 58 ++++++++++++++++++++++++++----------------------
> > > src/shared/bap.h | 9 +++++---
> > > unit/test-bap.c | 4 +++-
> > > 3 files changed, 40 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/src/shared/bap.c b/src/shared/bap.c
> > > index 22f2e6714..24113bb34 100644
> > > --- a/src/shared/bap.c
> > > +++ b/src/shared/bap.c
> > > @@ -110,11 +110,13 @@ struct bt_ascs {
> > >
> > > struct bt_bap_db {
> > > struct gatt_db *db;
> > > - struct bt_pacs *pacs;
> > > - struct bt_ascs *ascs;
> > > struct queue *sinks;
> > > struct queue *sources;
> > > +
> > > + /* The following are NULL if central-only (no GATT) */
> > > struct queue *endpoints;
> > > + struct bt_pacs *pacs;
> > > + struct bt_ascs *ascs;
> > > };
> > >
> > > struct bt_bap_req {
> > > @@ -565,6 +567,7 @@ static void bap_disconnected(int err, void *user_data)
> > > static struct bt_bap *bap_get_session(struct bt_att *att, struct gatt_db *db)
> > > {
> > > const struct queue_entry *entry;
> > > + struct bt_bap_db *ldb;
> > > struct bt_bap *bap;
> > >
> > > for (entry = queue_get_entries(sessions); entry; entry = entry->next) {
> > > @@ -574,7 +577,8 @@ static struct bt_bap *bap_get_session(struct bt_att *att, struct gatt_db *db)
> > > return bap;
> > > }
> > >
> > > - bap = bt_bap_new(db, NULL);
> > > + ldb = bt_bap_get_local_db(db, true);
> > > + bap = bt_bap_new(ldb, NULL);
> > > bap->att = att;
> > >
> > > bt_bap_attach(bap, NULL);
> > > @@ -620,7 +624,7 @@ static struct bt_bap_endpoint *bap_get_endpoint(struct bt_bap_db *db,
> > > {
> > > struct bt_bap_endpoint *ep;
> > >
> > > - if (!db || !attr)
> > > + if (!db || !attr || !db->endpoints)
> > > return NULL;
> > >
> > > ep = queue_find(db->endpoints, bap_endpoint_match, attr);
> > > @@ -652,7 +656,7 @@ static struct bt_bap_endpoint *bap_get_endpoint_id(struct bt_bap *bap,
> > > struct gatt_db_attribute *attr = NULL;
> > > size_t i;
> > >
> > > - if (!bap || !db)
> > > + if (!bap || !db || !db->endpoints)
> > > return NULL;
> > >
> > > ep = queue_find(db->endpoints, bap_endpoint_match_id, UINT_TO_PTR(id));
> > > @@ -2170,7 +2174,7 @@ static struct bt_ascs *ascs_new(struct gatt_db *db)
> > > return ascs;
> > > }
> > >
> > > -static struct bt_bap_db *bap_db_new(struct gatt_db *db)
> > > +static struct bt_bap_db *bap_db_new(struct gatt_db *db, bool peripheral)
> > > {
> > > struct bt_bap_db *bdb;
> > >
> > > @@ -2181,23 +2185,26 @@ static struct bt_bap_db *bap_db_new(struct gatt_db *db)
> > > bdb->db = gatt_db_ref(db);
> > > bdb->sinks = queue_new();
> > > bdb->sources = queue_new();
> > > - bdb->endpoints = queue_new();
> > >
> > > if (!bap_db)
> > > bap_db = queue_new();
> > >
> > > - bdb->pacs = pacs_new(db);
> > > - bdb->pacs->bdb = bdb;
> > > + if (peripheral) {
> > > + bdb->endpoints = queue_new();
> > >
> > > - bdb->ascs = ascs_new(db);
> > > - bdb->ascs->bdb = bdb;
> > > + bdb->pacs = pacs_new(db);
> > > + bdb->pacs->bdb = bdb;
> > > +
> > > + bdb->ascs = ascs_new(db);
> > > + bdb->ascs->bdb = bdb;
> > > + }
> > >
> > > queue_push_tail(bap_db, bdb);
> > >
> > > return bdb;
> > > }
> > >
> > > -static struct bt_bap_db *bap_get_db(struct gatt_db *db)
> > > +struct bt_bap_db *bt_bap_get_local_db(struct gatt_db *db, bool peripheral)
> > > {
> > > struct bt_bap_db *bdb;
> > >
> > > @@ -2205,7 +2212,7 @@ static struct bt_bap_db *bap_get_db(struct gatt_db *db)
> > > if (bdb)
> > > return bdb;
> > >
> > > - return bap_db_new(db);
> > > + return bap_db_new(db, peripheral);
> > > }
> > >
> > > static struct bt_pacs *bap_get_pacs(struct bt_bap *bap)
> > > @@ -2328,6 +2335,9 @@ static void bap_add_sink(struct bt_bap_pac *pac)
> > >
> > > queue_push_tail(pac->bdb->sinks, pac);
> > >
> > > + if (!pac->bdb->endpoints)
> > > + return;
> > > +
> > > memset(value, 0, sizeof(value));
> > >
> > > iov.iov_base = value;
> > > @@ -2346,6 +2356,9 @@ static void bap_add_source(struct bt_bap_pac *pac)
> > >
> > > queue_push_tail(pac->bdb->sources, pac);
> > >
> > > + if (!pac->bdb->endpoints)
> > > + return;
> > > +
> > > memset(value, 0, sizeof(value));
> > >
> > > iov.iov_base = value;
> > > @@ -2373,21 +2386,16 @@ static void notify_session_pac_added(void *data, void *user_data)
> > > queue_foreach(bap->pac_cbs, notify_pac_added, user_data);
> > > }
> > >
> > > -struct bt_bap_pac *bt_bap_add_vendor_pac(struct gatt_db *db,
> > > +struct bt_bap_pac *bt_bap_add_vendor_pac(struct bt_bap_db *bdb,
> > > const char *name, uint8_t type,
> > > uint8_t id, uint16_t cid, uint16_t vid,
> > > struct bt_bap_pac_qos *qos,
> > > struct iovec *data,
> > > struct iovec *metadata)
> > > {
> > > - struct bt_bap_db *bdb;
> > > struct bt_bap_pac *pac;
> > > struct bt_bap_codec codec;
> > >
> > > - if (!db)
> > > - return NULL;
> > > -
> > > - bdb = bap_get_db(db);
> > > if (!bdb)
> > > return NULL;
> > >
> > > @@ -2417,13 +2425,13 @@ struct bt_bap_pac *bt_bap_add_vendor_pac(struct gatt_db *db,
> > > return pac;
> > > }
> > >
> > > -struct bt_bap_pac *bt_bap_add_pac(struct gatt_db *db, const char *name,
> > > +struct bt_bap_pac *bt_bap_add_pac(struct bt_bap_db *bdb, const char *name,
> > > uint8_t type, uint8_t id,
> > > struct bt_bap_pac_qos *qos,
> > > struct iovec *data,
> > > struct iovec *metadata)
> > > {
> > > - return bt_bap_add_vendor_pac(db, name, type, id, 0x0000, 0x0000, qos,
> > > + return bt_bap_add_vendor_pac(bdb, name, type, id, 0x0000, 0x0000, qos,
> > > data, metadata);
> > > }
> > >
> > > @@ -2635,7 +2643,7 @@ static void bap_attached(void *data, void *user_data)
> > > cb->attached(bap, cb->user_data);
> > > }
> > >
> > > -struct bt_bap *bt_bap_new(struct gatt_db *ldb, struct gatt_db *rdb)
> > > +struct bt_bap *bt_bap_new(struct bt_bap_db *ldb, struct gatt_db *rdb)
> > > {
> > > struct bt_bap *bap;
> > > struct bt_bap_db *bdb;
> > > @@ -2643,12 +2651,8 @@ struct bt_bap *bt_bap_new(struct gatt_db *ldb, struct gatt_db *rdb)
> > > if (!ldb)
> > > return NULL;
> > >
> > > - bdb = bap_get_db(ldb);
> > > - if (!bdb)
> > > - return NULL;
> > > -
> > > bap = new0(struct bt_bap, 1);
> > > - bap->ldb = bdb;
> > > + bap->ldb = ldb;
> > > bap->reqs = queue_new();
> > > bap->pending = queue_new();
> > > bap->notify = queue_new();
> > > diff --git a/src/shared/bap.h b/src/shared/bap.h
> > > index 47a15636c..a4e8549a5 100644
> > > --- a/src/shared/bap.h
> > > +++ b/src/shared/bap.h
> > > @@ -34,6 +34,7 @@
> > > #define BT_BAP_CONFIG_PHY_CODEC 0x03
> > >
> > > struct bt_bap;
> > > +struct bt_bap_db;
> > > struct bt_bap_pac;
> > > struct bt_bap_stream;
> > >
> > > @@ -98,14 +99,14 @@ struct bt_bap_pac_qos {
> > > uint32_t ppd_max;
> > > };
> > >
> > > -struct bt_bap_pac *bt_bap_add_vendor_pac(struct gatt_db *db,
> > > +struct bt_bap_pac *bt_bap_add_vendor_pac(struct bt_bap_db *bdb,
> > > const char *name, uint8_t type,
> > > uint8_t id, uint16_t cid, uint16_t vid,
> > > struct bt_bap_pac_qos *qos,
> > > struct iovec *data,
> > > struct iovec *metadata);
> > >
> > > -struct bt_bap_pac *bt_bap_add_pac(struct gatt_db *db, const char *name,
> > > +struct bt_bap_pac *bt_bap_add_pac(struct bt_bap_db *bdb, const char *name,
> > > uint8_t type, uint8_t id,
> > > struct bt_bap_pac_qos *qos,
> > > struct iovec *data,
> > > @@ -135,7 +136,9 @@ unsigned int bt_bap_register(bt_bap_func_t added, bt_bap_func_t removed,
> > > void *user_data);
> > > bool bt_bap_unregister(unsigned int id);
> > >
> > > -struct bt_bap *bt_bap_new(struct gatt_db *ldb, struct gatt_db *rdb);
> > > +struct bt_bap_db *bt_bap_get_local_db(struct gatt_db *db, bool peripheral);
> > > +
> > > +struct bt_bap *bt_bap_new(struct bt_bap_db *ldb, struct gatt_db *rdb);
> > >
> > > bool bt_bap_set_user_data(struct bt_bap *bap, void *user_data);
> > >
> > > diff --git a/unit/test-bap.c b/unit/test-bap.c
> > > index afeefac84..b677f5197 100644
> > > --- a/unit/test-bap.c
> > > +++ b/unit/test-bap.c
> > > @@ -311,6 +311,7 @@ static void test_client(const void *user_data)
> > > {
> > > struct test_data *data = (void *)user_data;
> > > struct io *io;
> > > + struct bt_bap_db *ldb;
> > > struct gatt_db *db;
> > >
> > > io = tester_setup_io(data->iov, data->iovcnt);
> > > @@ -321,7 +322,8 @@ static void test_client(const void *user_data)
> > > db = gatt_db_new();
> > > g_assert(db);
> > >
> > > - data->bap = bt_bap_new(db, bt_gatt_client_get_db(data->client));
> > > + ldb = bt_bap_get_local_db(db, true);
> > > + data->bap = bt_bap_new(ldb, bt_gatt_client_get_db(data->client));
> > > g_assert(data->bap);
> > >
> > > bt_bap_set_debug(data->bap, print_debug, "bt_bap:", NULL);
> > > --
> > > 2.39.1
> > >
> >
> >
>
> --
> Pauli Virtanen
--
Luiz Augusto von Dentz
