From 92b12669d2b6f0571e27ec50029ce154b734f3f0 Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Tue, 18 Jun 2024 15:50:09 +0200 Subject: [PATCH] Bluetooth: BAP: Support setting different values per dir in CIG The interval and latency for a CIG are set for each direction now, allowing applications to use e.g. 10ms for sink ASEs and 7.5ms for source ASEs. Signed-off-by: Emil Gydesen --- subsys/bluetooth/audio/bap_endpoint.h | 14 +- subsys/bluetooth/audio/bap_unicast_client.c | 230 +++++++++++++----- .../audio/src/bap_unicast_client_test.c | 61 ++++- .../bap_unicast_client_async_group.sh | 23 ++ 4 files changed, 258 insertions(+), 70 deletions(-) create mode 100755 tests/bsim/bluetooth/audio/test_scripts/bap_unicast_client_async_group.sh diff --git a/subsys/bluetooth/audio/bap_endpoint.h b/subsys/bluetooth/audio/bap_endpoint.h index 4f52351e5fec537..610e12a4397c52f 100644 --- a/subsys/bluetooth/audio/bap_endpoint.h +++ b/subsys/bluetooth/audio/bap_endpoint.h @@ -64,11 +64,21 @@ struct bt_bap_ep { struct bt_bap_broadcast_sink *broadcast_sink; }; +struct bt_bap_unicast_group_cig_param { + uint32_t c_to_p_interval; + uint32_t p_to_c_interval; + uint16_t c_to_p_latency; + uint16_t p_to_c_latency; + uint8_t framing; +}; + struct bt_bap_unicast_group { + /* Group-wide QoS used to create the CIG */ + struct bt_bap_unicast_group_cig_param cig_param; + + /* Unicast group fields */ uint8_t index; bool allocated; - /* QoS used to create the CIG */ - const struct bt_audio_codec_qos *qos; struct bt_iso_cig *cig; /* The ISO API for CIG creation requires an array of pointers to ISO channels */ struct bt_iso_chan *cis[UNICAST_GROUP_STREAM_CNT]; diff --git a/subsys/bluetooth/audio/bap_unicast_client.c b/subsys/bluetooth/audio/bap_unicast_client.c index 236733c9c6bf2d8..db885f5df55d3de 100644 --- a/subsys/bluetooth/audio/bap_unicast_client.c +++ b/subsys/bluetooth/audio/bap_unicast_client.c @@ -2136,15 +2136,14 @@ static void unicast_client_ep_reset(struct bt_conn *conn, uint8_t reason) } static void bt_audio_codec_qos_to_cig_param(struct bt_iso_cig_param *cig_param, - const struct bt_audio_codec_qos *qos, + const struct bt_bap_unicast_group *group, const struct bt_bap_unicast_group_param *group_param) { - cig_param->framing = qos->framing; - cig_param->packing = BT_ISO_PACKING_SEQUENTIAL; /* TODO: Add to QoS struct */ - cig_param->c_to_p_interval = qos->interval; - cig_param->p_to_c_interval = qos->interval; - cig_param->c_to_p_latency = qos->latency; - cig_param->p_to_c_latency = qos->latency; + cig_param->framing = group->cig_param.framing; + cig_param->c_to_p_interval = group->cig_param.c_to_p_interval; + cig_param->p_to_c_interval = group->cig_param.p_to_c_interval; + cig_param->c_to_p_latency = group->cig_param.c_to_p_latency; + cig_param->p_to_c_latency = group->cig_param.p_to_c_latency; cig_param->sca = BT_GAP_SCA_UNKNOWN; if (group_param != NULL) { @@ -2155,21 +2154,26 @@ static void bt_audio_codec_qos_to_cig_param(struct bt_iso_cig_param *cig_param, cig_param->iso_interval = group_param->iso_interval; #endif /* CONFIG_BT_ISO_TEST_PARAMS */ } + + /* In the case that we only setup a single direction, we still need + * (as per section 7.8.97 LE Set CIG Parameters command) to set the interval both sides. + * If it is at this point unset, then we set the opposing direction to the same value. + */ + if (cig_param->c_to_p_interval == 0U) { + cig_param->c_to_p_interval = cig_param->p_to_c_interval; + } else if (cig_param->p_to_c_interval == 0U) { + cig_param->p_to_c_interval = cig_param->c_to_p_interval; + } } -/* FIXME: Remove `qos` parameter. Some of the QoS related CIG can be different - * between CIS'es. The implementation shall take the CIG parameters from - * unicast_group instead. - */ static int bt_audio_cig_create(struct bt_bap_unicast_group *group, - const struct bt_audio_codec_qos *qos, const struct bt_bap_unicast_group_param *group_param) { - struct bt_iso_cig_param param; + struct bt_iso_cig_param param = {0}; uint8_t cis_count; int err; - LOG_DBG("group %p qos %p", group, qos); + LOG_DBG("group %p", group); cis_count = 0U; for (size_t i = 0U; i < ARRAY_SIZE(group->cis); i++) { @@ -2183,7 +2187,7 @@ static int bt_audio_cig_create(struct bt_bap_unicast_group *group, param.num_cis = cis_count; param.cis_channels = group->cis; - bt_audio_codec_qos_to_cig_param(¶m, qos, group_param); + bt_audio_codec_qos_to_cig_param(¶m, group, group_param); err = bt_iso_cig_create(¶m, &group->cig); if (err != 0) { @@ -2191,19 +2195,16 @@ static int bt_audio_cig_create(struct bt_bap_unicast_group *group, return err; } - group->qos = qos; - return 0; } -static int bt_audio_cig_reconfigure(struct bt_bap_unicast_group *group, - const struct bt_audio_codec_qos *qos) +static int bt_audio_cig_reconfigure(struct bt_bap_unicast_group *group) { struct bt_iso_cig_param param; uint8_t cis_count; int err; - LOG_DBG("group %p qos %p", group, qos); + LOG_DBG("group %p ", group); cis_count = 0U; for (size_t i = 0U; i < ARRAY_SIZE(group->cis); i++) { @@ -2217,7 +2218,7 @@ static int bt_audio_cig_reconfigure(struct bt_bap_unicast_group *group, param.num_cis = cis_count; param.cis_channels = group->cis; - bt_audio_codec_qos_to_cig_param(¶m, qos, NULL); + bt_audio_codec_qos_to_cig_param(¶m, group, NULL); err = bt_iso_cig_reconfigure(group->cig, ¶m); if (err != 0) { @@ -2225,8 +2226,6 @@ static int bt_audio_cig_reconfigure(struct bt_bap_unicast_group *group, return err; } - group->qos = qos; - return 0; } @@ -2369,9 +2368,21 @@ static void unicast_group_add_stream(struct bt_bap_unicast_group *group, bt_bap_iso_bind_ep(iso, stream->ep); } - /* Store the Codec QoS in the bap_iso */ + /* Store the stream Codec QoS in the bap_iso */ unicast_client_codec_qos_to_iso_qos(iso, qos, dir); + /* Store the group Codec QoS in the group - This assume thats the parameters have been + * verified first + */ + group->cig_param.framing = qos->framing; + if (dir == BT_AUDIO_DIR_SOURCE) { + group->cig_param.p_to_c_interval = qos->interval; + group->cig_param.p_to_c_latency = qos->latency; + } else { + group->cig_param.c_to_p_interval = qos->interval; + group->cig_param.c_to_p_latency = qos->latency; + } + sys_slist_append(&group->streams, &stream->_node); } @@ -2558,25 +2569,135 @@ static int stream_pair_param_check(const struct bt_bap_unicast_group_stream_pair return 0; } -static int group_qos_common_set(const struct bt_audio_codec_qos **group_qos, - const struct bt_bap_unicast_group_stream_pair_param *param) +/** Validates that the stream parameter does not contain invalid values */ +static bool valid_unicast_group_stream_param(const struct bt_bap_unicast_group_stream_param *param, + struct bt_bap_unicast_group_cig_param *cig_param, + enum bt_audio_dir dir) { - if (param->rx_param != NULL && *group_qos == NULL) { - *group_qos = param->rx_param->qos; + const struct bt_audio_codec_qos *qos; + + CHECKIF(param->stream == NULL) { + LOG_DBG("param->stream is NULL"); + return -EINVAL; } - if (param->tx_param != NULL && *group_qos == NULL) { - *group_qos = param->tx_param->qos; + CHECKIF(param->qos == NULL) { + LOG_DBG("param->qos is NULL"); + return -EINVAL; } - return 0; + if (param->stream != NULL && param->stream->group != NULL) { + LOG_DBG("stream %p already part of group %p", param->stream, param->stream->group); + return -EALREADY; + } + + CHECKIF(bt_audio_verify_qos(param->qos) != BT_BAP_ASCS_REASON_NONE) { + LOG_DBG("Invalid QoS"); + return -EINVAL; + } + + qos = param->qos; + + /* If unset we set the interval else we verify that all streams use the same interval and + * latency in the same direction, as that is required when creating a CIG + */ + if (dir == BT_AUDIO_DIR_SINK) { + if (cig_param->c_to_p_interval == 0) { + cig_param->c_to_p_interval = qos->interval; + } else if (cig_param->c_to_p_interval != qos->interval) { + return false; + } + + if (cig_param->c_to_p_latency == 0) { + cig_param->c_to_p_latency = qos->latency; + } else if (cig_param->c_to_p_latency != qos->latency) { + return false; + } + } else { + if (cig_param->p_to_c_interval == 0) { + cig_param->p_to_c_interval = qos->interval; + } else if (cig_param->p_to_c_interval != qos->interval) { + return false; + } + + if (cig_param->p_to_c_latency == 0) { + cig_param->p_to_c_latency = qos->latency; + } else if (cig_param->p_to_c_latency != qos->latency) { + return false; + } + } + + if (cig_param->framing == 0) { + if (qos->framing == BT_AUDIO_CODEC_QOS_FRAMING_UNFRAMED) { + cig_param->framing = BT_ISO_FRAMING_UNFRAMED; + } else if (qos->framing == BT_AUDIO_CODEC_QOS_FRAMING_FRAMED) { + cig_param->framing = BT_ISO_FRAMING_FRAMED; + } + } else if ((qos->framing == BT_AUDIO_CODEC_QOS_FRAMING_UNFRAMED && + cig_param->framing != BT_ISO_FRAMING_UNFRAMED) || + (qos->framing == BT_AUDIO_CODEC_QOS_FRAMING_FRAMED && + cig_param->framing != BT_ISO_FRAMING_FRAMED)) { + return false; + } + + return true; +} + +static bool +valid_group_stream_pair_param(const struct bt_bap_unicast_group *unicast_group, + const struct bt_bap_unicast_group_stream_pair_param *pair_param) +{ + struct bt_bap_unicast_group_cig_param cig_param = {0}; + + CHECKIF(pair_param == NULL) { + LOG_DBG("pair_param is NULL"); + return false; + } + + if (pair_param->rx_param != NULL) { + if (!valid_unicast_group_stream_param(pair_param->rx_param, &cig_param, + BT_AUDIO_DIR_SOURCE)) { + return false; + } + } + + if (pair_param->tx_param != NULL) { + if (!valid_unicast_group_stream_param(pair_param->tx_param, &cig_param, + BT_AUDIO_DIR_SINK)) { + return false; + } + } + + return true; +} + +static bool valid_unicast_group_param(const struct bt_bap_unicast_group *unicast_group, + const struct bt_bap_unicast_group_param *param) +{ + CHECKIF(param == NULL) { + LOG_DBG("streams is NULL"); + return false; + } + + CHECKIF(param->params_count > UNICAST_GROUP_STREAM_CNT) { + LOG_DBG("Too many streams provided: %u/%u", param->params_count, + UNICAST_GROUP_STREAM_CNT); + return false; + } + + for (size_t i = 0U; i < param->params_count; i++) { + if (!valid_group_stream_pair_param(unicast_group, ¶m->params[i])) { + return false; + } + } + + return true; } int bt_bap_unicast_group_create(struct bt_bap_unicast_group_param *param, struct bt_bap_unicast_group **out_unicast_group) { struct bt_bap_unicast_group *unicast_group; - const struct bt_audio_codec_qos *group_qos = NULL; int err; CHECKIF(out_unicast_group == NULL) @@ -2587,40 +2708,23 @@ int bt_bap_unicast_group_create(struct bt_bap_unicast_group_param *param, /* Set out_unicast_group to NULL until the source has actually been created */ *out_unicast_group = NULL; - CHECKIF(param == NULL) - { - LOG_DBG("streams is NULL"); - return -EINVAL; - } - - CHECKIF(param->params_count > UNICAST_GROUP_STREAM_CNT) - { - LOG_DBG("Too many streams provided: %u/%u", param->params_count, - UNICAST_GROUP_STREAM_CNT); - return -EINVAL; - } - unicast_group = unicast_group_alloc(); if (unicast_group == NULL) { LOG_DBG("Could not allocate any more unicast groups"); return -ENOMEM; } + if (!valid_unicast_group_param(unicast_group, param)) { + unicast_group_free(unicast_group); + + return -EINVAL; + } + for (size_t i = 0U; i < param->params_count; i++) { struct bt_bap_unicast_group_stream_pair_param *stream_param; stream_param = ¶m->params[i]; - err = stream_pair_param_check(stream_param); - if (err < 0) { - return err; - } - - err = group_qos_common_set(&group_qos, stream_param); - if (err < 0) { - return err; - } - err = unicast_group_add_stream_pair(unicast_group, stream_param); if (err < 0) { LOG_DBG("unicast_group_add_stream failed: %d", err); @@ -2630,7 +2734,7 @@ int bt_bap_unicast_group_create(struct bt_bap_unicast_group_param *param, } } - err = bt_audio_cig_create(unicast_group, group_qos, param); + err = bt_audio_cig_create(unicast_group, param); if (err != 0) { LOG_DBG("bt_audio_cig_create failed: %d", err); unicast_group_free(unicast_group); @@ -2647,7 +2751,6 @@ int bt_bap_unicast_group_add_streams(struct bt_bap_unicast_group *unicast_group, struct bt_bap_unicast_group_stream_pair_param params[], size_t num_param) { - const struct bt_audio_codec_qos *group_qos = unicast_group->qos; struct bt_bap_stream *tmp_stream; size_t total_stream_cnt; struct bt_iso_cig *cig; @@ -2683,6 +2786,12 @@ int bt_bap_unicast_group_add_streams(struct bt_bap_unicast_group *unicast_group, return -EINVAL; } + for (size_t i = 0U; i < num_param; i++) { + if (!valid_group_stream_pair_param(unicast_group, ¶ms[i])) { + return -EINVAL; + } + } + /* We can just check the CIG state to see if any streams have started as * that would start the ISO connection procedure */ @@ -2702,11 +2811,6 @@ int bt_bap_unicast_group_add_streams(struct bt_bap_unicast_group *unicast_group, return err; } - err = group_qos_common_set(&group_qos, stream_param); - if (err < 0) { - return err; - } - err = unicast_group_add_stream_pair(unicast_group, stream_param); if (err < 0) { LOG_DBG("unicast_group_add_stream failed: %d", err); @@ -2714,7 +2818,7 @@ int bt_bap_unicast_group_add_streams(struct bt_bap_unicast_group *unicast_group, } } - err = bt_audio_cig_reconfigure(unicast_group, group_qos); + err = bt_audio_cig_reconfigure(unicast_group); if (err != 0) { LOG_DBG("bt_audio_cig_reconfigure failed: %d", err); goto fail; diff --git a/tests/bsim/bluetooth/audio/src/bap_unicast_client_test.c b/tests/bsim/bluetooth/audio/src/bap_unicast_client_test.c index d7b4b7a04ef1ead..a92fd610bb1a02e 100644 --- a/tests/bsim/bluetooth/audio/src/bap_unicast_client_test.c +++ b/tests/bsim/bluetooth/audio/src/bap_unicast_client_test.c @@ -52,7 +52,7 @@ static struct bt_bap_ep *g_sources[CONFIG_BT_BAP_UNICAST_CLIENT_ASE_SRC_COUNT]; static struct bt_bap_unicast_group_stream_pair_param pair_params[ARRAY_SIZE(test_streams)]; static struct bt_bap_unicast_group_stream_param stream_params[ARRAY_SIZE(test_streams)]; -/* Mandatory support preset by both client and server */ +/*Mandatory support preset by both client and server */ static struct bt_bap_lc3_preset preset_16_2_1 = BT_BAP_LC3_UNICAST_PRESET_16_2_1( BT_AUDIO_LOCATION_FRONT_LEFT, BT_AUDIO_CONTEXT_TYPE_UNSPECIFIED); @@ -627,7 +627,8 @@ static void discover_sources(void) WAIT_FOR_FLAG(flag_source_discovered); } -static int codec_configure_stream(struct bt_bap_stream *stream, struct bt_bap_ep *ep) +static int codec_configure_stream(struct bt_bap_stream *stream, struct bt_bap_ep *ep, + struct bt_audio_codec_cfg *codec_cfg) { int err; @@ -636,7 +637,7 @@ static int codec_configure_stream(struct bt_bap_stream *stream, struct bt_bap_ep do { - err = bt_bap_stream_config(default_conn, stream, ep, &preset_16_2_1.codec_cfg); + err = bt_bap_stream_config(default_conn, stream, ep, codec_cfg); if (err == -EBUSY) { k_sleep(BAP_STREAM_RETRY_WAIT); } else if (err != 0) { @@ -656,7 +657,8 @@ static void codec_configure_streams(size_t stream_cnt) for (size_t i = 0U; i < ARRAY_SIZE(pair_params); i++) { if (pair_params[i].rx_param != NULL && g_sources[i] != NULL) { struct bt_bap_stream *stream = pair_params[i].rx_param->stream; - const int err = codec_configure_stream(stream, g_sources[i]); + const int err = codec_configure_stream(stream, g_sources[i], + &preset_16_2_1.codec_cfg); if (err != 0) { FAIL("Unable to configure source stream[%zu]: %d", i, err); @@ -666,7 +668,8 @@ static void codec_configure_streams(size_t stream_cnt) if (pair_params[i].tx_param != NULL && g_sinks[i] != NULL) { struct bt_bap_stream *stream = pair_params[i].tx_param->stream; - const int err = codec_configure_stream(stream, g_sinks[i]); + const int err = codec_configure_stream(stream, g_sinks[i], + &preset_16_2_1.codec_cfg); if (err != 0) { FAIL("Unable to configure sink stream[%zu]: %d", i, err); @@ -1191,6 +1194,45 @@ static void test_main_acl_disconnect(void) PASS("Unicast client ACL disconnect passed\n"); } +static void test_main_async_group(void) +{ + struct bt_bap_stream rx_stream = {0}; + struct bt_bap_stream tx_stream = {0}; + struct bt_audio_codec_qos rx_qos = BT_AUDIO_CODEC_QOS_UNFRAMED(7500U, 30U, 2U, 75U, 40000U); + struct bt_audio_codec_qos tx_qos = + BT_AUDIO_CODEC_QOS_UNFRAMED(10000U, 40U, 2U, 100U, 40000U); + struct bt_bap_unicast_group_stream_param rx_param = { + .qos = &rx_qos, + .stream = &rx_stream, + }; + struct bt_bap_unicast_group_stream_param tx_param = { + .qos = &tx_qos, + .stream = &tx_stream, + }; + struct bt_bap_unicast_group_stream_pair_param pair_param = { + .rx_param = &rx_param, + .tx_param = &tx_param, + }; + struct bt_bap_unicast_group_param param = { + .params = &pair_param, + .params_count = 1U, + .packing = BT_ISO_PACKING_SEQUENTIAL, + }; + struct bt_bap_unicast_group *unicast_group; + int err; + + init(); + + err = bt_bap_unicast_group_create(¶m, &unicast_group); + if (err != 0) { + FAIL("Unable to create unicast group: %d", err); + + return; + } + + PASS("Unicast client async group parameters passed\n"); +} + static const struct bst_test_instance test_unicast_client[] = { { .test_id = "unicast_client", @@ -1204,6 +1246,15 @@ static const struct bst_test_instance test_unicast_client[] = { .test_tick_f = test_tick, .test_main_f = test_main_acl_disconnect, }, + { + .test_id = "unicast_client_async_group", + .test_pre_init_f = test_init, + .test_tick_f = test_tick, + .test_main_f = test_main_async_group, + .test_descr = "Tests that a unicast group (CIG) can be created with different " + "values in each direction, such as 10000us SDU interval in C to P " + "and 7500us for P to C", + }, BSTEST_END_MARKER, }; diff --git a/tests/bsim/bluetooth/audio/test_scripts/bap_unicast_client_async_group.sh b/tests/bsim/bluetooth/audio/test_scripts/bap_unicast_client_async_group.sh new file mode 100755 index 000000000000000..ae9ae830304e642 --- /dev/null +++ b/tests/bsim/bluetooth/audio/test_scripts/bap_unicast_client_async_group.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# +# Copyright (c) 2021-2024 Nordic Semiconductor ASA +# +# SPDX-License-Identifier: Apache-2.0 + +SIMULATION_ID="bap_unicast_client_async_group" +VERBOSITY_LEVEL=2 + +source ${ZEPHYR_BASE}/tests/bsim/sh_common.source + +cd ${BSIM_OUT_PATH}/bin + +printf "\n\n======== BAP Unicast Client Aync Group parameters test =========\n\n" + +Execute ./bs_${BOARD_TS}_tests_bsim_bluetooth_audio_prj_conf \ + -v=${VERBOSITY_LEVEL} -s=${SIMULATION_ID} -d=0 -testid=unicast_client_async_group -rs=23 -D=1 + +# Simulation time should be larger than the WAIT_TIME in common.h +Execute ./bs_2G4_phy_v1 -v=${VERBOSITY_LEVEL} -s=${SIMULATION_ID} \ + -D=1 -sim_length=60e6 $@ + +wait_for_background_jobs