| Message ID | 20230127205205.20235-1-pav@iki.fi |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,BlueZ] bap: check adapter support before enabling BAP | expand |
Hi Pauli, On Mon, Jan 30, 2023 at 10:58 AM Pauli Virtanen <pav@iki.fi> wrote: > > --- > src/adapter.c | 16 ++++++++++++++++ > src/adapter.h | 3 +++ > 2 files changed, 19 insertions(+) > > diff --git a/src/adapter.c b/src/adapter.c > index aadad4016..4f06bce53 100644 > --- a/src/adapter.c > +++ b/src/adapter.c > @@ -9033,6 +9033,22 @@ bool btd_adapter_ssp_enabled(struct btd_adapter *adapter) > return false; > } > > +bool btd_adapter_cis_central_capable(struct btd_adapter *adapter) > +{ > + if (adapter->current_settings & MGMT_SETTING_CIS_CENTRAL) > + return true; > + > + return false; > +} > + > +bool btd_adapter_cis_peripheral_capable(struct btd_adapter *adapter) > +{ > + if (adapter->current_settings & MGMT_SETTING_CIS_PERIPHERAL) > + return true; > + > + return false; > +} > + > void btd_adapter_set_oob_handler(struct btd_adapter *adapter, > struct oob_handler *handler) > { > diff --git a/src/adapter.h b/src/adapter.h > index 78eb069ae..3fcee30bc 100644 > --- a/src/adapter.h > +++ b/src/adapter.h > @@ -226,6 +226,9 @@ void btd_adapter_gatt_server_stop(struct btd_adapter *adapter); > > bool btd_adapter_ssp_enabled(struct btd_adapter *adapter); > > +bool btd_adapter_cis_central_capable(struct btd_adapter *adapter); > +bool btd_adapter_cis_peripheral_capable(struct btd_adapter *adapter); Lets just have btd_adapter_capable(struct btd_adapter *adapter, int flags) to make it simple to check multiple flags if needed. > int adapter_connect_list_add(struct btd_adapter *adapter, > struct btd_device *device); > void adapter_connect_list_remove(struct btd_adapter *adapter, > -- > 2.39.1 >
Hi Pauli, On Mon, Jan 30, 2023 at 10:48 AM Pauli Virtanen <pav@iki.fi> wrote: Let's have the btmon output in the description to make it easier to visualize how this changes affect it. > --- > monitor/packet.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/monitor/packet.c b/monitor/packet.c > index 44f1941bd..d9e8abf41 100644 > --- a/monitor/packet.c > +++ b/monitor/packet.c > @@ -12649,6 +12649,8 @@ static const struct bitfield_data mgmt_settings_table[] = { > { 15, "Static Address" }, > { 16, "PHY Configuration" }, > { 17, "Wideband Speech" }, > + { 18, "CIS Central" }, > + { 19, "CIS Peripheral" }, > { } > }; > > -- > 2.39.1 >
Hi Pauli, On Mon, Jan 30, 2023 at 12:04 PM Pauli Virtanen <pav@iki.fi> 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@iki.fi> 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
diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt index 90d612ed8..11798629a 100644 --- a/doc/mgmt-api.txt +++ b/doc/mgmt-api.txt @@ -333,6 +333,7 @@ Read Controller Information Command 16 PHY Configuration 17 Wideband Speech 18 Quality Report + 19 Connected Isochronous Stream Central This command generates a Command Complete event on success or a Command Status event on failure. @@ -2926,6 +2927,7 @@ Read Extended Controller Information Command 16 PHY Configuration 17 Wideband Speech 18 Quality Report + 19 Connected Isochronous Stream Central The EIR_Data field contains information about class of device, local name and other values. Not all of them might be present. For diff --git a/lib/mgmt.h b/lib/mgmt.h index 796190cd9..610770491 100644 --- a/lib/mgmt.h +++ b/lib/mgmt.h @@ -96,6 +96,7 @@ struct mgmt_rp_read_index_list { #define MGMT_SETTING_STATIC_ADDRESS 0x00008000 #define MGMT_SETTING_PHY_CONFIGURATION 0x00010000 #define MGMT_SETTING_WIDEBAND_SPEECH 0x00020000 +#define MGMT_SETTING_CIS_CENTRAL 0x00040000 #define MGMT_OP_READ_INFO 0x0004 struct mgmt_rp_read_info { diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c index e5ffb7230..2cd12465a 100644 --- a/profiles/audio/bap.c +++ b/profiles/audio/bap.c @@ -1264,6 +1264,11 @@ static int bap_probe(struct btd_service *service) return -ENOTSUP; } + if (!btd_adapter_has_feature(adapter, ADAPTER_FEAT_CIS_CENTRAL)) { + error("BAP requires CIS Central, but unsupported by adapter"); + return -ENOTSUP; + } + /* Ignore, if we were probed for this device already */ if (data) { error("Profile probed twice for the same device!"); diff --git a/profiles/audio/media.c b/profiles/audio/media.c index fbb350889..873dee33e 100644 --- a/profiles/audio/media.c +++ b/profiles/audio/media.c @@ -1259,6 +1259,9 @@ static bool experimental_endpoint_supported(struct btd_adapter *adapter) if (!btd_adapter_has_exp_feature(adapter, EXP_FEAT_ISO_SOCKET)) return false; + if (!btd_adapter_has_feature(adapter, ADAPTER_FEAT_CIS_CENTRAL)) + return false; + return g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL; } diff --git a/src/adapter.c b/src/adapter.c index aadad4016..2e038ace0 100644 --- a/src/adapter.c +++ b/src/adapter.c @@ -10717,6 +10717,17 @@ bool btd_has_kernel_features(uint32_t features) return (kernel_features & features) ? true : false; } +bool btd_adapter_has_feature(struct btd_adapter *adapter, uint32_t feature) +{ + size_t i; + uint32_t features = 0; + + if (adapter->supported_settings & MGMT_SETTING_CIS_CENTRAL) + features |= ADAPTER_FEAT_CIS_CENTRAL; + + return features & feature; +} + bool btd_adapter_has_exp_feature(struct btd_adapter *adapter, uint32_t feature) { size_t i; diff --git a/src/adapter.h b/src/adapter.h index 78eb069ae..6a9a626bc 100644 --- a/src/adapter.h +++ b/src/adapter.h @@ -256,6 +256,12 @@ void btd_adapter_for_each_device(struct btd_adapter *adapter, bool btd_le_connect_before_pairing(void); +enum adapter_features { + ADAPTER_FEAT_CIS_CENTRAL = 1 << 0, +}; + +bool btd_adapter_has_feature(struct btd_adapter *adapter, uint32_t feature); + enum experimental_features { EXP_FEAT_DEBUG = 1 << 0, EXP_FEAT_LE_SIMULT_ROLES = 1 << 1, diff --git a/tools/btmgmt.c b/tools/btmgmt.c index 29f86091f..de614ced1 100644 --- a/tools/btmgmt.c +++ b/tools/btmgmt.c @@ -353,6 +353,7 @@ static const char *settings_str[] = { "static-addr", "phy-configuration", "wide-band-speech", + "cis-central", }; static const char *settings2str(uint32_t settings) diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h index 743f6f59dff8..dc284c5f5cbb 100644 --- a/include/net/bluetooth/mgmt.h +++ b/include/net/bluetooth/mgmt.h @@ -109,6 +109,7 @@ struct mgmt_rp_read_index_list { #define MGMT_SETTING_STATIC_ADDRESS 0x00008000 #define MGMT_SETTING_PHY_CONFIGURATION 0x00010000 #define MGMT_SETTING_WIDEBAND_SPEECH 0x00020000 +#define MGMT_SETTING_CIS_CENTRAL 0x00040000 #define MGMT_OP_READ_INFO 0x0004 #define MGMT_READ_INFO_SIZE 0 diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 0dd30a3beb77..d802faf60f26 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -859,6 +859,9 @@ static u32 get_supported_settings(struct hci_dev *hdev) hdev->set_bdaddr) settings |= MGMT_SETTING_CONFIGURATION; + if (cis_central_capable(hdev)) + settings |= MGMT_SETTING_CIS_CENTRAL; + settings |= MGMT_SETTING_PHY_CONFIGURATION; return settings; @@ -932,6 +935,9 @@ static u32 get_current_settings(struct hci_dev *hdev) if (hci_dev_test_flag(hdev, HCI_WIDEBAND_SPEECH_ENABLED)) settings |= MGMT_SETTING_WIDEBAND_SPEECH; + if (cis_central_capable(hdev) && iso_enabled()) + settings |= MGMT_SETTING_CIS_CENTRAL; + return settings; }