diff mbox series

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

Message ID 5f103220d38f8eb549eb41ac971d1f4cf1e684ba.1740844616.git.pav@iki.fi (mailing list archive)
State New
Headers show
Series BAP stream reconfiguration | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success

Commit Message

Pauli Virtanen March 1, 2025, 3:57 p.m. UTC 
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(-)

Comments

Pauli Virtanen March 1, 2025, 4:26 p.m. UTC | #1 
la, 2025-03-01 kello 17:57 +0200, Pauli Virtanen kirjoitti:
> 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.

diff --git a/src/shared/bap.c b/src/shared/bap.c
index 4f44db07a..a85169009 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -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;



> ---
>  src/shared/bap.c | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index 54c6e8629..4f44db07a 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -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)
diff mbox series

Patch

diff --git a/src/shared/bap.c b/src/shared/bap.c
index 54c6e8629..4f44db07a 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -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)