Re: [PATCH BlueZ 7/8] shared/bap: handle central-only case

Hi Pauli,

On Mon, Jan 30, 2023 at 12:04 PM Pauli Virtanen [off-list ref] wrote:

Hi Luiz,

ma, 2023-01-30 kello 11:39 -0800, Luiz Augusto von Dentz kirjoitti:

quoted

On Mon, Jan 30, 2023 at 11:06 AM Pauli Virtanen [off-list ref] wrote:

quoted

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.

quoted

quoted

---
 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