OrgZephyr/zephyr
OrgZephyr
/
zephyr
mirror of https://github.com/zephyrproject-rtos/zephyr.git
1
0
Fork 0
Code Issues Releases Wiki Activity

Bluetooth: BAP: Broadcast: Fix state checks 

The existing state checks for both the broadcast sink
and broadcast source only ever checked the first BIS.
This sort of made sense, since they are all linked by HCI
(i.e. they share the same state), but there is a race condition
in the ISO and BAP callbacks that could allow applications
to delete sinks and sources before all the ISO callbacks
had been handled.

By treating the sink and source states as the highest value
of the BIS, then we better treat all BIS the same.

Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
This commit is contained in:
Emil Gydesen 2024-07-01 10:53:58 +02:00 committed by Alberto Escolar
parent 38ab4b38db
commit b413b505ee
2 changed files with 50 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
subsys/bluetooth/audio/bap_broadcast_sink.c
View File

@ -299,6 +299,33 @@ static void broadcast_sink_iso_recv(struct bt_iso_chan *chan,
}
}
/** Gets the "highest" state of all BIS in the broadcast sink */
static enum bt_bap_ep_state broadcast_sink_get_state(struct bt_bap_broadcast_sink *sink)
{
enum bt_bap_ep_state state = BT_BAP_EP_STATE_IDLE;
struct bt_bap_stream *stream;
if (sink == NULL) {
LOG_DBG("sink is NULL");
return state;
}
if (sys_slist_is_empty(&sink->streams)) {
LOG_DBG("Sink does not have any streams");
return state;
}
SYS_SLIST_FOR_EACH_CONTAINER(&sink->streams, stream, _node) {
if (stream->ep != NULL) {
state = MAX(state, stream->ep->status.state);
}
}
return state;
}
static void broadcast_sink_iso_connected(struct bt_iso_chan *chan)
{
struct bt_bap_iso *iso = CONTAINER_OF(chan, struct bt_bap_iso, chan);
@ -306,7 +333,6 @@ static void broadcast_sink_iso_connected(struct bt_iso_chan *chan)
struct bt_bap_broadcast_sink *sink;
struct bt_bap_stream *stream;
struct bt_bap_ep *ep = iso->rx.ep;
bool all_connected;
if (ep == NULL) {
LOG_ERR("iso %p not bound with ep", chan);
@ -340,17 +366,7 @@ static void broadcast_sink_iso_connected(struct bt_iso_chan *chan)
LOG_WRN("No callback for started set");
}
all_connected = true;
SYS_SLIST_FOR_EACH_CONTAINER(&sink->streams, stream, _node) {
__ASSERT(stream->ep, "Endpoint is NULL");
if (stream->ep->status.state != BT_BAP_EP_STATE_STREAMING) {
all_connected = false;
break;
}
}
if (all_connected) {
if (broadcast_sink_get_state(sink) != BT_BAP_EP_STATE_STREAMING) {
update_recv_state_big_synced(sink);
}
}
@ -1278,8 +1294,7 @@ int bt_bap_broadcast_sink_sync(struct bt_bap_broadcast_sink *sink, uint32_t inde
int bt_bap_broadcast_sink_stop(struct bt_bap_broadcast_sink *sink)
{
struct bt_bap_stream *stream;
sys_snode_t *head_node;
enum bt_bap_ep_state state;
int err;
CHECKIF(sink == NULL) {
@ -1292,21 +1307,9 @@ int bt_bap_broadcast_sink_stop(struct bt_bap_broadcast_sink *sink)
return -EALREADY;
}
head_node = sys_slist_peek_head(&sink->streams);
stream = CONTAINER_OF(head_node, struct bt_bap_stream, _node);
/* All streams in a broadcast source is in the same state,
* so we can just check the first stream
*/
if (stream->ep == NULL) {
LOG_DBG("stream->ep is NULL");
return -EINVAL;
}
if (stream->ep->status.state != BT_BAP_EP_STATE_STREAMING &&
stream->ep->status.state != BT_BAP_EP_STATE_QOS_CONFIGURED) {
LOG_DBG("Broadcast sink stream %p invalid state: %u", stream,
stream->ep->status.state);
state = broadcast_sink_get_state(sink);
if (state != BT_BAP_EP_STATE_STREAMING && state != BT_BAP_EP_STATE_QOS_CONFIGURED) {
LOG_DBG("Broadcast sink %p invalid state: %u", sink, state);
return -EBADMSG;
}
@ -1324,25 +1327,17 @@ int bt_bap_broadcast_sink_stop(struct bt_bap_broadcast_sink *sink)
int bt_bap_broadcast_sink_delete(struct bt_bap_broadcast_sink *sink)
{
enum bt_bap_ep_state state;
CHECKIF(sink == NULL) {
LOG_DBG("sink is NULL");
return -EINVAL;
}
if (!sys_slist_is_empty(&sink->streams)) {
struct bt_bap_stream *stream;
sys_snode_t *head_node;
head_node = sys_slist_peek_head(&sink->streams);
stream = CONTAINER_OF(head_node, struct bt_bap_stream, _node);
/* All streams in a broadcast source is in the same state,
* so we can just check the first stream
*/
if (stream->ep != NULL) {
LOG_DBG("Sink is not stopped");
return -EBADMSG;
}
state = broadcast_sink_get_state(sink);
if (state != BT_BAP_EP_STATE_IDLE) {
LOG_DBG("Broadcast sink %p invalid state: %u", sink, state);
return -EBADMSG;
}
/* Reset the broadcast sink */

29
subsys/bluetooth/audio/bap_broadcast_source.c
View File

@ -624,38 +624,33 @@ static bool valid_broadcast_source_param(const struct bt_bap_broadcast_source_pa
return true;
}
/** Gets the "highest" state of all BIS in the broadcast source */
static enum bt_bap_ep_state broadcast_source_get_state(struct bt_bap_broadcast_source *source)
{
enum bt_bap_ep_state state = BT_BAP_EP_STATE_IDLE;
struct bt_bap_broadcast_subgroup *subgroup;
struct bt_bap_stream *stream;
sys_snode_t *head_node;
if (source == NULL) {
LOG_DBG("source is NULL");
return BT_BAP_EP_STATE_IDLE;
return state;
}
if (sys_slist_is_empty(&source->subgroups)) {
LOG_DBG("Source does not have any streams");
return BT_BAP_EP_STATE_IDLE;
return state;
}
/* Get the first stream */
head_node = sys_slist_peek_head(&source->subgroups);
subgroup = CONTAINER_OF(head_node, struct bt_bap_broadcast_subgroup, _node);
SYS_SLIST_FOR_EACH_CONTAINER(&source->subgroups, subgroup, _node) {
struct bt_bap_stream *stream;
head_node = sys_slist_peek_head(&subgroup->streams);
stream = CONTAINER_OF(head_node, struct bt_bap_stream, _node);
/* All streams in a broadcast source is in the same state,
* so we can just check the first stream
*/
if (stream->ep == NULL) {
LOG_DBG("stream->ep is NULL");
return BT_BAP_EP_STATE_IDLE;
SYS_SLIST_FOR_EACH_CONTAINER(&subgroup->streams, stream, _node) {
if (stream->ep != NULL) {
state = MAX(state, stream->ep->status.state);
}
}
}
return stream->ep->status.state;
return state;
}
static bool merge_bis_and_subgroup_data_cb(struct bt_data *data, void *user_data)