From 332d3ae62a130bdf73db63662544119319c84a6f Mon Sep 17 00:00:00 2001 From: Lars Knudsen Date: Fri, 10 Jan 2025 12:54:52 +0100 Subject: [PATCH] Bluetooth: BAP: Remove ISO limitation on BASE parsing BASE is now parsed on-the-fly on sink sync, removing unnecessary restriction in intermediate parsed storage and without requiring an excessive amount of memory. Signed-off-by: Lars Knudsen --- subsys/bluetooth/audio/bap_broadcast_sink.c | 418 +++++++++----------- subsys/bluetooth/audio/bap_endpoint.h | 1 + 2 files changed, 195 insertions(+), 224 deletions(-) diff --git a/subsys/bluetooth/audio/bap_broadcast_sink.c b/subsys/bluetooth/audio/bap_broadcast_sink.c index 7e55c8c3aa235f..1a3b61dd7d96af 100644 --- a/subsys/bluetooth/audio/bap_broadcast_sink.c +++ b/subsys/bluetooth/audio/bap_broadcast_sink.c @@ -566,182 +566,6 @@ static bool codec_lookup_id(const struct bt_pacs_cap *cap, void *user_data) return true; } -struct store_base_info_data { - struct bt_bap_broadcast_sink_bis bis[CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT]; - struct bt_bap_broadcast_sink_subgroup subgroups[CONFIG_BT_BAP_BROADCAST_SNK_SUBGROUP_COUNT]; - struct bt_audio_codec_cfg *subgroup_codec_cfg; - uint32_t valid_indexes_bitfield; - uint8_t subgroup_count; - uint8_t bis_count; -}; - -static bool merge_bis_and_subgroup_data_cb(struct bt_data *data, void *user_data) -{ - struct bt_audio_codec_cfg *codec_cfg = user_data; - int err; - - err = bt_audio_codec_cfg_set_val(codec_cfg, data->type, data->data, data->data_len); - if (err < 0) { - LOG_DBG("Failed to set type %u with len %u in codec_cfg: %d", data->type, - data->data_len, err); - - return false; - } - - return true; -} - -static bool base_subgroup_bis_index_cb(const struct bt_bap_base_subgroup_bis *bis, void *user_data) -{ - struct bt_bap_broadcast_sink_subgroup *sink_subgroup; - struct store_base_info_data *data = user_data; - struct bt_bap_broadcast_sink_bis *sink_bis; - - if (data->bis_count == ARRAY_SIZE(data->bis)) { - /* We've parsed as many subgroups as we support */ - LOG_DBG("Could only store %u BIS", data->bis_count); - return false; - } - - sink_bis = &data->bis[data->bis_count]; - sink_subgroup = &data->subgroups[data->subgroup_count]; - - sink_bis->index = bis->index; - sink_subgroup->bis_indexes |= BT_ISO_BIS_INDEX_BIT(bis->index); - -#if CONFIG_BT_AUDIO_CODEC_CFG_MAX_DATA_SIZE > 0 - - memcpy(&sink_bis->codec_cfg, data->subgroup_codec_cfg, sizeof(sink_bis->codec_cfg)); - - if (bis->data_len > 0) { - /* Merge subgroup codec configuration with the BIS configuration - * As per the BAP spec, if a value exist at level 2 (subgroup) and 3 (BIS), then it - * is the value at level 3 that shall be used - */ - if (sink_bis->codec_cfg.id == BT_HCI_CODING_FORMAT_LC3) { - int err; - - memcpy(&sink_bis->codec_cfg, data->subgroup_codec_cfg, - sizeof(sink_bis->codec_cfg)); - - err = bt_audio_data_parse(bis->data, bis->data_len, - merge_bis_and_subgroup_data_cb, - &sink_bis->codec_cfg); - if (err != 0) { - LOG_DBG("Could not merge BIS and subgroup config in codec_cfg: %d", - err); - - return false; - } - } else { - /* If it is not LC3, then we don't know how to merge the subgroup and BIS - * codecs, so we just append them - */ - if (sink_bis->codec_cfg.data_len + bis->data_len > - sizeof(sink_bis->codec_cfg.data)) { - LOG_DBG("Could not store BIS and subgroup config in codec_cfg (%u " - "> %u)", - sink_bis->codec_cfg.data_len + bis->data_len, - sizeof(sink_bis->codec_cfg.data)); - - return false; - } - - memcpy(&sink_bis->codec_cfg.data[sink_bis->codec_cfg.data_len], bis->data, - bis->data_len); - sink_bis->codec_cfg.data_len += bis->data_len; - } - } -#endif /* CONFIG_BT_AUDIO_CODEC_CFG_MAX_DATA_SIZE > 0 */ - - data->bis_count++; - - return true; -} - -static bool base_subgroup_cb(const struct bt_bap_base_subgroup *subgroup, void *user_data) -{ - struct bt_bap_broadcast_sink_subgroup *sink_subgroup; - struct codec_cap_lookup_id_data lookup_data = {0}; - struct store_base_info_data *data = user_data; - struct bt_audio_codec_cfg codec_cfg; - int ret; - - if (data->subgroup_count == ARRAY_SIZE(data->subgroups)) { - /* We've parsed as many subgroups as we support */ - LOG_DBG("Could only store %u subgroups", data->subgroup_count); - return false; - } - - sink_subgroup = &data->subgroups[data->subgroup_count]; - - ret = bt_bap_base_subgroup_codec_to_codec_cfg(subgroup, &codec_cfg); - if (ret < 0) { - LOG_DBG("Could not store codec_cfg: %d", ret); - return false; - } - - /* Lookup and assign path_id based on capabilities */ - lookup_data.id = codec_cfg.id; - - bt_pacs_cap_foreach(BT_AUDIO_DIR_SINK, codec_lookup_id, &lookup_data); - if (lookup_data.codec_cap == NULL) { - LOG_DBG("Codec with id %u is not supported by our capabilities", lookup_data.id); - } else { - codec_cfg.path_id = lookup_data.codec_cap->path_id; - codec_cfg.ctlr_transcode = lookup_data.codec_cap->ctlr_transcode; - - data->subgroup_codec_cfg = &codec_cfg; - - ret = bt_bap_base_subgroup_foreach_bis(subgroup, base_subgroup_bis_index_cb, data); - if (ret < 0) { - LOG_DBG("Could not parse BISes: %d", ret); - return false; - } - - /* Add BIS to bitfield of valid BIS indexes we support */ - data->valid_indexes_bitfield |= sink_subgroup->bis_indexes; - data->subgroup_count++; - } - - return true; -} - -static int store_base_info(struct bt_bap_broadcast_sink *sink, const struct bt_bap_base *base) -{ - /* data is static due to its size, which easily can exceed the stack size */ - static struct store_base_info_data data; - uint32_t pres_delay; - int ret; - - ret = bt_bap_base_get_pres_delay(base); - if (ret < 0) { - LOG_DBG("Could not get presentation delay: %d", ret); - return ret; - } - - pres_delay = (uint32_t)ret; - - memset(&data, 0, sizeof(data)); - - ret = bt_bap_base_foreach_subgroup(base, base_subgroup_cb, &data); - if (ret != 0) { - LOG_DBG("Failed to parse all subgroups: %d", ret); - return ret; - } - - /* Ensure that we have not synced while parsing the BASE */ - if (sink->big == NULL) { - sink->qos_cfg.pd = pres_delay; - memcpy(sink->bis, data.bis, sizeof(sink->bis)); - memcpy(sink->subgroups, data.subgroups, sizeof(sink->subgroups)); - sink->subgroup_count = data.subgroup_count; - sink->valid_indexes_bitfield = data.valid_indexes_bitfield; - } - - return 0; -} - static bool base_subgroup_bis_count_cb(const struct bt_bap_base_subgroup *subgroup, void *user_data) { uint8_t *bis_cnt = user_data; @@ -798,23 +622,22 @@ static bool pa_decode_base(struct bt_data *data, void *user_data) } } + base_size = bt_bap_base_get_size(base); + /* Store newest BASE info until we are BIG synced */ if (sink->big == NULL) { LOG_DBG("Updating BASE for sink %p with %d subgroups\n", sink, bt_bap_base_get_subgroup_count(base)); - ret = store_base_info(sink, base); + ret = bt_bap_base_get_pres_delay(base); if (ret < 0) { - LOG_DBG("Could not store BASE information: %d", ret); - - /* If it returns -ECANCELED it means that we stopped parsing ourselves due - * to lack of memory. In this case we can still provide the BASE to the - * application else abort - */ - if (ret != -ECANCELED) { - return false; - } + LOG_DBG("Could not get presentation delay: %d", ret); + return ret; } + + sink->qos_cfg.pd = (uint32_t)ret; + + memcpy(sink->base, base, base_size); } if (atomic_test_bit(sink->flags, BT_BAP_BROADCAST_SINK_FLAG_SRC_ID_VALID)) { @@ -822,7 +645,6 @@ static bool pa_decode_base(struct bt_data *data, void *user_data) } /* We provide the BASE without the service data UUID */ - base_size = bt_bap_base_get_size(base); if (base_size < 0) { LOG_DBG("BASE get size failed (%d)", base_size); @@ -1146,23 +968,6 @@ static void broadcast_sink_cleanup(struct bt_bap_broadcast_sink *sink) (void)memset(sink, 0, sizeof(*sink)); /* also clears flags */ } -static struct bt_audio_codec_cfg *codec_cfg_from_base_by_index(struct bt_bap_broadcast_sink *sink, - uint8_t index) -{ - for (size_t i = 0U; i < ARRAY_SIZE(sink->bis); i++) { - struct bt_bap_broadcast_sink_bis *bis = &sink->bis[i]; - - if (bis->index == index) { - return &bis->codec_cfg; - } else if (bis->index == 0) { - /* index 0 is invalid, so we can use that as a terminator in the array */ - break; - } - } - - return NULL; -} - int bt_bap_broadcast_sink_create(struct bt_le_per_adv_sync *pa_sync, uint32_t broadcast_id, struct bt_bap_broadcast_sink **out_sink) { @@ -1220,15 +1025,171 @@ int bt_bap_broadcast_sink_create(struct bt_le_per_adv_sync *pa_sync, uint32_t br return 0; } +uint8_t bit_count(uint32_t bitfield) +{ +#ifdef POPCOUNT + return POPCOUNT(bitfield); +#else + uint8_t cnt = 0U; + + while (bitfield != 0U) { + cnt += bitfield & 1U; + bitfield >>= 1U; + } + + return cnt; +#endif +} + +struct sync_base_info_data { + struct bt_audio_codec_cfg codec_cfgs[CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT]; + struct bt_bap_broadcast_sink_subgroup subgroups[CONFIG_BT_BAP_BROADCAST_SNK_SUBGROUP_COUNT]; + struct bt_audio_codec_cfg *subgroup_codec_cfg; + uint32_t sync_indexes_bitfield; + uint8_t subgroup_count; + uint8_t stream_count; +}; + +static bool merge_bis_and_subgroup_data_cb(struct bt_data *data, void *user_data) +{ + struct bt_audio_codec_cfg *codec_cfg = user_data; + int err; + + err = bt_audio_codec_cfg_set_val(codec_cfg, data->type, data->data, data->data_len); + if (err < 0) { + LOG_DBG("Failed to set type %u with len %u in codec_cfg: %d", data->type, + data->data_len, err); + + return false; + } + + return true; +} + +static bool sync_base_subgroup_bis_index_cb(const struct bt_bap_base_subgroup_bis *bis, + void *user_data) +{ + struct bt_bap_broadcast_sink_subgroup *sink_subgroup; + struct sync_base_info_data *data = user_data; + struct bt_audio_codec_cfg *codec_cfg; + + sink_subgroup = &data->subgroups[data->subgroup_count]; + + sink_subgroup->bis_indexes |= BT_ISO_BIS_INDEX_BIT(bis->index); + + /* Only process selected BISes */ + if ((data->sync_indexes_bitfield & BT_ISO_BIS_INDEX_BIT(bis->index)) == 0) { + return true; + } + +#if CONFIG_BT_AUDIO_CODEC_CFG_MAX_DATA_SIZE > 0 + + codec_cfg = &data->codec_cfgs[data->stream_count]; + + memcpy(codec_cfg, data->subgroup_codec_cfg, sizeof(struct bt_audio_codec_cfg)); + + if (bis->data_len > 0) { + /* Merge subgroup codec configuration with the BIS configuration + * As per the BAP spec, if a value exist at level 2 (subgroup) and 3 (BIS), then it + * is the value at level 3 that shall be used + */ + if (codec_cfg->id == BT_HCI_CODING_FORMAT_LC3) { + int err; + + memcpy(codec_cfg, data->subgroup_codec_cfg, + sizeof(struct bt_audio_codec_cfg)); + + err = bt_audio_data_parse(bis->data, bis->data_len, + merge_bis_and_subgroup_data_cb, codec_cfg); + if (err != 0) { + LOG_DBG("Could not merge BIS and subgroup config in codec_cfg: %d", + err); + + return false; + } + } else { + /* If it is not LC3, then we don't know how to merge the subgroup and BIS + * codecs, so we just append them + */ + if (codec_cfg->data_len + bis->data_len > sizeof(codec_cfg->data)) { + LOG_DBG("Could not store BIS and subgroup config in codec_cfg (%u " + "> %u)", + codec_cfg->data_len + bis->data_len, + sizeof(codec_cfg->data)); + + return false; + } + + memcpy(&codec_cfg->data[codec_cfg->data_len], bis->data, bis->data_len); + codec_cfg->data_len += bis->data_len; + } + } +#endif /* CONFIG_BT_AUDIO_CODEC_CFG_MAX_DATA_SIZE > 0 */ + + data->stream_count++; + + return true; +} + +static bool sync_base_subgroup_cb(const struct bt_bap_base_subgroup *subgroup, void *user_data) +{ + struct bt_bap_broadcast_sink_subgroup *sink_subgroup; + struct codec_cap_lookup_id_data lookup_data = {0}; + struct sync_base_info_data *data = user_data; + struct bt_audio_codec_cfg codec_cfg; + int ret; + + if (data->subgroup_count == ARRAY_SIZE(data->subgroups)) { + /* We've parsed as many subgroups as we support */ + LOG_DBG("Could only store %u subgroups", data->subgroup_count); + return false; + } + + sink_subgroup = &data->subgroups[data->subgroup_count]; + + ret = bt_bap_base_subgroup_codec_to_codec_cfg(subgroup, &codec_cfg); + if (ret < 0) { + LOG_DBG("Could not store codec_cfg: %d", ret); + return false; + } + + /* Lookup and assign path_id based on capabilities */ + lookup_data.id = codec_cfg.id; + + bt_pacs_cap_foreach(BT_AUDIO_DIR_SINK, codec_lookup_id, &lookup_data); + if (lookup_data.codec_cap == NULL) { + LOG_DBG("Codec with id %u is not supported by our capabilities", lookup_data.id); + } else { + codec_cfg.path_id = lookup_data.codec_cap->path_id; + codec_cfg.ctlr_transcode = lookup_data.codec_cap->ctlr_transcode; + + data->subgroup_codec_cfg = &codec_cfg; + + ret = bt_bap_base_subgroup_foreach_bis(subgroup, sync_base_subgroup_bis_index_cb, + data); + if (ret < 0) { + LOG_DBG("Could not parse BISes: %d", ret); + return false; + } + + data->subgroup_count++; + } + + return true; +} + int bt_bap_broadcast_sink_sync(struct bt_bap_broadcast_sink *sink, uint32_t indexes_bitfield, struct bt_bap_stream *streams[], const uint8_t broadcast_code[BT_ISO_BROADCAST_CODE_SIZE]) { + static struct sync_base_info_data data; struct bt_iso_big_sync_param param; - struct bt_audio_codec_cfg *codec_cfgs[CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT] = {NULL}; struct bt_iso_chan *bis_channels[CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT]; + uint8_t bis_count; + uint32_t base_bis_indexes; uint8_t stream_count; int err; + int ret; CHECKIF(sink == NULL) { LOG_DBG("sink is NULL"); @@ -1268,32 +1229,41 @@ int bt_bap_broadcast_sink_sync(struct bt_bap_broadcast_sink *sink, uint32_t inde return -EINVAL; } - /* Validate that number of bits set is less than number of streams */ - if ((indexes_bitfield & sink->valid_indexes_bitfield) != indexes_bitfield) { - LOG_DBG("Request BIS indexes 0x%08X contains bits not support by the Broadcast " - "Sink 0x%08X", - indexes_bitfield, sink->valid_indexes_bitfield); + /* Validate that number of bits set is within supported range */ + bis_count = bit_count(indexes_bitfield); + if (bis_count > CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT) { + LOG_DBG("Cannot sync to more than %d streams (%u was requested)", + CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT, bis_count); return -EINVAL; } - stream_count = 0; - for (int i = 1; i < BT_ISO_MAX_GROUP_ISO_COUNT; i++) { - if ((indexes_bitfield & BT_ISO_BIS_INDEX_BIT(i)) != 0) { - struct bt_audio_codec_cfg *codec_cfg = - codec_cfg_from_base_by_index(sink, i); + /* Fetch total BIS index bitfield present in BASE */ + if (bt_bap_base_get_bis_indexes((const struct bt_bap_base *)sink->base, + &base_bis_indexes) != 0) { + LOG_DBG("Error parsing BASE"); + return -EINVAL; + } - __ASSERT(codec_cfg != NULL, "Index %d not found in sink", i); + /* Validate that the bits set are present in BASE */ + if ((indexes_bitfield & base_bis_indexes) != indexes_bitfield) { + LOG_DBG("Request BIS indexes (0x%08X) contains bits not present in BASE (0x%08X)", + indexes_bitfield, base_bis_indexes); + return -EINVAL; + } - codec_cfgs[stream_count++] = codec_cfg; + memset(&data, 0, sizeof(data)); - if (stream_count > CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT) { - LOG_DBG("Cannot sync to more than %d streams (%u was requested)", - CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT, stream_count); - return -EINVAL; - } - } + data.sync_indexes_bitfield = indexes_bitfield; + + ret = bt_bap_base_foreach_subgroup((const struct bt_bap_base *)sink->base, + sync_base_subgroup_cb, &data); + if (ret != 0) { + LOG_DBG("Failed to parse all subgroups: %d", ret); + return ret; } + stream_count = data.stream_count; + for (size_t i = 0; i < stream_count; i++) { CHECKIF(streams[i] == NULL) { LOG_DBG("streams[%zu] is NULL", i); @@ -1307,7 +1277,7 @@ int bt_bap_broadcast_sink_sync(struct bt_bap_broadcast_sink *sink, uint32_t inde struct bt_audio_codec_cfg *codec_cfg; stream = streams[i]; - codec_cfg = codec_cfgs[i]; + codec_cfg = &data.codec_cfgs[i]; err = bt_bap_broadcast_sink_setup_stream(sink, stream, codec_cfg); if (err != 0) { diff --git a/subsys/bluetooth/audio/bap_endpoint.h b/subsys/bluetooth/audio/bap_endpoint.h index 08d385aefacbac..3879335cbc361a 100644 --- a/subsys/bluetooth/audio/bap_endpoint.h +++ b/subsys/bluetooth/audio/bap_endpoint.h @@ -180,6 +180,7 @@ struct bt_bap_broadcast_sink { struct bt_bap_qos_cfg qos_cfg; struct bt_le_per_adv_sync *pa_sync; struct bt_iso_big *big; + uint8_t base[255]; struct bt_bap_broadcast_sink_bis bis[CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT]; struct bt_bap_broadcast_sink_subgroup subgroups[CONFIG_BT_BAP_BROADCAST_SNK_SUBGROUP_COUNT]; const struct bt_bap_scan_delegator_recv_state *recv_state;