Skip to contentSkip to conversation

BAP stream reconfiguration

RFC PATCH Superseded Pauli Virtanen 3 participants 18 messages Mar 1, 2025 – Mar 17, 2025 Bluetooth drivers

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Pauli Virtanen

author
#0 lore raw

cover BAP stream reconfiguration

Proposed DBus API extension is:

- org.bluez.MediaEndpoint.ClearConfiguration(transport_path):

Destroy stream associated with transport

- org.bluez.MediaEndpoint.ClearConfiguration(remote_endpoint_path):

Destroy all streams associated with endpoint

- org.bluez.MediaEndpoint.SelectProperties(remote_endpoint_path):

Destroy streams of endpoint, and re-run same unicast configuration as on initial connect, which invokes SelectProperties() callbacks on sound server and it can return new configuration it wants.

For future:

- org.bluez.MediaEndpoint.SetConfiguration(...):

I think how this already works for broadcast is good: one call creates one new stream. Unicast should be changed to work exactly the same way.

This allows for more detailed control of the configuration than SelectProperties(), and sound server can use it if it wants to deal with the different ASE configurations by itself.

- We will need to figure out how to handle devices rejecting ASE configurations on Config Codec or Enable.

Whose responsibility is it to try a different configuration, if the current configuration is rejected by device?

Example: Sony Linkbuds S has 48kHz sink & 48kHz source PACs. However, it does not support the duplx configuration with both 48kHz sink and 48kHz source ASE -- it rejects that in Enable. It does support 32 kHz sink + 32kHz source duplex configuration. AFAICS it is not possible to know which combinations of sinks & sources are possible, except trying them one by one. How do we handle this?

Unicast works with this Pipewire branch (can reconfigure to sink/source/duplex):
https://gitlab.freedesktop.org/pvir/pipewire/-/tree/bap-codec-switch-select

(With device sets, each device needs to be switched separately. Some more work is needed to make reconfiguration while CIG is active to work correctly, sound server must release transports before reconfiguring.)

Broadcast has not been tested at all.

Pauli Virtanen (9):
org.bluez.MediaEndpoint: removing BAP streams with ClearConfiguration org.bluez.MediaEndpoint: add client role SelectProperties
bap: add and use chainable future abstraction
bap: use futures for select operation
shared/bap: bap_abort_stream_req() should cancel also current req shared/bap: make sure ucast client stream is destroyed after releasing
bap: support removing streams with ClearConfiguration()
bap: do not try QoS before links are updated & io created
bap: implement client role SelectProperties()

 doc/org.bluez.MediaEndpoint.rst |  27 ++
 profiles/audio/bap.c            | 533 ++++++++++++++++++++++++++------
 profiles/audio/transport.c      |  17 +
 profiles/audio/transport.h      |   1 +
 src/shared/bap.c                |  39 ++-
 5 files changed, 522 insertions(+), 95 deletions(-)
Signature · 1 line
2.48.1
author
#1 lore raw

RFC PATCH 1/9 org.bluez.MediaEndpoint: removing BAP streams with ClearConfiguration

Allow user to remove specific streams by calling ClearConfiguration(transport_path) on the endpoint. If the path is the endpoint path instead, clear all streams associated with the endpoint.

---
 doc/org.bluez.MediaEndpoint.rst | 6 ++++++
 1 file changed, 6 insertions(+)
doc/org.bluez.MediaEndpoint.rst +6 -0
index f2b830ab0..b81106f0b 100644
@@ -109,6 +109,12 @@ void ClearConfiguration(object transport)
 
 	Clear transport configuration.
 
+	**Server role:** [ISO only]
+
+	Close the stream associated with the given transport. If the
+	path given is the path of this endpoint, all its streams are
+	closed.
+
 void Release()
 ``````````````
Signature · 1 line
2.48.1
author
#5 lore raw

RFC PATCH 2/9 org.bluez.MediaEndpoint: add client role SelectProperties

In reply to Pauli Virtanen

Add a simple way for the sound server to reconfigure a BAP unicast endpoint, by calling org.bluez.MediaEndpoint.SelectProperties().

This shall destroy all streams of the endpoint, and restart the SelectProperties() configuration flow from the beginning.

Since it may be necessary to reconfigure multiple endpoints at once to correctly make bidirectional CIS, add Defer argument to just mark eps for configuration.

In future, org.bluez.MediaEndpoint.SetConfiguration() could be changed to handle unicast in the same way as broadcast: only create streams. This allows sound server to have full control over stream configuration itself, and not rely on bt_bap_select().

---
 doc/org.bluez.MediaEndpoint.rst | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
doc/org.bluez.MediaEndpoint.rst +21 -0
index b81106f0b..5c42b878c 100644
@@ -69,6 +69,8 @@ array{byte} SelectConfiguration(array{byte} capabilities)
 dict SelectProperties(dict capabilities)
 ````````````````````````````````````````
 
+	**Server Role**
+
 	Select BAP unicast configuration from the supported capabilities:
 
 	:object Endpoint:

@@ -104,6 +106,25 @@ dict SelectProperties(dict capabilities)
 	Note: There is no need to cache the selected properties since on
 	success the configuration is send back as parameter of SetConfiguration.
 
+	**Client Role**
+
+	Reconfigure a BAP unicast endpoint. This closes all existing
+	streams of the endpoint, and restarts the configuration
+	selection flow which e.g. triggers calls to *SelectProperties*
+	allowing the sound server to modify the configuration.
+
+	The following arguments are taken in *capabilities*:
+
+	:boolean Defer [optional]:
+
+		If True, mark endpoint for reconfiguration, but
+		postpone it until a non-deferred *SelectProperties()*
+		operation is made on an endpoint of the same device.
+
+		This is necessary to use when reconfiguring source and
+		sink streams with the intention that they be combined
+		into the same CIG, possibly forming bidirectional CIS.
+
 void ClearConfiguration(object transport)
 `````````````````````````````````````````
Signature · 1 line
2.48.1
author
#7 lore raw

RFC PATCH 3/9 bap: add and use chainable future abstraction

In reply to Pauli Virtanen

Multi-part operations will need to postpone things like DBus replies until all parts are complete. Make this a bit simpler with a chainable future.

---
 profiles/audio/bap.c | 136 +++++++++++++++++++++++++++++++++----------
 1 file changed, 105 insertions(+), 31 deletions(-)
profiles/audio/bap.c +105 -31
index 37168e58c..8b9b89c70 100644
@@ -80,7 +80,7 @@ struct bap_setup {
 	struct iovec *metadata;
 	unsigned int id;
 	struct iovec *base;
-	DBusMessage *msg;
+	struct future *qos_done;
 	void (*destroy)(struct bap_setup *setup);
 };
 

@@ -114,6 +114,17 @@ struct bap_data {
 	void *user_data;
 };
 
+typedef void (*future_func_t)(int err, const char *err_msg, void *data);
+
+struct future {
+	unsigned int step, steps;
+	const char *name;
+	future_func_t func;
+	void *data;
+	int err;
+	const char *err_msg;
+};
+
 static struct queue *sessions;
 
 /* Structure holding the parameters for Periodic Advertisement create sync.

@@ -155,6 +166,94 @@ struct bt_iso_qos bap_sink_pa_qos = {
 	}
 };
 
+static void future_clear(struct future **p, int err, const char *err_msg)
+{
+	struct future *h = *p;
+
+	if (!h)
+		return;
+
+	DBG("future %p (%s) 0x%02x (%s) step %u/%u", h, h->name ? h->name : "",
+		err, (err && err_msg) ? err_msg : "", h->step + 1, h->steps);
+
+	*p = NULL;
+
+	if (err && !h->err) {
+		h->err = err;
+		h->err_msg = err_msg;
+	}
+
+	if (++h->step < h->steps)
+		return;
+
+	h->func(h->err, h->err_msg, h->data);
+	free(h);
+}
+
+static void future_dbus_callback_func(int err, const char *err_msg, void *data)
+{
+	DBusMessage *msg = data;
+	DBusMessage *reply;
+
+	if (err && !err_msg)
+		err_msg = strerror(err);
+
+	switch (err) {
+	case 0:
+		reply = dbus_message_new_method_return(msg);
+		break;
+	case EINVAL:
+		reply = btd_error_invalid_args_str(msg, err_msg);
+		break;
+	default:
+		reply = btd_error_failed(msg, err_msg);
+		break;
+	}
+
+	g_dbus_send_message(btd_get_dbus_connection(), reply);
+
+	dbus_message_unref(msg);
+}
+
+static void future_init(struct future **p, const char *name, future_func_t func,
+								void *data)
+{
+	struct future *h;
+
+	future_clear(p, ECANCELED, NULL);
+
+	h = new0(struct future, 1);
+	h->steps = 1;
+	h->name = name;
+	h->func = func;
+	h->data = data;
+
+	DBG("future %p (%s) init", h, h->name ? h->name : "");
+
+	*p = h;
+}
+
+static void future_init_dbus_reply(struct future **p, const char *name,
+							DBusMessage *msg)
+{
+	future_init(p, name, future_dbus_callback_func, dbus_message_ref(msg));
+}
+
+__attribute__((unused))
+static void future_init_chain(struct future **p, struct future *h)
+{
+	future_clear(p, ECANCELED, NULL);
+
+	if (h) {
+		h->steps++;
+
+		DBG("future %p (%s) init step %u", h, h->name ? h->name : "",
+								h->steps);
+	}
+
+	*p = h;
+}
+
 static bool bap_data_set_user_data(struct bap_data *data, void *user_data)
 {
 	if (!data)

@@ -740,24 +839,12 @@ static void qos_cb(struct bt_bap_stream *stream, uint8_t code, uint8_t reason,
 					void *user_data)
 {
 	struct bap_setup *setup = user_data;
-	DBusMessage *reply;
 
 	DBG("stream %p code 0x%02x reason 0x%02x", stream, code, reason);
 
 	setup->id = 0;
 
-	if (!setup->msg)
-		return;
-
-	if (!code)
-		reply = dbus_message_new_method_return(setup->msg);
-	else
-		reply = btd_error_failed(setup->msg, "Unable to configure");
-
-	g_dbus_send_message(btd_get_dbus_connection(), reply);
-
-	dbus_message_unref(setup->msg);
-	setup->msg = NULL;
+	future_clear(&setup->qos_done, code ? EIO : 0, "Unable to configure");
 }
 
 static void config_cb(struct bt_bap_stream *stream,

@@ -765,7 +852,6 @@ static void config_cb(struct bt_bap_stream *stream,
 					void *user_data)
 {
 	struct bap_setup *setup = user_data;
-	DBusMessage *reply;
 
 	DBG("stream %p code 0x%02x reason 0x%02x", stream, code, reason);
 

@@ -786,14 +872,7 @@ static void config_cb(struct bt_bap_stream *stream,
 		return;
 	}
 
-	if (!setup->msg)
-		return;
-
-	reply = btd_error_failed(setup->msg, "Unable to configure");
-	g_dbus_send_message(btd_get_dbus_connection(), reply);
-
-	dbus_message_unref(setup->msg);
-	setup->msg = NULL;
+	future_clear(&setup->qos_done, EIO, "Unable to configure");
 }
 
 static void setup_io_close(void *data, void *user_data)

@@ -872,7 +951,6 @@ static struct bap_setup *setup_new(struct bap_ep *ep)
 static void setup_free(void *data)
 {
 	struct bap_setup *setup = data;
-	DBusMessage *reply;
 
 	DBG("%p", setup);
 

@@ -881,12 +959,7 @@ static void setup_free(void *data)
 		setup->id = 0;
 	}
 
-	if (setup->msg) {
-		reply = btd_error_failed(setup->msg, "Canceled");
-		g_dbus_send_message(btd_get_dbus_connection(), reply);
-		dbus_message_unref(setup->msg);
-		setup->msg = NULL;
-	}
+	future_clear(&setup->qos_done, ECANCELED, NULL);
 
 	if (setup->ep)
 		queue_remove(setup->ep->setups, setup);

@@ -1038,7 +1111,8 @@ static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,
 
 	switch (bt_bap_stream_get_type(setup->stream)) {
 	case BT_BAP_STREAM_TYPE_UCAST:
-		setup->msg = dbus_message_ref(msg);
+		future_init_dbus_reply(&setup->qos_done, "set_configuration",
+									msg);
 		break;
 	case BT_BAP_STREAM_TYPE_BCAST:
 		/* No message sent over the air for broadcast */
Signature · 1 line
2.48.1
author
#9 lore raw

RFC PATCH 4/9 bap: use futures for select operation

In reply to Pauli Virtanen

Change select operation to use futures.

Remove spurious bap_config for bcast streams, it does not even run if there are only bcast PACs, and bcast streams are created differently.

---
 profiles/audio/bap.c | 74 ++++++++++++++++++++++++++++++++------------
 1 file changed, 55 insertions(+), 19 deletions(-)
profiles/audio/bap.c +55 -19
index 8b9b89c70..39484c74a 100644
@@ -93,6 +93,8 @@ struct bap_ep {
 	uint16_t supported_context;
 	uint16_t context;
 	struct queue *setups;
+	struct future *select_done;
+	int selecting;
 };
 
 struct bap_data {

@@ -110,7 +112,6 @@ struct bap_data {
 	struct queue *streams;
 	GIOChannel *listen_io;
 	unsigned int io_id;
-	int selecting;
 	void *user_data;
 };
 

@@ -166,6 +167,8 @@ struct bt_iso_qos bap_sink_pa_qos = {
 	}
 };
 
+static void pac_select_all(struct bap_data *data);
+
 static void future_clear(struct future **p, int err, const char *err_msg)
 {
 	struct future *h = *p;

@@ -239,7 +242,6 @@ static void future_init_dbus_reply(struct future **p, const char *name,
 	future_init(p, name, future_dbus_callback_func, dbus_message_ref(msg));
 }
 
-__attribute__((unused))
 static void future_init_chain(struct future **p, struct future *h)
 {
 	future_clear(p, ECANCELED, NULL);

@@ -1595,7 +1597,6 @@ static void select_cb(struct bt_bap_pac *pac, int err, struct iovec *caps,
 
 	if (err) {
 		error("err %d", err);
-		ep->data->selecting--;
 		goto done;
 	}
 

@@ -1604,16 +1605,12 @@ static void select_cb(struct bt_bap_pac *pac, int err, struct iovec *caps,
 	setup->metadata = util_iov_dup(metadata, 1);
 	setup->qos = *qos;
 
-	DBG("selecting %d", ep->data->selecting);
-	ep->data->selecting--;
-
 done:
-	if (ep->data->selecting)
-		return;
+	DBG("ep %p selecting %d", ep, ep->selecting);
 
-	queue_foreach(ep->data->srcs, bap_config, NULL);
-	queue_foreach(ep->data->snks, bap_config, NULL);
-	queue_foreach(ep->data->bcast, bap_config, NULL);
+	ep->selecting--;
+	if (!ep->selecting)
+		future_clear(&ep->select_done, 0, NULL);
 }
 
 static bool pac_register(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,

@@ -1631,11 +1628,17 @@ static bool pac_register(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
 	return true;
 }
 
+struct pac_select_data {
+	struct bap_data *data;
+	struct future *select_done;
+};
+
 static bool pac_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
 							void *user_data)
 {
-	struct btd_service *service = user_data;
-	struct bap_data *data = btd_service_get_user_data(service);
+	struct pac_select_data *select_data = user_data;
+	struct bap_data *data = select_data->data;
+	struct btd_service *service = data->service;
 	struct match_ep match = { lpac, rpac };
 	struct queue *queue;
 	struct bap_ep *ep;

@@ -1658,8 +1661,12 @@ static bool pac_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
 	}
 
 	/* TODO: Cache LRU? */
-	if (btd_service_is_initiator(service))
-		bt_bap_select(lpac, rpac, &ep->data->selecting, select_cb, ep);
+	if (btd_service_is_initiator(service)) {
+		bt_bap_select(lpac, rpac, &ep->selecting, select_cb, ep);
+		if (ep->selecting)
+			future_init_chain(&ep->select_done,
+						select_data->select_done);
+	}
 
 	return true;
 }

@@ -1678,8 +1685,12 @@ static void ep_cancel_select(struct bap_ep *ep)
 {
 	struct bt_bap *bap = ep->data->bap;
 
+	future_clear(&ep->select_done, ECANCELED, NULL);
+
 	bt_bap_foreach_pac(bap, BT_BAP_SOURCE, pac_cancel_select, ep);
 	bt_bap_foreach_pac(bap, BT_BAP_SINK, pac_cancel_select, ep);
+
+	ep->selecting = 0;
 }
 
 static bool pac_found_bcast(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,

@@ -1703,9 +1714,36 @@ static bool pac_found_bcast(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
 	return true;
 }
 
+static void select_complete_cb(int err, const char *err_msg, void *user_data)
+{
+	struct bap_data *data = user_data;
+
+	if (err == ECANCELED)
+		return;
+
+	queue_foreach(data->srcs, bap_config, NULL);
+	queue_foreach(data->snks, bap_config, NULL);
+}
+
+static void pac_select_all(struct bap_data *data)
+{
+	struct pac_select_data select_data = {
+		.data = data,
+	};
+
+	future_init(&select_data.select_done, "pac_select_all",
+						select_complete_cb, data);
+
+	bt_bap_foreach_pac(data->bap, BT_BAP_SOURCE, pac_select, &select_data);
+	bt_bap_foreach_pac(data->bap, BT_BAP_SINK, pac_select, &select_data);
+
+	future_clear(&select_data.select_done, 0, NULL);
+}
+
 static void bap_ready(struct bt_bap *bap, void *user_data)
 {
 	struct btd_service *service = user_data;
+	struct bap_data *data = btd_service_get_user_data(service);
 
 	DBG("bap %p", bap);
 

@@ -1715,8 +1753,7 @@ static void bap_ready(struct bt_bap *bap, void *user_data)
 	bt_bap_foreach_pac(bap, BT_BAP_SOURCE, pac_register, service);
 	bt_bap_foreach_pac(bap, BT_BAP_SINK, pac_register, service);
 
-	bt_bap_foreach_pac(bap, BT_BAP_SOURCE, pac_select, service);
-	bt_bap_foreach_pac(bap, BT_BAP_SINK, pac_select, service);
+	pac_select_all(data);
 }
 
 static bool match_setup_stream(const void *data, const void *user_data)

@@ -2684,8 +2721,7 @@ static void pac_added(struct bt_bap_pac *pac, void *user_data)
 	bt_bap_foreach_pac(data->bap, BT_BAP_SOURCE, pac_register, service);
 	bt_bap_foreach_pac(data->bap, BT_BAP_SINK, pac_register, service);
 
-	bt_bap_foreach_pac(data->bap, BT_BAP_SOURCE, pac_select, service);
-	bt_bap_foreach_pac(data->bap, BT_BAP_SINK, pac_select, service);
+	pac_select_all(data);
 }
 
 static void pac_added_broadcast(struct bt_bap_pac *pac, void *user_data)
Signature · 1 line
2.48.1
author
#10 lore raw

RFC PATCH 5/9 shared/bap: bap_abort_stream_req() should cancel also current req

In reply to Pauli Virtanen

After bap_abort_stream_req() no req callbacks for stream shall be called, so it has to fail also the currently in-flight request.

---
 src/shared/bap.c | 7 +++++++
 1 file changed, 7 insertions(+)
src/shared/bap.c +7 -0
index 208fc1bf2..54c6e8629 100644
@@ -1229,6 +1229,13 @@ static void bap_abort_stream_req(struct bt_bap *bap,
 						struct bt_bap_stream *stream)
 {
 	queue_remove_all(bap->reqs, match_req_stream, stream, bap_req_abort);
+
+	if (bap->req && bap->req->stream == stream) {
+		struct bt_bap_req *req = bap->req;
+
+		bap->req = NULL;
+		bap_req_complete(req, NULL);
+	}
 }
 
 static void bt_bap_stream_unref(struct bt_bap_stream *stream)
Signature · 1 line
2.48.1
author
#11 lore raw

RFC PATCH 6/9 shared/bap: make sure ucast client stream is destroyed after releasing

In reply to Pauli Virtanen

Upper layer as Unicast Client needs to be able to destroy streams when it wants to reconfigure endpoints.

This does not currently work right, because of Server issued Releasing->Config (caching) state transitions, which currently cause streams never enter Idle (so they are never destroyed).

Fix this by considering Releasing->Config as Releasing->Idle->Config. Also do not make new streams from cached config data as Unicast Client, and leave all stream configuration to upper layer.

---
 src/shared/bap.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)
src/shared/bap.c +30 -2
index 54c6e8629..4f44db07a 100644
@@ -1363,6 +1363,31 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream)
 	struct bt_bap *bap = stream->bap;
 	const struct queue_entry *entry;
 
+	switch (stream->ep->old_state) {
+	case BT_ASCS_ASE_STATE_RELEASING:
+		/* After Releasing, Server may either transition to Config or
+		 * Idle. Our Unicast Client streams shall be considered
+		 * destroyed after Releasing, so that upper layer can control
+		 * stream creation. Make the lifecycle management simpler by
+		 * making sure the streams are destroyed by always emitting Idle
+		 * to upper layer after Releasing, even if the remote ASE did
+		 * not go through that state.
+		 */
+		if (stream->client &&
+				stream->ep->state != BT_ASCS_ASE_STATE_IDLE &&
+				(stream->lpac->type & (BT_BAP_SINK |
+							BT_BAP_SOURCE))) {
+			struct bt_bap_endpoint *ep = stream->ep;
+			uint8_t state = ep->state;
+
+			ep->state = BT_ASCS_ASE_STATE_IDLE;
+			bap_stream_state_changed(stream);
+			ep->state = state;
+			return;
+		}
+		break;
+	}
+
 	/* Pre notification updates */
 	switch (stream->ep->state) {
 	case BT_ASCS_ASE_STATE_IDLE:

@@ -4851,7 +4876,8 @@ static void ep_status_config(struct bt_bap *bap, struct bt_bap_endpoint *ep,
 	}
 
 	/* Any previously applied codec configuration may be cached by the
-	 * server.
+	 * server. However, all Unicast Client stream creation shall be left to
+	 * the upper layer.
 	 */
 	if (!ep->stream) {
 		struct match_pac match;

@@ -4866,7 +4892,9 @@ static void ep_status_config(struct bt_bap *bap, struct bt_bap_endpoint *ep,
 		if (!match.lpac || !match.rpac)
 			return;
 
-		bap_stream_new(bap, ep, match.lpac, match.rpac, NULL, true);
+		if (!(match.lpac->type & (BT_BAP_SINK | BT_BAP_SOURCE)))
+			bap_stream_new(bap, ep, match.lpac, match.rpac,
+								NULL, true);
 	}
 
 	if (!ep->stream)
Signature · 1 line
2.48.1
author
#15 lore raw

RFC PATCH 7/9 bap: support removing streams with ClearConfiguration()

In reply to Pauli Virtanen

Implement removing streams via ClearConfiguration().

---
 profiles/audio/bap.c       | 87 ++++++++++++++++++++++++++++++++++++--
 profiles/audio/transport.c | 17 ++++++++
 profiles/audio/transport.h |  1 +
 3 files changed, 102 insertions(+), 3 deletions(-)
profiles/audio/bap.c +84 -3
index 39484c74a..46512a7f3 100644
@@ -58,6 +58,7 @@
 
 #include "bap.h"
 #include "bass.h"
+#include "transport.h"
 
 #define ISO_SOCKET_UUID "6fbaf188-05e0-496a-9885-d6ddfdb4e03e"
 #define PACS_UUID_STR "00001850-0000-1000-8000-00805f9b34fb"

@@ -81,6 +82,7 @@ struct bap_setup {
 	unsigned int id;
 	struct iovec *base;
 	struct future *qos_done;
+	struct future *close_done;
 	void (*destroy)(struct bap_setup *setup);
 };
 

@@ -903,12 +905,30 @@ static void setup_io_close(void *data, void *user_data)
 	bt_bap_stream_io_connecting(setup->stream, -1);
 }
 
-static void ep_close(struct bap_ep *ep)
+static void setup_close(void *data, void *user_data)
+{
+	struct bap_setup *setup = data;
+	struct future *close_done = user_data;
+	struct bt_bap_stream *stream = setup->stream;
+
+	DBG("%p", setup);
+
+	future_init_chain(&setup->close_done, close_done);
+
+	setup_io_close(setup, NULL);
+
+	if (bt_bap_stream_get_state(stream) != BT_BAP_STREAM_STATE_RELEASING)
+		bt_bap_stream_release(stream, NULL, NULL);
+	else
+		setup_free(setup);
+}
+
+static void ep_close(struct bap_ep *ep, struct future *close_done)
 {
 	if (!ep)
 		return;
 
-	queue_foreach(ep->setups, setup_io_close, NULL);
+	queue_foreach(ep->setups, setup_close, close_done);
 }
 
 static struct bap_setup *setup_new(struct bap_ep *ep)

@@ -962,6 +982,7 @@ static void setup_free(void *data)
 	}
 
 	future_clear(&setup->qos_done, ECANCELED, NULL);
+	future_clear(&setup->close_done, 0, NULL);
 
 	if (setup->ep)
 		queue_remove(setup->ep->setups, setup);

@@ -1077,7 +1098,7 @@ static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,
 	 * TO DO reconfiguration of a BIS.
 	 */
 	if (bt_bap_pac_get_type(ep->lpac) != BT_BAP_BCAST_SOURCE)
-		ep_close(ep);
+		ep_close(ep, NULL);
 
 	setup = setup_new(ep);
 

@@ -1129,6 +1150,63 @@ static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,
 	return NULL;
 }
 
+struct close_stream_data {
+	const char *path;
+	struct future *close_done;
+	unsigned int count;
+};
+
+static void close_stream(void *data, void *user_data)
+{
+	struct bap_setup *setup = data;
+	struct close_stream_data *info = user_data;
+	struct bt_bap_stream *stream = setup->stream;
+	const char *path = media_transport_stream_path(stream);
+
+	if (info->path && (!path || strcmp(info->path, path)))
+		return;
+
+	setup_close(setup, info->close_done);
+	info->count++;
+}
+
+static unsigned int ep_close_stream(struct bap_ep *ep,
+						struct future *close_done,
+						const char *transport_path)
+{
+	struct close_stream_data info = { transport_path, close_done, 0 };
+
+	queue_foreach(ep->setups, close_stream, &info);
+	return info.count;
+}
+
+
+static DBusMessage *clear_configuration(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	struct bap_ep *ep = data;
+	const char *path;
+	DBusMessageIter args;
+	struct future *done = NULL;
+
+	dbus_message_iter_init(msg, &args);
+	dbus_message_iter_get_basic(&args, &path);
+
+	DBG("%s: %s", ep->path, path ? path : "NULL");
+
+	future_init_dbus_reply(&done, "clear_configuration", msg);
+
+	if (strcmp(path, ep->path) == 0)
+		path = NULL;
+
+	if (ep_close_stream(ep, done, path))
+		future_clear(&done, 0, NULL);
+	else
+		future_clear(&done, path ? EINVAL : 0, NULL);
+
+	return NULL;
+}
+
 static bool stream_io_unset(const void *data, const void *user_data)
 {
 	struct bt_bap_stream *stream = (struct bt_bap_stream *)data;

@@ -1350,6 +1428,9 @@ static const GDBusMethodTable ep_methods[] = {
 					GDBUS_ARGS({ "endpoint", "o" },
 						{ "Configuration", "a{sv}" } ),
 					NULL, set_configuration) },
+	{ GDBUS_EXPERIMENTAL_ASYNC_METHOD("ClearConfiguration",
+					GDBUS_ARGS({ "transport", "o" }),
+					NULL, clear_configuration) },
 	{ },
 };
profiles/audio/transport.c +17 -0
index f3ac1a251..e81ef6e9a 100644
@@ -2671,3 +2671,20 @@ void media_transport_update_device_volume(struct btd_device *dev,
 
 	btd_device_set_volume(dev, volume);
 }
+
+const char *media_transport_stream_path(void *stream)
+{
+	GSList *l;
+
+	if (!stream)
+		return NULL;
+
+	for (l = transports; l; l = l->next) {
+		struct media_transport *transport = l->data;
+
+		if (media_transport_get_stream(transport) == stream)
+			return transport->path;
+	}
+
+	return NULL;
+}
profiles/audio/transport.h +1 -0
index 808e1a193..7c107281a 100644
@@ -33,3 +33,4 @@ void transport_get_properties(struct media_transport *transport,
 int media_transport_get_device_volume(struct btd_device *dev);
 void media_transport_update_device_volume(struct btd_device *dev,
 								int volume);
+const char *media_transport_stream_path(void *stream);
Signature · 1 line
2.48.1
author
#16 lore raw

RFC PATCH 8/9 bap: do not try QoS before links are updated & io created

In reply to Pauli Virtanen

It's not possible to QoS in config_cb, even if stream is in Config state, because stream->io may not exist and stream links are not yet updated. The stream links are updated only before bap_state(), so the completion of config has to be handled only there.

Wait for both events to complete before proceeding to QoS.

---
 profiles/audio/bap.c | 80 +++++++++++++++++++++++---------------------
 1 file changed, 42 insertions(+), 38 deletions(-)
profiles/audio/bap.c +42 -38
index 46512a7f3..da1f9feb1 100644
@@ -83,6 +83,7 @@ struct bap_setup {
 	struct iovec *base;
 	struct future *qos_done;
 	struct future *close_done;
+	bool config_done;
 	void (*destroy)(struct bap_setup *setup);
 };
 

@@ -851,32 +852,49 @@ static void qos_cb(struct bt_bap_stream *stream, uint8_t code, uint8_t reason,
 	future_clear(&setup->qos_done, code ? EIO : 0, "Unable to configure");
 }
 
+static void setup_create_io(struct bap_data *data, struct bap_setup *setup,
+				struct bt_bap_stream *stream, int defer);
+
+static void setup_qos(struct bap_setup *setup)
+{
+	struct bap_data *data = setup->ep->data;
+	struct bt_bap_stream *stream = setup->stream;
+
+	if (setup->id || !stream)
+		return;
+
+	setup_create_io(data, setup, stream, true);
+	if (!setup->io) {
+		error("Unable to create io");
+		if (bt_bap_stream_get_state(stream) !=
+						BT_BAP_STREAM_STATE_RELEASING)
+			bt_bap_stream_release(stream, NULL, NULL);
+		return;
+	}
+
+	/* Wait QoS response to respond */
+	setup->id = bt_bap_stream_qos(stream, &setup->qos, qos_cb, setup);
+	if (!setup->id) {
+		error("Failed to Configure QoS");
+		bt_bap_stream_release(stream, NULL, NULL);
+	}
+}
+
 static void config_cb(struct bt_bap_stream *stream,
 					uint8_t code, uint8_t reason,
 					void *user_data)
 {
 	struct bap_setup *setup = user_data;
+	const char *err_msg = "Unable to configure";
 
 	DBG("stream %p code 0x%02x reason 0x%02x", stream, code, reason);
 
 	setup->id = 0;
 
-	if (!code) {
-		/* Check state is already set to config then proceed to qos */
-		if (bt_bap_stream_get_state(stream) ==
-					BT_BAP_STREAM_STATE_CONFIG) {
-			setup->id = bt_bap_stream_qos(stream, &setup->qos,
-							qos_cb, setup);
-			if (!setup->id) {
-				error("Failed to Configure QoS");
-				bt_bap_stream_release(stream, NULL, NULL);
-			}
-		}
-
-		return;
-	}
-
-	future_clear(&setup->qos_done, EIO, "Unable to configure");
+	if (code)
+		future_clear(&setup->qos_done, EIO, err_msg);
+	else if (setup->config_done)
+		setup_qos(setup);
 }
 
 static void setup_io_close(void *data, void *user_data)

@@ -1128,6 +1146,8 @@ static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,
 		return btd_error_invalid_args(msg);
 	}
 
+	setup->config_done = false;
+
 	if (setup->metadata && setup->metadata->iov_len)
 		bt_bap_stream_metadata(setup->stream, setup->metadata, NULL,
 								NULL);

@@ -1655,6 +1675,8 @@ static void setup_config(void *data, void *user_data)
 		return;
 	}
 
+	setup->config_done = false;
+
 	if (setup->metadata && setup->metadata->iov_len)
 		bt_bap_stream_metadata(setup->stream, setup->metadata, NULL,
 								NULL);

@@ -2086,9 +2108,6 @@ static bool is_cig_busy(struct bap_data *data, uint8_t cig)
 	return queue_find(sessions, cig_busy_session, &info);
 }
 
-static void setup_create_io(struct bap_data *data, struct bap_setup *setup,
-				struct bt_bap_stream *stream, int defer);
-
 static gboolean setup_io_recreate(void *user_data)
 {
 	struct bap_setup *setup = user_data;

@@ -2473,25 +2492,10 @@ static void bap_state(struct bt_bap_stream *stream, uint8_t old_state,
 			queue_remove(data->streams, stream);
 		break;
 	case BT_BAP_STREAM_STATE_CONFIG:
-		if (setup && !setup->id) {
-			setup_create_io(data, setup, stream, true);
-			if (!setup->io) {
-				error("Unable to create io");
-				if (old_state != BT_BAP_STREAM_STATE_RELEASING)
-					bt_bap_stream_release(stream, NULL,
-								NULL);
-				return;
-			}
-
-			/* Wait QoS response to respond */
-			setup->id = bt_bap_stream_qos(stream,
-							&setup->qos,
-							qos_cb,	setup);
-			if (!setup->id) {
-				error("Failed to Configure QoS");
-				bt_bap_stream_release(stream,
-							NULL, NULL);
-			}
+		if (setup) {
+			setup->config_done = true;
+			if (!setup->id)
+				setup_qos(setup);
 		}
 		break;
 	case BT_BAP_STREAM_STATE_QOS:
Signature · 1 line
2.48.1
author
#17 lore raw

RFC PATCH 9/9 bap: implement client role SelectProperties()

In reply to Pauli Virtanen

Add SelectProperties() on a BAP unicast endpoint, which triggers its reconfiguration or marks it for reconfiguration.

First, all associated streams are closed. After that, endpoints marked for reconfiguration are reconfigured using the same flow as in the initial configuration.

---
 profiles/audio/bap.c | 180 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 166 insertions(+), 14 deletions(-)
profiles/audio/bap.c +166 -14
index da1f9feb1..0590067eb 100644
@@ -96,8 +96,10 @@ struct bap_ep {
 	uint16_t supported_context;
 	uint16_t context;
 	struct queue *setups;
+	struct future *setup_done;
 	struct future *select_done;
 	int selecting;
+	bool reconfigure;
 };
 
 struct bap_data {

@@ -170,7 +172,8 @@ struct bt_iso_qos bap_sink_pa_qos = {
 	}
 };
 
-static void pac_select_all(struct bap_data *data);
+static void pac_select_all(struct bap_data *data, bool reconfigure,
+							struct future *done);
 
 static void future_clear(struct future **p, int err, const char *err_msg)
 {

@@ -849,7 +852,8 @@ static void qos_cb(struct bt_bap_stream *stream, uint8_t code, uint8_t reason,
 
 	setup->id = 0;
 
-	future_clear(&setup->qos_done, code ? EIO : 0, "Unable to configure");
+	if (code)
+		future_clear(&setup->qos_done, EIO, "Unable to configure");
 }
 
 static void setup_create_io(struct bap_data *data, struct bap_setup *setup,

@@ -1227,6 +1231,120 @@ static DBusMessage *clear_configuration(DBusConnection *conn, DBusMessage *msg,
 	return NULL;
 }
 
+static void ep_close_if_reconfigure(void *obj, void *user_data)
+{
+	struct bap_ep *ep = obj;
+	struct future *done = user_data;
+
+	if (ep->reconfigure)
+		ep_close(ep, done);
+}
+
+static int select_properties_parse(DBusMessageIter *props, bool *defer)
+{
+	const char *key;
+
+	if (dbus_message_iter_get_arg_type(props) != DBUS_TYPE_DICT_ENTRY)
+		return -EINVAL;
+
+	while (dbus_message_iter_get_arg_type(props) == DBUS_TYPE_DICT_ENTRY) {
+		DBusMessageIter value, entry;
+		int var;
+
+		dbus_message_iter_recurse(props, &entry);
+		dbus_message_iter_get_basic(&entry, &key);
+
+		dbus_message_iter_next(&entry);
+		dbus_message_iter_recurse(&entry, &value);
+
+		var = dbus_message_iter_get_arg_type(&value);
+
+		if (!strcasecmp(key, "Defer")) {
+			dbus_bool_t flag;
+
+			if (var != DBUS_TYPE_BOOLEAN)
+				goto fail;
+
+			dbus_message_iter_get_basic(&value, &flag);
+			*defer = flag;
+		}
+
+		dbus_message_iter_next(props);
+	}
+
+	return 0;
+
+fail:
+	DBG("Failed parsing %s", key);
+
+	return -EINVAL;
+}
+
+struct select_properties_data {
+	struct bap_data *data;
+	DBusMessage *msg;
+};
+
+static void select_properties_reconfig(int err, const char *err_msg, void *data)
+{
+	struct select_properties_data *info = data;
+	struct future *done = NULL;
+
+	future_init_dbus_reply(&done, "select_properties_reconfig", info->msg);
+	dbus_message_unref(info->msg);
+
+	/* Reconfigure endpoints using the same flow as for initial config */
+	if (!err)
+		pac_select_all(info->data, true, done);
+
+	future_clear(&done, err, err_msg);
+
+	free(info);
+}
+
+static DBusMessage *select_properties(DBusConnection *conn, DBusMessage *msg,
+								void *user_data)
+{
+	struct bap_ep *ep = user_data;
+	struct bap_data *data = ep->data;
+	struct select_properties_data *info;
+	struct future *done = NULL;
+	bool defer = false;
+	DBusMessageIter args, props;
+
+	switch (bt_bap_pac_get_type(ep->lpac)) {
+	case BT_BAP_SOURCE:
+	case BT_BAP_SINK:
+		break;
+	default:
+		return btd_error_invalid_args(msg);
+	}
+
+	dbus_message_iter_init(msg, &args);
+	dbus_message_iter_recurse(&args, &props);
+	if (select_properties_parse(&props, &defer))
+		return btd_error_invalid_args(msg);
+
+	DBG("%s defer %d", ep->path, (int)defer);
+
+	ep->reconfigure = true;
+	if (defer)
+		return dbus_message_new_method_return(msg);
+
+	info = new0(struct select_properties_data, 1);
+	info->data = ep->data;
+	info->msg = dbus_message_ref(msg);
+
+	future_init(&done, "select_properties", select_properties_reconfig,
+									info);
+
+	queue_foreach(data->snks, ep_close_if_reconfigure, done);
+	queue_foreach(data->srcs, ep_close_if_reconfigure, done);
+
+	future_clear(&done, 0, NULL);
+	return NULL;
+}
+
 static bool stream_io_unset(const void *data, const void *user_data)
 {
 	struct bt_bap_stream *stream = (struct bt_bap_stream *)data;

@@ -1451,6 +1569,10 @@ static const GDBusMethodTable ep_methods[] = {
 	{ GDBUS_EXPERIMENTAL_ASYNC_METHOD("ClearConfiguration",
 					GDBUS_ARGS({ "transport", "o" }),
 					NULL, clear_configuration) },
+	{ GDBUS_EXPERIMENTAL_ASYNC_METHOD("SelectProperties",
+					GDBUS_ARGS(
+						{ "Properties", "a{sv}" }),
+					NULL, select_properties) },
 	{ },
 };
 

@@ -1684,13 +1806,6 @@ static void setup_config(void *data, void *user_data)
 	bt_bap_stream_set_user_data(setup->stream, ep->path);
 }
 
-static void bap_config(void *data, void *user_data)
-{
-	struct bap_ep *ep = data;
-
-	queue_foreach(ep->setups, setup_config, NULL);
-}
-
 static void select_cb(struct bt_bap_pac *pac, int err, struct iovec *caps,
 				struct iovec *metadata, struct bt_bap_qos *qos,
 				void *user_data)

@@ -1707,13 +1822,16 @@ static void select_cb(struct bt_bap_pac *pac, int err, struct iovec *caps,
 	setup->caps = util_iov_dup(caps, 1);
 	setup->metadata = util_iov_dup(metadata, 1);
 	setup->qos = *qos;
+	future_init_chain(&setup->qos_done, ep->setup_done);
 
 done:
 	DBG("ep %p selecting %d", ep, ep->selecting);
 
 	ep->selecting--;
-	if (!ep->selecting)
+	if (!ep->selecting) {
+		future_clear(&ep->setup_done, 0, NULL);
 		future_clear(&ep->select_done, 0, NULL);
+	}
 }
 
 static bool pac_register(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,

@@ -1734,6 +1852,8 @@ static bool pac_register(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
 struct pac_select_data {
 	struct bap_data *data;
 	struct future *select_done;
+	struct future *setup_done;
+	bool reconfigure;
 };
 
 static bool pac_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,

@@ -1763,12 +1883,21 @@ static bool pac_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
 		return true;
 	}
 
+	if (select_data->reconfigure) {
+		if (!ep->reconfigure)
+			return true;
+		ep->reconfigure = false;
+	}
+
 	/* TODO: Cache LRU? */
 	if (btd_service_is_initiator(service)) {
 		bt_bap_select(lpac, rpac, &ep->selecting, select_cb, ep);
-		if (ep->selecting)
+		if (ep->selecting) {
 			future_init_chain(&ep->select_done,
 						select_data->select_done);
+			future_init_chain(&ep->setup_done,
+						select_data->setup_done);
+		}
 	}
 
 	return true;

@@ -1788,6 +1917,7 @@ static void ep_cancel_select(struct bap_ep *ep)
 {
 	struct bt_bap *bap = ep->data->bap;
 
+	future_clear(&ep->setup_done, ECANCELED, NULL);
 	future_clear(&ep->select_done, ECANCELED, NULL);
 
 	bt_bap_foreach_pac(bap, BT_BAP_SOURCE, pac_cancel_select, ep);

@@ -1817,6 +1947,21 @@ static bool pac_found_bcast(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
 	return true;
 }
 
+static void setup_ensure_stream(void *data, void *user_data)
+{
+	struct bap_setup *setup = data;
+
+	if (!setup->stream)
+		setup_config(setup, user_data);
+}
+
+static void bap_config(void *data, void *user_data)
+{
+	struct bap_ep *ep = data;
+
+	queue_foreach(ep->setups, setup_ensure_stream, NULL);
+}
+
 static void select_complete_cb(int err, const char *err_msg, void *user_data)
 {
 	struct bap_data *data = user_data;

@@ -1828,10 +1973,13 @@ static void select_complete_cb(int err, const char *err_msg, void *user_data)
 	queue_foreach(data->snks, bap_config, NULL);
 }
 
-static void pac_select_all(struct bap_data *data)
+static void pac_select_all(struct bap_data *data, bool reconfigure,
+							struct future *done)
 {
 	struct pac_select_data select_data = {
 		.data = data,
+		.reconfigure = reconfigure,
+		.setup_done = done,
 	};
 
 	future_init(&select_data.select_done, "pac_select_all",

@@ -1856,7 +2004,7 @@ static void bap_ready(struct bt_bap *bap, void *user_data)
 	bt_bap_foreach_pac(bap, BT_BAP_SOURCE, pac_register, service);
 	bt_bap_foreach_pac(bap, BT_BAP_SINK, pac_register, service);
 
-	pac_select_all(data);
+	pac_select_all(data, false, NULL);
 }
 
 static bool match_setup_stream(const void *data, const void *user_data)

@@ -2499,7 +2647,11 @@ static void bap_state(struct bt_bap_stream *stream, uint8_t old_state,
 		}
 		break;
 	case BT_BAP_STREAM_STATE_QOS:
+		if (setup) {
 			setup_create_io(data, setup, stream, true);
+			future_clear(&setup->qos_done, setup->io ? 0 : EIO,
+							"Unable to configure");
+		}
 		break;
 	case BT_BAP_STREAM_STATE_ENABLING:
 		if (setup)

@@ -2806,7 +2958,7 @@ static void pac_added(struct bt_bap_pac *pac, void *user_data)
 	bt_bap_foreach_pac(data->bap, BT_BAP_SOURCE, pac_register, service);
 	bt_bap_foreach_pac(data->bap, BT_BAP_SINK, pac_register, service);
 
-	pac_select_all(data);
+	pac_select_all(data, false, NULL);
 }
 
 static void pac_added_broadcast(struct bt_bap_pac *pac, void *user_data)
Signature · 1 line
2.48.1
author
#12 lore raw

Re: [RFC PATCH BlueZ 6/9] shared/bap: make sure ucast client stream is destroyed after releasing

In reply to Pauli Virtanen

la, 2025-03-01 kello 17:57 +0200, Pauli Virtanen kirjoitti:

Quoted 10 lines

Upper layer as Unicast Client needs to be able to destroy streams when it wants to reconfigure endpoints.

This does not currently work right, because of Server issued Releasing->Config (caching) state transitions, which currently cause streams never enter Idle (so they are never destroyed).

Fix this by considering Releasing->Config as Releasing->Idle->Config. Also do not make new streams from cached config data as Unicast Client, and leave all stream configuration to upper layer.

This change also implies the following, so that reconfiguring multi-ASE configurations works right:

shared/bap: ucast client streams always use a free ASE

Because upper layer controls Unicast Client streams, bt_bap_stream_new() should for unicast always allocate an unused ASE.

src/shared/bap.c +4 -31
index 4f44db07a..a85169009 100644
@@ -5836,29 +5836,6 @@ static bool find_ep_unused(const void *data, const void *user_data)
 		return true;
 }
 
-static bool find_ep_pacs(const void *data, const void *user_data)
-{
-	const struct bt_bap_endpoint *ep = data;
-	const struct match_pac *match = user_data;
-
-	if (!ep->stream)
-		return false;
-
-	if (ep->stream->lpac != match->lpac)
-		return false;
-
-	if (ep->stream->rpac != match->rpac)
-		return false;
-
-	switch (ep->state) {
-	case BT_BAP_STREAM_STATE_CONFIG:
-	case BT_BAP_STREAM_STATE_QOS:
-		return true;
-	}
-
-	return false;
-}
-
 static bool find_ep_source(const void *data, const void *user_data)
 {
 	const struct bt_bap_endpoint *ep = data;

@@ -6053,15 +6030,11 @@ static struct bt_bap_stream *bap_ucast_stream_new(struct bt_bap *bap,
 	match.lpac = lpac;
 	match.rpac = rpac;
 
-	/* Check for existing stream */
-	ep = queue_find(bap->remote_eps, find_ep_pacs, &match);
+	/* Find unused ASE */
+	ep = queue_find(bap->remote_eps, find_ep_unused, &match);
 	if (!ep) {
-		/* Check for unused ASE */
-		ep = queue_find(bap->remote_eps, find_ep_unused, &match);
-		if (!ep) {
-			DBG(bap, "Unable to find unused ASE");
-			return NULL;
-		}
+		DBG(bap, "Unable to find unused ASE");
+		return NULL;
 	}
 
 	stream = ep->stream;
Quoted 61 lines
---
 src/shared/bap.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)
src/shared/bap.c +30 -2
index 54c6e8629..4f44db07a 100644
@@ -1363,6 +1363,31 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream)
 	struct bt_bap *bap = stream->bap;
 	const struct queue_entry *entry;
 
+	switch (stream->ep->old_state) {
+	case BT_ASCS_ASE_STATE_RELEASING:
+		/* After Releasing, Server may either transition to Config or
+		 * Idle. Our Unicast Client streams shall be considered
+		 * destroyed after Releasing, so that upper layer can control
+		 * stream creation. Make the lifecycle management simpler by
+		 * making sure the streams are destroyed by always emitting Idle
+		 * to upper layer after Releasing, even if the remote ASE did
+		 * not go through that state.
+		 */
+		if (stream->client &&
+				stream->ep->state != BT_ASCS_ASE_STATE_IDLE &&
+				(stream->lpac->type & (BT_BAP_SINK |
+							BT_BAP_SOURCE))) {
+			struct bt_bap_endpoint *ep = stream->ep;
+			uint8_t state = ep->state;
+
+			ep->state = BT_ASCS_ASE_STATE_IDLE;
+			bap_stream_state_changed(stream);
+			ep->state = state;
+			return;
+		}
+		break;
+	}
+
 	/* Pre notification updates */
 	switch (stream->ep->state) {
 	case BT_ASCS_ASE_STATE_IDLE:

@@ -4851,7 +4876,8 @@ static void ep_status_config(struct bt_bap *bap, struct bt_bap_endpoint *ep,
 	}
 
 	/* Any previously applied codec configuration may be cached by the
-	 * server.
+	 * server. However, all Unicast Client stream creation shall be left to
+	 * the upper layer.
 	 */
 	if (!ep->stream) {
 		struct match_pac match;

@@ -4866,7 +4892,9 @@ static void ep_status_config(struct bt_bap *bap, struct bt_bap_endpoint *ep,
 		if (!match.lpac || !match.rpac)
 			return;
 
-		bap_stream_new(bap, ep, match.lpac, match.rpac, NULL, true);
+		if (!(match.lpac->type & (BT_BAP_SINK | BT_BAP_SOURCE)))
+			bap_stream_new(bap, ep, match.lpac, match.rpac,
+								NULL, true);
 	}
 
 	if (!ep->stream)
#2 lore raw
In reply to Pauli Virtanen

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=939281

---Test result---

Test Summary:

CheckPatch                    PENDING   0.24 seconds
GitLint                       PENDING   0.24 seconds
BuildEll                      PASS      20.51 seconds
BluezMake                     PASS      1476.20 seconds
MakeCheck                     PASS      13.02 seconds
MakeDistcheck                 PASS      160.25 seconds
CheckValgrind                 PASS      215.99 seconds
CheckSmatch                   WARNING   299.79 seconds
bluezmakeextell               PASS      98.83 seconds
IncrementalBuild              PENDING   0.30 seconds
ScanBuild                     PASS      883.73 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/bap.c:305:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:305:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:305:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:305:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:305:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:305:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structures
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:

---
Regards,
Linux Bluetooth

maintainer
#3 lore raw

Re: [RFC PATCH BlueZ 1/9] org.bluez.MediaEndpoint: removing BAP streams with ClearConfiguration

In reply to Pauli Virtanen

Hi Pauli,

Quoted 13 lines · Pauli Virtanen

Allow user to remove specific streams by calling ClearConfiguration(transport_path) on the endpoint. If the path is the endpoint path instead, clear all streams associated with the endpoint.

---
 doc/org.bluez.MediaEndpoint.rst | 6 ++++++
 1 file changed, 6 insertions(+)
doc/org.bluez.MediaEndpoint.rst +0 -0
index f2b830ab0..b81106f0b 100644

doc/org.bluez.MediaEndpoint.rst void ClearConfiguration(object transport) · line 116

Quoted 8 lines
doc/org.bluez.MediaEndpoint.rst +5 -0
@@ -109,6 +109,12 @@ void ClearConfiguration(object transport)
 
        Clear transport configuration.
 
+       **Server role:** [ISO only]
+
+       Close the stream associated with the given transport. If the
+       path given is the path of this endpoint, all its streams are
+       closed.

This seems sort of trivial, that said we can't really guarantee the MediaTransports will be closed even if we send an ASCS_Release operation the server may still cache the codec configuration.

Quoted 7 lines
doc/org.bluez.MediaEndpoint.rst +0 -1
 void Release()
 ``````````````
 
--

2.48.1

Signature · 1 line
Luiz Augusto von Dentz
maintainer
#6 lore raw

Re: [RFC PATCH BlueZ 2/9] org.bluez.MediaEndpoint: add client role SelectProperties

In reply to Pauli Virtanen

Hi Pauli,

Quoted 7 lines · Pauli Virtanen

Add a simple way for the sound server to reconfigure a BAP unicast endpoint, by calling org.bluez.MediaEndpoint.SelectProperties().

This shall destroy all streams of the endpoint, and restart the SelectProperties() configuration flow from the beginning.

Quoted 8 lines

Since it may be necessary to reconfigure multiple endpoints at once to correctly make bidirectional CIS, add Defer argument to just mark eps for configuration.

In future, org.bluez.MediaEndpoint.SetConfiguration() could be changed to handle unicast in the same way as broadcast: only create streams. This allows sound server to have full control over stream configuration itself, and not rely on bt_bap_select().

This one Im not too sure, perhaps it would be better to add something like org.bluez.MediaTransport.Reconfigure to avoid having to add role dependent documentation like bellow

Quoted 29 lines
---
 doc/org.bluez.MediaEndpoint.rst | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
doc/org.bluez.MediaEndpoint.rst +10 -0
index b81106f0b..5c42b878c 100644
@@ -69,6 +69,8 @@ array{byte} SelectConfiguration(array{byte} capabilities)
 dict SelectProperties(dict capabilities)
 ````````````````````````````````````````
 
+       **Server Role**
+
        Select BAP unicast configuration from the supported capabilities:
 
        :object Endpoint:

@@ -104,6 +106,25 @@ dict SelectProperties(dict capabilities)
        Note: There is no need to cache the selected properties since on
        success the configuration is send back as parameter of SetConfiguration.
 
+       **Client Role**
+
+       Reconfigure a BAP unicast endpoint. This closes all existing
+       streams of the endpoint, and restarts the configuration
+       selection flow which e.g. triggers calls to *SelectProperties*
+       allowing the sound server to modify the configuration.
+
+       The following arguments are taken in *capabilities*:

doc/org.bluez.MediaEndpoint.rst dict SelectProperties(dict capabilities) · line 126

Quoted 10 lines
doc/org.bluez.MediaEndpoint.rst +10 -0
+
+       :boolean Defer [optional]:
+
+               If True, mark endpoint for reconfiguration, but
+               postpone it until a non-deferred *SelectProperties()*
+               operation is made on an endpoint of the same device.
+
+               This is necessary to use when reconfiguring source and
+               sink streams with the intention that they be combined
+               into the same CIG, possibly forming bidirectional CIS.

We could perhaps adds Defer option to MediaTransport.Reconfigure and then use MediaTransport.Select to clear it, that said for broadcast we did set a dedicated state when it is not active so perhaps we should do something similar here.

Quoted 7 lines
doc/org.bluez.MediaEndpoint.rst +0 -1
 void ClearConfiguration(object transport)
 `````````````````````````````````````````
 
--

2.48.1

Signature · 1 line
Luiz Augusto von Dentz
maintainer
#8 lore raw

Re: [RFC PATCH BlueZ 3/9] bap: add and use chainable future abstraction

In reply to Pauli Virtanen

Hi Pauli,

Quoted 25 lines · Pauli Virtanen

Multi-part operations will need to postpone things like DBus replies until all parts are complete. Make this a bit simpler with a chainable future.

---
 profiles/audio/bap.c | 136 +++++++++++++++++++++++++++++++++----------
 1 file changed, 105 insertions(+), 31 deletions(-)
profiles/audio/bap.c +1 -1
index 37168e58c..8b9b89c70 100644
@@ -80,7 +80,7 @@ struct bap_setup {
        struct iovec *metadata;
        unsigned int id;
        struct iovec *base;
-       DBusMessage *msg;
+       struct future *qos_done;
        void (*destroy)(struct bap_setup *setup);
 };
 

@@ -114,6 +114,17 @@ struct bap_data {
        void *user_data;
 };

profiles/audio/bap.c struct bap_data { · line 126

Quoted 10 lines
profiles/audio/bap.c +10 -0
+typedef void (*future_func_t)(int err, const char *err_msg, void *data);
+
+struct future {
+       unsigned int step, steps;
+       const char *name;
+       future_func_t func;
+       void *data;
+       int err;
+       const char *err_msg;
+};

This I'm not convinced is the right direction, it will be sort of hard to make it generic enough besides I think this should actually be handled directly by bt_bap instead, it is actually weird that the testing spec doesn't capture stream reconfiguration from streaming state for example, in any case we can come up with our on tests for it to ensure the stream is properly released and CIS is closed before we attempt reconfigure it for example.

Quoted 184 lines
profiles/audio/bap.c +93 -31
 static struct queue *sessions;
 
 /* Structure holding the parameters for Periodic Advertisement create sync.

@@ -155,6 +166,94 @@ struct bt_iso_qos bap_sink_pa_qos = {
        }
 };
 
+static void future_clear(struct future **p, int err, const char *err_msg)
+{
+       struct future *h = *p;
+
+       if (!h)
+               return;
+
+       DBG("future %p (%s) 0x%02x (%s) step %u/%u", h, h->name ? h->name : "",
+               err, (err && err_msg) ? err_msg : "", h->step + 1, h->steps);
+
+       *p = NULL;
+
+       if (err && !h->err) {
+               h->err = err;
+               h->err_msg = err_msg;
+       }
+
+       if (++h->step < h->steps)
+               return;
+
+       h->func(h->err, h->err_msg, h->data);
+       free(h);
+}
+
+static void future_dbus_callback_func(int err, const char *err_msg, void *data)
+{
+       DBusMessage *msg = data;
+       DBusMessage *reply;
+
+       if (err && !err_msg)
+               err_msg = strerror(err);
+
+       switch (err) {
+       case 0:
+               reply = dbus_message_new_method_return(msg);
+               break;
+       case EINVAL:
+               reply = btd_error_invalid_args_str(msg, err_msg);
+               break;
+       default:
+               reply = btd_error_failed(msg, err_msg);
+               break;
+       }
+
+       g_dbus_send_message(btd_get_dbus_connection(), reply);
+
+       dbus_message_unref(msg);
+}
+
+static void future_init(struct future **p, const char *name, future_func_t func,
+                                                               void *data)
+{
+       struct future *h;
+
+       future_clear(p, ECANCELED, NULL);
+
+       h = new0(struct future, 1);
+       h->steps = 1;
+       h->name = name;
+       h->func = func;
+       h->data = data;
+
+       DBG("future %p (%s) init", h, h->name ? h->name : "");
+
+       *p = h;
+}
+
+static void future_init_dbus_reply(struct future **p, const char *name,
+                                                       DBusMessage *msg)
+{
+       future_init(p, name, future_dbus_callback_func, dbus_message_ref(msg));
+}
+
+__attribute__((unused))
+static void future_init_chain(struct future **p, struct future *h)
+{
+       future_clear(p, ECANCELED, NULL);
+
+       if (h) {
+               h->steps++;
+
+               DBG("future %p (%s) init step %u", h, h->name ? h->name : "",
+                                                               h->steps);
+       }
+
+       *p = h;
+}
+
 static bool bap_data_set_user_data(struct bap_data *data, void *user_data)
 {
        if (!data)

@@ -740,24 +839,12 @@ static void qos_cb(struct bt_bap_stream *stream, uint8_t code, uint8_t reason,
                                        void *user_data)
 {
        struct bap_setup *setup = user_data;
-       DBusMessage *reply;
 
        DBG("stream %p code 0x%02x reason 0x%02x", stream, code, reason);
 
        setup->id = 0;
 
-       if (!setup->msg)
-               return;
-
-       if (!code)
-               reply = dbus_message_new_method_return(setup->msg);
-       else
-               reply = btd_error_failed(setup->msg, "Unable to configure");
-
-       g_dbus_send_message(btd_get_dbus_connection(), reply);
-
-       dbus_message_unref(setup->msg);
-       setup->msg = NULL;
+       future_clear(&setup->qos_done, code ? EIO : 0, "Unable to configure");
 }
 
 static void config_cb(struct bt_bap_stream *stream,

@@ -765,7 +852,6 @@ static void config_cb(struct bt_bap_stream *stream,
                                        void *user_data)
 {
        struct bap_setup *setup = user_data;
-       DBusMessage *reply;
 
        DBG("stream %p code 0x%02x reason 0x%02x", stream, code, reason);
 

@@ -786,14 +872,7 @@ static void config_cb(struct bt_bap_stream *stream,
                return;
        }
 
-       if (!setup->msg)
-               return;
-
-       reply = btd_error_failed(setup->msg, "Unable to configure");
-       g_dbus_send_message(btd_get_dbus_connection(), reply);
-
-       dbus_message_unref(setup->msg);
-       setup->msg = NULL;
+       future_clear(&setup->qos_done, EIO, "Unable to configure");
 }
 
 static void setup_io_close(void *data, void *user_data)

@@ -872,7 +951,6 @@ static struct bap_setup *setup_new(struct bap_ep *ep)
 static void setup_free(void *data)
 {
        struct bap_setup *setup = data;
-       DBusMessage *reply;
 
        DBG("%p", setup);
 

@@ -881,12 +959,7 @@ static void setup_free(void *data)
                setup->id = 0;
        }
 
-       if (setup->msg) {
-               reply = btd_error_failed(setup->msg, "Canceled");
-               g_dbus_send_message(btd_get_dbus_connection(), reply);
-               dbus_message_unref(setup->msg);
-               setup->msg = NULL;
-       }
+       future_clear(&setup->qos_done, ECANCELED, NULL);
 
        if (setup->ep)
                queue_remove(setup->ep->setups, setup);

@@ -1038,7 +1111,8 @@ static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,
 
        switch (bt_bap_stream_get_type(setup->stream)) {
        case BT_BAP_STREAM_TYPE_UCAST:
-               setup->msg = dbus_message_ref(msg);
+               future_init_dbus_reply(&setup->qos_done, "set_configuration",
+                                                                       msg);
                break;
        case BT_BAP_STREAM_TYPE_BCAST:
                /* No message sent over the air for broadcast */
--

2.48.1

Signature · 1 line
Luiz Augusto von Dentz
maintainer
#13 lore raw

Re: [RFC PATCH BlueZ 6/9] shared/bap: make sure ucast client stream is destroyed after releasing

In reply to Pauli Virtanen

Hi Pauli,

Quoted 12 lines · Pauli Virtanen

la, 2025-03-01 kello 17:57 +0200, Pauli Virtanen kirjoitti:

Quoted 10 lines

Upper layer as Unicast Client needs to be able to destroy streams when it wants to reconfigure endpoints.

This does not currently work right, because of Server issued Releasing->Config (caching) state transitions, which currently cause streams never enter Idle (so they are never destroyed).

Fix this by considering Releasing->Config as Releasing->Idle->Config. Also do not make new streams from cached config data as Unicast Client, and leave all stream configuration to upper layer.

Not sure I follow you here, how can we consider it idle even for cached config data? If we don't keep the stream there won't be a MediaTransport representing it either so it won't even be possible to know from the upper layer there is something already configured, so I really think we should have an option to reconfigure at MediaTransport layer rather than trying to hide it somehow.

Quoted 43 lines

This change also implies the following, so that reconfiguring multi-ASE configurations works right:

shared/bap: ucast client streams always use a free ASE

Because upper layer controls Unicast Client streams, bt_bap_stream_new() should for unicast always allocate an unused ASE.

src/shared/bap.c +0 -23
index 4f44db07a..a85169009 100644
@@ -5836,29 +5836,6 @@ static bool find_ep_unused(const void *data, const void *user_data)
                return true;
 }
 
-static bool find_ep_pacs(const void *data, const void *user_data)
-{
-       const struct bt_bap_endpoint *ep = data;
-       const struct match_pac *match = user_data;
-
-       if (!ep->stream)
-               return false;
-
-       if (ep->stream->lpac != match->lpac)
-               return false;
-
-       if (ep->stream->rpac != match->rpac)
-               return false;
-
-       switch (ep->state) {
-       case BT_BAP_STREAM_STATE_CONFIG:
-       case BT_BAP_STREAM_STATE_QOS:
-               return true;
-       }
-
-       return false;
-}
-
 static bool find_ep_source(const void *data, const void *user_data)
 {
        const struct bt_bap_endpoint *ep = data;

src/shared/bap.c static struct bt_bap_stream *bap_ucast_stream_new(struct bt_bap *bap, · line 6034

Quoted 7 lines
src/shared/bap.c +2 -2
@@ -6053,15 +6030,11 @@ static struct bt_bap_stream *bap_ucast_stream_new(struct bt_bap *bap,
        match.lpac = lpac;
        match.rpac = rpac;
 
-       /* Check for existing stream */
-       ep = queue_find(bap->remote_eps, find_ep_pacs, &match);
+       /* Find unused ASE */
+       ep = queue_find(bap->remote_eps, find_ep_unused, &match);

And if there aren't any ASE left then what? I'd go with the design the spec suggests that stream can be reconfigured for QoS Configuration state down to Idle, the only hard part is figuring out if a stream state affects another, etc.

Quoted 78 lines
src/shared/bap.c +2 -6
        if (!ep) {
-               /* Check for unused ASE */
-               ep = queue_find(bap->remote_eps, find_ep_unused, &match);
-               if (!ep) {
-                       DBG(bap, "Unable to find unused ASE");
-                       return NULL;
-               }
+               DBG(bap, "Unable to find unused ASE");
+               return NULL;
        }
 
        stream = ep->stream;
Quoted 64 lines


---
 src/shared/bap.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)
src/shared/bap.c +30 -2
index 54c6e8629..4f44db07a 100644
@@ -1363,6 +1363,31 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream)
      struct bt_bap *bap = stream->bap;
      const struct queue_entry *entry;
 
+     switch (stream->ep->old_state) {
+     case BT_ASCS_ASE_STATE_RELEASING:
+             /* After Releasing, Server may either transition to Config or
+              * Idle. Our Unicast Client streams shall be considered
+              * destroyed after Releasing, so that upper layer can control
+              * stream creation. Make the lifecycle management simpler by
+              * making sure the streams are destroyed by always emitting Idle
+              * to upper layer after Releasing, even if the remote ASE did
+              * not go through that state.
+              */
+             if (stream->client &&
+                             stream->ep->state != BT_ASCS_ASE_STATE_IDLE &&
+                             (stream->lpac->type & (BT_BAP_SINK |
+                                                     BT_BAP_SOURCE))) {
+                     struct bt_bap_endpoint *ep = stream->ep;
+                     uint8_t state = ep->state;
+
+                     ep->state = BT_ASCS_ASE_STATE_IDLE;
+                     bap_stream_state_changed(stream);
+                     ep->state = state;
+                     return;
+             }
+             break;
+     }
+
      /* Pre notification updates */
      switch (stream->ep->state) {
      case BT_ASCS_ASE_STATE_IDLE:

@@ -4851,7 +4876,8 @@ static void ep_status_config(struct bt_bap *bap, struct bt_bap_endpoint *ep,
      }
 
      /* Any previously applied codec configuration may be cached by the
-      * server.
+      * server. However, all Unicast Client stream creation shall be left to
+      * the upper layer.
       */
      if (!ep->stream) {
              struct match_pac match;

@@ -4866,7 +4892,9 @@ static void ep_status_config(struct bt_bap *bap, struct bt_bap_endpoint *ep,
              if (!match.lpac || !match.rpac)
                      return;
 
-             bap_stream_new(bap, ep, match.lpac, match.rpac, NULL, true);
+             if (!(match.lpac->type & (BT_BAP_SINK | BT_BAP_SOURCE)))
+                     bap_stream_new(bap, ep, match.lpac, match.rpac,
+                                                             NULL, true);
      }
 
      if (!ep->stream)
Signature · 1 line
Luiz Augusto von Dentz
author
#14 lore raw

Re: [RFC PATCH BlueZ 6/9] shared/bap: make sure ucast client stream is destroyed after releasing

Hi Luiz,

ma, 2025-03-17 kello 14:55 -0400, Luiz Augusto von Dentz kirjoitti:

Quoted 16 lines

Hi Pauli,

Quoted 12 lines · Pauli Virtanen

la, 2025-03-01 kello 17:57 +0200, Pauli Virtanen kirjoitti:

Quoted 10 lines

Upper layer as Unicast Client needs to be able to destroy streams when it wants to reconfigure endpoints.

This does not currently work right, because of Server issued Releasing->Config (caching) state transitions, which currently cause streams never enter Idle (so they are never destroyed).

Fix this by considering Releasing->Config as Releasing->Idle->Config. Also do not make new streams from cached config data as Unicast Client, and leave all stream configuration to upper layer.

Quoted 6 lines

Not sure I follow you here, how can we consider it idle even for cached config data? If we don't keep the stream there won't be a MediaTransport representing it either so it won't even be possible to know from the upper layer there is something already configured, so I really think we should have an option to reconfigure at MediaTransport layer rather than trying to hide it somehow.

The idea is that in Client role, the upper layer does not care whether an ASE is configured or idle. It has to (re)configure it before it wants to use it. The cost of this is that you are sometimes doing some Config Codec that wouldn't be strictly needed, but it is harmless.

The problem here was that that linking of CIS gets broken by the configured streams. That said, now I see it might be addressed just by clearing the stream QoS and linking state when Releasing (QoS is then invalid anyway so it needs to be cleared).

Quoted 136 lines
Quoted 43 lines

This change also implies the following, so that reconfiguring multi-ASE configurations works right:

shared/bap: ucast client streams always use a free ASE

Because upper layer controls Unicast Client streams, bt_bap_stream_new() should for unicast always allocate an unused ASE.

src/shared/bap.c +0 -23
index 4f44db07a..a85169009 100644
@@ -5836,29 +5836,6 @@ static bool find_ep_unused(const void *data, const void *user_data)
                return true;
 }
 
-static bool find_ep_pacs(const void *data, const void *user_data)
-{
-       const struct bt_bap_endpoint *ep = data;
-       const struct match_pac *match = user_data;
-
-       if (!ep->stream)
-               return false;
-
-       if (ep->stream->lpac != match->lpac)
-               return false;
-
-       if (ep->stream->rpac != match->rpac)
-               return false;
-
-       switch (ep->state) {
-       case BT_BAP_STREAM_STATE_CONFIG:
-       case BT_BAP_STREAM_STATE_QOS:
-               return true;
-       }
-
-       return false;
-}
-
 static bool find_ep_source(const void *data, const void *user_data)
 {
        const struct bt_bap_endpoint *ep = data;

src/shared/bap.c static struct bt_bap_stream *bap_ucast_stream_new(struct bt_bap *bap, · line 6034

Quoted 7 lines
src/shared/bap.c +2 -2
@@ -6053,15 +6030,11 @@ static struct bt_bap_stream *bap_ucast_stream_new(struct bt_bap *bap,
        match.lpac = lpac;
        match.rpac = rpac;
 
-       /* Check for existing stream */
-       ep = queue_find(bap->remote_eps, find_ep_pacs, &match);
+       /* Find unused ASE */
+       ep = queue_find(bap->remote_eps, find_ep_unused, &match);

And if there aren't any ASE left then what? I'd go with the design the spec suggests that stream can be reconfigured for QoS Configuration state down to Idle, the only hard part is figuring out if a stream state affects another, etc.

Quoted 78 lines
src/shared/bap.c +2 -6
        if (!ep) {
-               /* Check for unused ASE */
-               ep = queue_find(bap->remote_eps, find_ep_unused, &match);
-               if (!ep) {
-                       DBG(bap, "Unable to find unused ASE");
-                       return NULL;
-               }
+               DBG(bap, "Unable to find unused ASE");
+               return NULL;
        }
 
        stream = ep->stream;
Quoted 64 lines


---
 src/shared/bap.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)
src/shared/bap.c +30 -2
index 54c6e8629..4f44db07a 100644
@@ -1363,6 +1363,31 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream)
      struct bt_bap *bap = stream->bap;
      const struct queue_entry *entry;
 
+     switch (stream->ep->old_state) {
+     case BT_ASCS_ASE_STATE_RELEASING:
+             /* After Releasing, Server may either transition to Config or
+              * Idle. Our Unicast Client streams shall be considered
+              * destroyed after Releasing, so that upper layer can control
+              * stream creation. Make the lifecycle management simpler by
+              * making sure the streams are destroyed by always emitting Idle
+              * to upper layer after Releasing, even if the remote ASE did
+              * not go through that state.
+              */
+             if (stream->client &&
+                             stream->ep->state != BT_ASCS_ASE_STATE_IDLE &&
+                             (stream->lpac->type & (BT_BAP_SINK |
+                                                     BT_BAP_SOURCE))) {
+                     struct bt_bap_endpoint *ep = stream->ep;
+                     uint8_t state = ep->state;
+
+                     ep->state = BT_ASCS_ASE_STATE_IDLE;
+                     bap_stream_state_changed(stream);
+                     ep->state = state;
+                     return;
+             }
+             break;
+     }
+
      /* Pre notification updates */
      switch (stream->ep->state) {
      case BT_ASCS_ASE_STATE_IDLE:

@@ -4851,7 +4876,8 @@ static void ep_status_config(struct bt_bap *bap, struct bt_bap_endpoint *ep,
      }
 
      /* Any previously applied codec configuration may be cached by the
-      * server.
+      * server. However, all Unicast Client stream creation shall be left to
+      * the upper layer.
       */
      if (!ep->stream) {
              struct match_pac match;

@@ -4866,7 +4892,9 @@ static void ep_status_config(struct bt_bap *bap, struct bt_bap_endpoint *ep,
              if (!match.lpac || !match.rpac)
                      return;
 
-             bap_stream_new(bap, ep, match.lpac, match.rpac, NULL, true);
+             if (!(match.lpac->type & (BT_BAP_SINK | BT_BAP_SOURCE)))
+                     bap_stream_new(bap, ep, match.lpac, match.rpac,
+                                                             NULL, true);
      }
 
      if (!ep->stream)
Signature · 1 line
Pauli Virtanen
author
#4 lore raw

Re: [RFC PATCH BlueZ 1/9] org.bluez.MediaEndpoint: removing BAP streams with ClearConfiguration

In reply to Luiz Augusto von Dentz

Hi,

ma, 2025-03-17 kello 14:10 -0400, Luiz Augusto von Dentz kirjoitti:

Quoted 19 lines

Hi Pauli,

Quoted 16 lines · Pauli Virtanen

Allow user to remove specific streams by calling ClearConfiguration(transport_path) on the endpoint. If the path is the endpoint path instead, clear all streams associated with the endpoint.

---
 doc/org.bluez.MediaEndpoint.rst | 6 ++++++
 1 file changed, 6 insertions(+)
doc/org.bluez.MediaEndpoint.rst +0 -0
index f2b830ab0..b81106f0b 100644
@@ -109,6 +109,12 @@ void ClearConfiguration(object transport)
 
        Clear transport configuration.
Quoted 9 lines

doc/org.bluez.MediaEndpoint.rst void ClearConfiguration(object transport) · line 116

Quoted 5 lines
doc/org.bluez.MediaEndpoint.rst +5 -0
+       **Server role:** [ISO only]
+
+       Close the stream associated with the given transport. If the
+       path given is the path of this endpoint, all its streams are
+       closed.

This seems sort of trivial, that said we can't really guarantee the MediaTransports will be closed even if we send an ASCS_Release operation the server may still cache the codec configuration.

It does not work if we consider every ASE in Config Codec state is associated with a transport. BAP Client should be able to fully control what it transitions to QoS state, so transport = QoS state makes more sense. (See also BAP §2.2.1.1 / 2.2.1.2)

Client must be able to transition from state where it e.g. uses sources & sinks to a state where it only uses sink.

Some devices have restrictions that they cannot use sources with eg. 48kHz rate sinks together. The same devices may also always transition all ASEs to Config Codec. As Client, we must be able to ignore some of the configured ASEs, and transition to QoS only the ones we want to use.

Signature · 1 line
Pauli Virtanen