Skip to content

LE Audio: Receiver start fix #51030

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 20, 2023
Merged

LE Audio: Receiver start fix #51030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions include/zephyr/bluetooth/audio/audio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1931,6 +1931,17 @@ int bt_audio_stream_disable(struct bt_audio_stream *stream);
* This procedure is used by a unicast client or unicast server to make a
* stream start streaming.
*
* For the unicast client, this will connect the CIS for the stream before
* sending the start command.
*
* For the unicast server, this will put a @ref BT_AUDIO_DIR_SINK stream into
* the streaming state if the CIS is connected (initialized by the unicast
* client). If the CIS is not connected yet, the stream will go into the
* streaming state as soon as the CIS is connected.
* @ref BT_AUDIO_DIR_SOURCE streams will go into the streaming state when the
* unicast client sends the Receiver Start Ready operation, which will trigger
* the @ref bt_audio_unicast_server_cb.start() callback.
*
* This shall only be called for unicast streams.
* Broadcast sinks will always be started once synchronized, and broadcast
* source streams shall be started with bt_audio_broadcast_source_start().
Expand Down
81 changes: 65 additions & 16 deletions samples/bluetooth/unicast_audio_server/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ static struct bt_codec lc3_codec =

static struct bt_conn *default_conn;
static struct k_work_delayable audio_send_work;
static struct bt_audio_stream streams[CONFIG_BT_ASCS_ASE_SNK_COUNT + CONFIG_BT_ASCS_ASE_SRC_COUNT];
static struct bt_audio_stream sink_streams[CONFIG_BT_ASCS_ASE_SNK_COUNT];
static struct bt_audio_source {
struct bt_audio_stream *stream;
struct bt_audio_stream stream;
uint16_t seq_num;
uint16_t max_sdu;
size_t len_to_send;
Expand Down Expand Up @@ -72,7 +72,7 @@ static const struct bt_data ad[] = {
static uint16_t get_and_incr_seq_num(const struct bt_audio_stream *stream)
{
for (size_t i = 0U; i < configured_source_stream_count; i++) {
if (stream == source_streams[i].stream) {
if (stream == &source_streams[i].stream) {
return source_streams[i].seq_num++;
}
}
Expand Down Expand Up @@ -190,7 +190,7 @@ static void audio_timer_timeout(struct k_work *work)
* data going to the server)
*/
for (size_t i = 0; i < configured_source_stream_count; i++) {
struct bt_audio_stream *stream = source_streams[i].stream;
struct bt_audio_stream *stream = &source_streams[i].stream;

buf = net_buf_alloc(&tx_pool, K_FOREVER);
net_buf_reserve(buf, BT_ISO_CHAN_SEND_RESERVE);
Expand All @@ -217,13 +217,41 @@ static void audio_timer_timeout(struct k_work *work)
k_work_schedule(&audio_send_work, K_MSEC(1000U));
}

static struct bt_audio_stream *stream_alloc(void)
static enum bt_audio_dir stream_dir(const struct bt_audio_stream *stream)
{
for (size_t i = 0; i < ARRAY_SIZE(streams); i++) {
struct bt_audio_stream *stream = &streams[i];
for (size_t i = 0U; i < ARRAY_SIZE(source_streams); i++) {
if (stream == &source_streams[i].stream) {
return BT_AUDIO_DIR_SOURCE;
}
}

if (!stream->conn) {
return stream;
for (size_t i = 0U; i < ARRAY_SIZE(sink_streams); i++) {
if (stream == &sink_streams[i]) {
return BT_AUDIO_DIR_SINK;
}
}

__ASSERT(false, "Invalid stream %p", stream);
return 0;
}

static struct bt_audio_stream *stream_alloc(enum bt_audio_dir dir)
{
if (dir == BT_AUDIO_DIR_SOURCE) {
for (size_t i = 0; i < ARRAY_SIZE(source_streams); i++) {
struct bt_audio_stream *stream = &source_streams[i].stream;

if (!stream->conn) {
return stream;
}
}
} else {
for (size_t i = 0; i < ARRAY_SIZE(sink_streams); i++) {
struct bt_audio_stream *stream = &sink_streams[i];

if (!stream->conn) {
return stream;
}
}
}

Expand All @@ -238,7 +266,7 @@ static int lc3_config(struct bt_conn *conn, const struct bt_audio_ep *ep, enum b

print_codec(codec);

*stream = stream_alloc();
*stream = stream_alloc(dir);
if (*stream == NULL) {
printk("No streams available\n");

Expand All @@ -248,7 +276,7 @@ static int lc3_config(struct bt_conn *conn, const struct bt_audio_ep *ep, enum b
printk("ASE Codec Config stream %p\n", *stream);

if (dir == BT_AUDIO_DIR_SOURCE) {
source_streams[configured_source_stream_count++].stream = *stream;
configured_source_stream_count++;
}

*pref = qos_pref;
Expand Down Expand Up @@ -284,7 +312,7 @@ static int lc3_qos(struct bt_audio_stream *stream, const struct bt_codec_qos *qo
print_qos(qos);

for (size_t i = 0U; i < configured_source_stream_count; i++) {
if (source_streams[i].stream == stream) {
if (stream == &source_streams[i].stream) {
source_streams[i].max_sdu = qos->sdu;
break;
}
Expand Down Expand Up @@ -335,8 +363,9 @@ static int lc3_start(struct bt_audio_stream *stream)
printk("Start: stream %p\n", stream);

for (size_t i = 0U; i < configured_source_stream_count; i++) {
if (source_streams[i].stream == stream) {
if (stream == &source_streams[i].stream) {
source_streams[i].seq_num = 0U;
source_streams[i].len_to_send = 0U;
break;
}
}
Expand Down Expand Up @@ -514,13 +543,29 @@ static void stream_stopped(struct bt_audio_stream *stream)
k_work_cancel_delayable(&audio_send_work);
}


static void stream_enabled_cb(struct bt_audio_stream *stream)
{
/* The unicast server is responsible for starting sink ASEs after the
* client has enabled them.
*/
if (stream_dir(stream) == BT_AUDIO_DIR_SINK) {
const int err = bt_audio_stream_start(stream);

if (err != 0) {
printk("Failed to start stream %p: %d", stream, err);
}
}
}
Comment on lines +552 to +559
Copy link
Contributor

@MariuszSkamra MariuszSkamra Feb 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why bt_audio_stream_start is called for SINK only? The API says:

 *  For the unicast server, this will put the stream into the streaming state if
 *  the CIS is connected (initialized by the unicast client). If the CIS is not
 *  connected yet, the stream will go into the streaming state as soon as the
 *  CIS is connected.

In other words I would expect bt_audio_stream_start will put the stream in streaming state, where both streams SINK and SOURCE need to be handled the same way from API point of view.

It's an ASCS specification detail that in case of SINK and SOURCE this method will be handled differently internally:
SINK:

  • Wait for CIS to be connected if not yet
  • Notify ASE state: Streaming
  • Call stream->started()

SOURCE:

  • Wait for CIS to be connected if not yet
  • Wait for ASE control operation: Receiver Start Ready
  • Notify ASE state: Streaming
  • Call stream->started()

Copy link
Contributor Author

@Thalley Thalley Feb 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have similar behavior for sink and source ASEs for bt_audio_stream_start, but my thought here would be that bt_audio_stream_start is a wrapper for the receiver start ready operation, which can only be done on sink ASEs on the server, and similarly only for source ASEs on the client (as per the ASCS spec).

I agree that the documentation of the function is incorrect in this regard, and should be updated.

Copy link
Contributor Author

@Thalley Thalley Feb 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the documentation a bit :)


static struct bt_audio_stream_ops stream_ops = {
#if defined(CONFIG_LIBLC3)
.recv = stream_recv_lc3_codec,
#else
.recv = stream_recv,
#endif
.stopped = stream_stopped,
.enabled = stream_enabled_cb,
};

static void connected(struct bt_conn *conn, uint8_t err)
Expand Down Expand Up @@ -674,8 +719,13 @@ void main(void)
bt_pacs_cap_register(BT_AUDIO_DIR_SINK, &cap_sink);
bt_pacs_cap_register(BT_AUDIO_DIR_SOURCE, &cap_source);

for (size_t i = 0; i < ARRAY_SIZE(streams); i++) {
bt_audio_stream_cb_register(&streams[i], &stream_ops);
for (size_t i = 0; i < ARRAY_SIZE(sink_streams); i++) {
bt_audio_stream_cb_register(&sink_streams[i], &stream_ops);
}

for (size_t i = 0; i < ARRAY_SIZE(source_streams); i++) {
bt_audio_stream_cb_register(&source_streams[i].stream,
&stream_ops);
}

err = set_location();
Expand Down Expand Up @@ -726,7 +776,6 @@ void main(void)
}

/* reset data */
(void)memset(source_streams, 0, sizeof(source_streams));
configured_source_stream_count = 0U;
k_work_cancel_delayable_sync(&audio_send_work, &sync);

Expand Down
69 changes: 31 additions & 38 deletions subsys/bluetooth/audio/ascs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,50 +567,28 @@ static void ascs_iso_sent(struct bt_iso_chan *chan)
}
}

static int ase_stream_start(struct bt_audio_stream *stream)
{
int err = 0;

if (unicast_server_cb != NULL && unicast_server_cb->start != NULL) {
err = unicast_server_cb->start(stream);
} else {
err = -ENOTSUP;
}

ascs_ep_set_state(stream->ep, BT_AUDIO_EP_STATE_STREAMING);

return err;
}

static void ascs_ep_iso_connected(struct bt_audio_ep *ep)
{
struct bt_audio_stream *stream;
int err;

if (ep->status.state != BT_AUDIO_EP_STATE_ENABLING) {
LOG_DBG("ep %p not in enabling state: %s",
ep, bt_audio_ep_state_str(ep->status.state));
return;
}

if (ep->dir == BT_AUDIO_DIR_SOURCE && !ep->receiver_ready) {
return;
} else if (ep->dir == BT_AUDIO_DIR_SINK) {
/* SINK ASEs can autonomously go into the streaming state if
* the CIS is connected
*/
ascs_ep_set_state(ep, BT_AUDIO_EP_STATE_STREAMING);
}

stream = ep->stream;
if (stream == NULL) {
LOG_ERR("No stream for ep %p", ep);
return;
}

err = ase_stream_start(stream);
if (err) {
LOG_ERR("Could not start stream %d", err);
if (ep->dir == BT_AUDIO_DIR_SINK && ep->receiver_ready) {
/* Source ASEs shall be ISO connected first, and then receive
* the receiver start ready command to enter the streaming
* state
*/
ascs_ep_set_state(ep, BT_AUDIO_EP_STATE_STREAMING);
}
}

Expand Down Expand Up @@ -1988,6 +1966,7 @@ static ssize_t ascs_enable(struct bt_ascs *ascs, struct net_buf_simple *buf)
static void ase_start(struct bt_ascs_ase *ase)
{
struct bt_audio_ep *ep;
int err;

LOG_DBG("ase %p", ase);

Expand All @@ -2012,22 +1991,36 @@ static void ase_start(struct bt_ascs_ase *ase)
ascs_cp_rsp_add(ASE_ID(ase), BT_ASCS_START_OP,
BT_ASCS_RSP_INVALID_DIR, BT_ASCS_REASON_NONE);
return;
} else if (ep->iso->chan.state != BT_ISO_STATE_CONNECTED) {
/* An ASE may not go into the streaming state unless the CIS
* is connected
*/
LOG_WRN("Start failed: CIS not connected: %u",
ep->iso->chan.state);
ascs_cp_rsp_add(ASE_ID(ase), BT_ASCS_START_OP,
BT_ASCS_RSP_INVALID_ASE_STATE,
BT_ASCS_REASON_NONE);
return;
}

ep->receiver_ready = true;
if (unicast_server_cb != NULL && unicast_server_cb->start != NULL) {
err = unicast_server_cb->start(ep->stream);
} else {
err = -ENOTSUP;
}

if (ep->iso->chan.state == BT_ISO_STATE_CONNECTED) {
int err;
if (err) {
LOG_ERR("Start failed: %d", err);
ascs_cp_rsp_add(ASE_ID(ase), BT_ASCS_START_OP, err,
BT_ASCS_REASON_NONE);

err = ase_stream_start(ep->stream);
if (err) {
LOG_ERR("Start failed: %d", err);
ascs_cp_rsp_add(ASE_ID(ase), BT_ASCS_START_OP, err,
BT_ASCS_REASON_NONE);
return;
}
return;
}

ep->receiver_ready = true;

ascs_ep_set_state(ep, BT_AUDIO_EP_STATE_STREAMING);

ascs_cp_rsp_success(ASE_ID(ase), BT_ASCS_START_OP);
}

Expand Down
2 changes: 1 addition & 1 deletion subsys/bluetooth/audio/endpoint.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ struct bt_audio_ep {
struct bt_codec_qos_pref qos_pref;
struct bt_audio_iso *iso;

/* FIXME: Replace with metastate */
/* Used by the unicast server */
bool receiver_ready;

/* TODO: Create a union to reduce memory usage */
Expand Down
23 changes: 14 additions & 9 deletions subsys/bluetooth/audio/unicast_server.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include <zephyr/bluetooth/audio/audio.h>

#include "audio_iso.h"
#include "pacs_internal.h"
#include "endpoint.h"

Expand Down Expand Up @@ -82,19 +83,23 @@ int bt_unicast_server_reconfig(struct bt_audio_stream *stream,

int bt_unicast_server_start(struct bt_audio_stream *stream)
{
int err;
struct bt_audio_ep *ep = stream->ep;

if (unicast_server_cb != NULL && unicast_server_cb->start != NULL) {
err = unicast_server_cb->start(stream);
} else {
err = -ENOTSUP;
}
if (ep->dir != BT_AUDIO_DIR_SINK) {
LOG_DBG("Invalid operation for stream %p with dir %u",
stream, ep->dir);

if (err != 0) {
return err;
return -EINVAL;
}

ascs_ep_set_state(stream->ep, BT_AUDIO_EP_STATE_STREAMING);
/* If ISO is connected to go streaming state,
* else wait for ISO to be connected
*/
if (ep->iso->chan.state == BT_ISO_STATE_CONNECTED) {
ascs_ep_set_state(ep, BT_AUDIO_EP_STATE_STREAMING);
} else {
ep->receiver_ready = true;
}

return 0;
}
Expand Down