Skip to content

Commit

Permalink
[nrf fromtree] Bluetooth: BAP: Fix issue with setting invalid iso dat…
Browse files Browse the repository at this point in the history
…a path

BAP would always set up the ISO data path in both directions,
even for unidirectional CIS. This meant that in the
unconfigured direction, the data path configuration data
would all be 0, which causes issues on some controllers.

The new refactored approach implemented by this commit
will always ensure that the data path is setup correctly,
and that we only set the data path in one direction for
unidirectional CIS. The unset path will use the default
ISO path of HCI and transparant format, which should always
be allowed by a controller.

This is building on the requirement in BAP that all streams in
a unicast group shall be QoS configured at the same time. This
ensures that before any streams in the CIG has been connected,
they have all been configured.

Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
(cherry picked from commit aef39f6)
Signed-off-by: Jui-Chou Chung <jui-chou.chung@nordicsemi.no>
  • Loading branch information
Thalley authored and cvinayak committed Jan 25, 2024
1 parent e793786 commit efade47
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 52 deletions.
6 changes: 1 addition & 5 deletions subsys/bluetooth/audio/ascs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1820,11 +1820,7 @@ static int ase_stream_qos(struct bt_bap_stream *stream, struct bt_audio_codec_qo
* we have the ISO <-> EP coupling completed (due to setting
* the CIS ID in the QoS procedure).
*/
if (ep->dir == BT_AUDIO_DIR_SINK) {
bt_audio_codec_cfg_to_iso_path(&ep->iso->rx.path, stream->codec_cfg);
} else {
bt_audio_codec_cfg_to_iso_path(&ep->iso->tx.path, stream->codec_cfg);
}
bt_bap_iso_configure_data_path(ep, stream->codec_cfg);

ep->cig_id = cig_id;
ep->cis_id = cis_id;
Expand Down
2 changes: 1 addition & 1 deletion subsys/bluetooth/audio/bap_broadcast_sink.c
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ static int bt_bap_broadcast_sink_setup_stream(struct bt_bap_broadcast_sink *sink
bt_bap_iso_bind_ep(iso, ep);

bt_audio_codec_qos_to_iso_qos(iso->chan.qos->rx, &sink->codec_qos);
bt_audio_codec_cfg_to_iso_path(iso->chan.qos->rx->path, codec_cfg);
bt_bap_iso_configure_data_path(ep, codec_cfg);

bt_bap_iso_unref(iso);

Expand Down
4 changes: 2 additions & 2 deletions subsys/bluetooth/audio/bap_broadcast_source.c
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ static int broadcast_source_setup_stream(uint8_t index, struct bt_bap_stream *st
bt_bap_iso_bind_ep(iso, ep);

bt_audio_codec_qos_to_iso_qos(iso->chan.qos->tx, qos);
bt_audio_codec_cfg_to_iso_path(iso->chan.qos->tx->path, codec_cfg);
bt_bap_iso_configure_data_path(ep, codec_cfg);
#if defined(CONFIG_BT_ISO_TEST_PARAMS)
iso->chan.qos->num_subevents = qos->num_subevents;
#endif /* CONFIG_BT_ISO_TEST_PARAMS */
Expand Down Expand Up @@ -878,7 +878,7 @@ int bt_bap_broadcast_source_reconfig(struct bt_bap_broadcast_source *source,

iso_qos = stream->ep->iso->chan.qos->tx;
bt_bap_stream_attach(NULL, stream, stream->ep, codec_cfg);
bt_audio_codec_cfg_to_iso_path(iso_qos->path, codec_cfg);
bt_bap_iso_configure_data_path(stream->ep, codec_cfg);
}
}

Expand Down
48 changes: 39 additions & 9 deletions subsys/bluetooth/audio/bap_iso.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,19 +129,12 @@ void bt_bap_iso_init(struct bt_bap_iso *iso, struct bt_iso_chan_ops *ops)
iso->chan.ops = ops;
iso->chan.qos = &iso->qos;

/* Setup points for both Tx and Rx
/* Setup the QoS for both Tx and Rx
* This is due to the limitation in the ISO API where pointers like
* the `qos->tx` shall be initialized before the CIS is connected if
* ever want to use it for TX, and ditto for RX. They cannot be
* initialized after the CIS has been connected
* the `qos->tx` shall be initialized before the CIS is created
*/
iso->chan.qos->rx = &iso->rx.qos;
iso->chan.qos->rx->path = &iso->rx.path;
iso->chan.qos->rx->path->cc = iso->rx.cc;

iso->chan.qos->tx = &iso->tx.qos;
iso->chan.qos->tx->path = &iso->tx.path;
iso->chan.qos->tx->path->cc = iso->tx.cc;
}

static struct bt_bap_iso_dir *bap_iso_get_iso_dir(bool unicast_client, struct bt_bap_iso *iso,
Expand All @@ -164,6 +157,43 @@ static struct bt_bap_iso_dir *bap_iso_get_iso_dir(bool unicast_client, struct bt
}
}

void bt_bap_iso_configure_data_path(struct bt_bap_ep *ep, struct bt_audio_codec_cfg *codec_cfg)
{
struct bt_bap_iso *bap_iso = ep->iso;
struct bt_iso_chan_qos *qos = bap_iso->chan.qos;
const bool is_unicast_client =
IS_ENABLED(CONFIG_BT_BAP_UNICAST_CLIENT) && bt_bap_ep_is_unicast_client(ep);
struct bt_bap_iso_dir *iso_dir = bap_iso_get_iso_dir(is_unicast_client, bap_iso, ep->dir);
struct bt_iso_chan_path *path = &iso_dir->path;

/* Setup the data path objects */
if (iso_dir == &bap_iso->rx) {
qos->rx->path = path;
} else {
qos->tx->path = path;
}

/* Configure the data path to either use the controller for transcoding, or set the path to
* be transparant to indicate that the transcoding happens somewhere else
*/
path->pid = codec_cfg->path_id;

if (codec_cfg->ctlr_transcode) {
path->format = codec_cfg->id;
path->cid = codec_cfg->cid;
path->vid = codec_cfg->vid;
path->delay = 0;
path->cc_len = codec_cfg->data_len;
path->cc = codec_cfg->data;
} else {
path->format = BT_HCI_CODING_FORMAT_TRANSPARENT;
path->cid = 0;
path->vid = 0;
path->delay = 0;
path->cc_len = 0;
path->cc = NULL;
}
}
static bool is_unicast_client_ep(struct bt_bap_ep *ep)
{
return IS_ENABLED(CONFIG_BT_BAP_UNICAST_CLIENT) && bt_bap_ep_is_unicast_client(ep);
Expand Down
1 change: 1 addition & 0 deletions subsys/bluetooth/audio/bap_iso.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ void bt_bap_iso_foreach(bt_bap_iso_func_t func, void *user_data);
struct bt_bap_iso *bt_bap_iso_find(bt_bap_iso_func_t func, void *user_data);
void bt_bap_iso_init(struct bt_bap_iso *iso, struct bt_iso_chan_ops *ops);
void bt_bap_iso_bind_ep(struct bt_bap_iso *iso, struct bt_bap_ep *ep);
void bt_bap_iso_configure_data_path(struct bt_bap_ep *ep, struct bt_audio_codec_cfg *codec_cfg);
void bt_bap_iso_unbind_ep(struct bt_bap_iso *iso, struct bt_bap_ep *ep);
struct bt_bap_ep *bt_bap_iso_get_ep(bool unicast_client, struct bt_bap_iso *iso,
enum bt_audio_dir dir);
Expand Down
22 changes: 0 additions & 22 deletions subsys/bluetooth/audio/bap_stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,28 +31,6 @@

LOG_MODULE_REGISTER(bt_bap_stream, CONFIG_BT_BAP_STREAM_LOG_LEVEL);

void bt_audio_codec_cfg_to_iso_path(struct bt_iso_chan_path *path,
struct bt_audio_codec_cfg *codec_cfg)
{
path->pid = codec_cfg->path_id;

if (codec_cfg->ctlr_transcode) {
path->format = codec_cfg->id;
path->cid = codec_cfg->cid;
path->vid = codec_cfg->vid;
path->delay = 0;
path->cc_len = codec_cfg->data_len;
path->cc = codec_cfg->data;
} else {
path->format = BT_HCI_CODING_FORMAT_TRANSPARENT;
path->cid = 0;
path->vid = 0;
path->delay = 0;
path->cc_len = 0;
path->cc = NULL;
}
}

#if defined(CONFIG_BT_BAP_UNICAST_CLIENT) || defined(CONFIG_BT_BAP_BROADCAST_SOURCE) || \
defined(CONFIG_BT_BAP_BROADCAST_SINK)
void bt_audio_codec_qos_to_iso_qos(struct bt_iso_chan_io_qos *io,
Expand Down
2 changes: 0 additions & 2 deletions subsys/bluetooth/audio/bap_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ void bt_bap_stream_reset(struct bt_bap_stream *stream);
void bt_bap_stream_attach(struct bt_conn *conn, struct bt_bap_stream *stream, struct bt_bap_ep *ep,
struct bt_audio_codec_cfg *codec_cfg);

void bt_audio_codec_cfg_to_iso_path(struct bt_iso_chan_path *path,
struct bt_audio_codec_cfg *codec_cfg);
void bt_audio_codec_qos_to_iso_qos(struct bt_iso_chan_io_qos *io,
const struct bt_audio_codec_qos *codec_qos);

Expand Down
13 changes: 2 additions & 11 deletions subsys/bluetooth/audio/bap_unicast_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -774,17 +774,8 @@ static void unicast_client_ep_qos_state(struct bt_bap_ep *ep, struct net_buf_sim
* we have the ISO <-> EP coupling completed (due to setting
* the CIS ID in the QoS procedure).
*/
if (ep->dir == BT_AUDIO_DIR_SOURCE) {
/* If the endpoint is a source, then we need to
* configure our RX parameters
*/
bt_audio_codec_cfg_to_iso_path(&ep->iso->rx.path, stream->codec_cfg);
} else {
/* If the endpoint is a sink, then we need to
* configure our TX parameters
*/
bt_audio_codec_cfg_to_iso_path(&ep->iso->tx.path, stream->codec_cfg);
}

bt_bap_iso_configure_data_path(ep, stream->codec_cfg);
}

/* Notify upper layer */
Expand Down

0 comments on commit efade47

Please sign in to comment.