[PATCH BlueZ 1/2] shared/bap: detach io for source ASEs at QoS when disabling

[PATCH BlueZ 1/2] shared/bap: detach io for source ASEs at QoS when disabling

[Pauli Virtanen]

The Client may terminate a CIS when sink is in QOS and source in
Disabling states (BAP v1.0.1 Sec 5.6.5).  It may also terminate it when
Receiver Stop Ready has completed successfully (BAP v1.0.1 Sec 5.6.5.1).
On successful Receiver Stop Ready the Server shall transition the ASE
back to QoS state (ASCS v1.0 Sec 5.6).

It appears Samsung Galaxy Buds2 Pro (R510XXUOAWA5) ignores the Receiver
Stop Ready command if CIS is already disconnected, and then gets stuck
in disabling state. It works if CIS is disconnected after Receiver Stop
Ready.

For better compatibility, disconnect CIS only after the source ASE is
back in the QoS state. This is what we also do with sinks.

Link: https://github.com/bluez/bluez/issues/516
---
 src/shared/bap.c | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/src/shared/bap.c b/src/shared/bap.c
index f194f466f..16a9cec5b 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -1115,18 +1115,6 @@ static bool match_stream_io(const void *data, const void *user_data)
 	return stream->io == io;
 }
 
-static void stream_stop_disabling(void *data, void *user_data)
-{
-	struct bt_bap_stream *stream = data;
-
-	if (stream->io || stream->ep->state != BT_ASCS_ASE_STATE_DISABLING)
-		return;
-
-	DBG(stream->bap, "stream %p", stream);
-
-	bt_bap_stream_stop(stream, NULL, NULL);
-}
-
 static bool bap_stream_io_detach(struct bt_bap_stream *stream)
 {
 	struct bt_bap_stream *link;
@@ -1145,9 +1133,6 @@ static bool bap_stream_io_detach(struct bt_bap_stream *stream)
 		/* Detach link if in QoS state */
 		if (link->ep->state == BT_ASCS_ASE_STATE_QOS)
 			bap_stream_io_detach(link);
-	} else {
-		/* Links without IO on disabling state shall be stopped. */
-		queue_foreach(stream->links, stream_stop_disabling, NULL);
 	}
 
 	stream_io_unref(io);
@@ -1218,7 +1203,6 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream)
 		bap_stream_update_io_links(stream);
 		break;
 	case BT_ASCS_ASE_STATE_DISABLING:
-		bap_stream_io_detach(stream);
 		break;
 	case BT_ASCS_ASE_STATE_QOS:
 		if (stream->io && !stream->io->connecting)
@@ -1252,8 +1236,8 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream)
 			bt_bap_stream_start(stream, NULL, NULL);
 		break;
 	case BT_ASCS_ASE_STATE_DISABLING:
-		if (!bt_bap_stream_get_io(stream))
-			bt_bap_stream_stop(stream, NULL, NULL);
+		/* IO is detached when back in QOS */
+		bt_bap_stream_stop(stream, NULL, NULL);
 		break;
 	}
 
-- 
2.40.1

[PATCH BlueZ 2/2] bap: wait for CIG to become configurable before recreating CIS

[Pauli Virtanen]

ISO sockets cannot be reconnected before all sockets in the same CIG
have been closed, if the CIG was previously active.

Keep track which endpoints have active CIG, and postpone connecting CIS
until their CIG is no longer active.

This addresses getting EBUSY from connect() when multiple CIS in the
same CIG move streaming -> qos at the same time, which disconnects CIS
and recreates them.  The EBUSY originates from COMMAND_DISALLOWED
response to Set CIG Parameters.

This requires the kernel side do the Disconnect CIS / Remove CIG / Set
CIG Parameters HCI command steps in the right order, when all old
sockets are closed first before connecting new ones.
---

Notes:
    I sent a patchset that fixes also the kernel issues. With this BlueZ
    patchset and the kernel patchset, disabling and re-enabling ASEs with
    Intel AX210 + Samsung Galaxy Buds2 Pro works. Sound server can also now
    release and reacquire the sinks normally, fixing
    https://github.com/bluez/bluez/issues/516
    
    Current bluetooth-next kernel does not guarantee correct ordering of the
    operations when the socket operations are done fast, and looking at this
    found a few other problems with doing the Set CIG Parameters and Create
    CIS commands.
    
    I get the wrong ordering of Remove CIG / Set CIG Parameters when
    doing this with Intel AX210 + Samsung Galaxy Buds2 Pro.
    
    I tried to write reconnect tests to iso-tester to reproduce this, but
    the required timing to trigger it wasn't easy to do, so the test doesn't
    hit this condition. It however uncovered issues in handling of Create
    CIS.

 profiles/audio/bap.c | 107 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 97 insertions(+), 10 deletions(-)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index 1a543a9ce..063da0786 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -66,6 +66,7 @@ struct bap_ep {
 	GIOChannel *io;
 	unsigned int io_id;
 	bool recreate;
+	bool cig_active;
 	struct iovec *caps;
 	struct iovec *metadata;
 	struct bt_bap_qos qos;
@@ -428,6 +429,7 @@ static void bap_io_close(struct bap_ep *ep)
 
 	g_io_channel_unref(ep->io);
 	ep->io = NULL;
+	ep->cig_active = false;
 }
 
 static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,
@@ -794,7 +796,7 @@ drop:
 	g_io_channel_shutdown(io, TRUE, NULL);
 }
 
-static void bap_accept_io(struct bap_data *data, struct bt_bap_stream *stream,
+static void bap_accept_io(struct bap_ep *ep, struct bt_bap_stream *stream,
 							int fd, int defer)
 {
 	char c;
@@ -831,12 +833,52 @@ static void bap_accept_io(struct bap_data *data, struct bt_bap_stream *stream,
 		}
 	}
 
+	ep->cig_active = true;
+
 	return;
 
 fail:
 	close(fd);
 }
 
+struct cig_busy_data {
+	struct btd_adapter *adapter;
+	uint8_t cig;
+};
+
+static bool cig_busy_ep(const void *data, const void *match_data)
+{
+	const struct bap_ep *ep = data;
+	const struct cig_busy_data *info = match_data;
+
+	return (ep->qos.cig_id == info->cig) && ep->cig_active;
+}
+
+static bool cig_busy_session(const void *data, const void *match_data)
+{
+	const struct bap_data *session = data;
+	const struct cig_busy_data *info = match_data;
+
+	if (device_get_adapter(session->device) != info->adapter)
+		return false;
+
+	return queue_find(session->snks, cig_busy_ep, match_data) ||
+			queue_find(session->srcs, cig_busy_ep, match_data);
+}
+
+static bool is_cig_busy(struct bap_data *data, uint8_t cig)
+{
+	struct cig_busy_data info;
+
+	if (cig == BT_ISO_QOS_CIG_UNSET)
+		return false;
+
+	info.adapter = device_get_adapter(data->device);
+	info.cig = cig;
+
+	return queue_find(sessions, cig_busy_session, &info);
+}
+
 static void bap_create_io(struct bap_data *data, struct bap_ep *ep,
 				struct bt_bap_stream *stream, int defer);
 
@@ -853,6 +895,48 @@ static gboolean bap_io_recreate(void *user_data)
 	return FALSE;
 }
 
+static void recreate_cig_ep(void *data, void *match_data)
+{
+	struct bap_ep *ep = (struct bap_ep *)data;
+	struct cig_busy_data *info = match_data;
+
+	if (ep->qos.cig_id != info->cig || !ep->recreate || ep->io_id)
+		return;
+
+	ep->recreate = false;
+	ep->io_id = g_idle_add(bap_io_recreate, ep);
+}
+
+static void recreate_cig_session(void *data, void *match_data)
+{
+	struct bap_data *session = data;
+	struct cig_busy_data *info = match_data;
+
+	if (device_get_adapter(session->device) != info->adapter)
+		return;
+
+	queue_foreach(session->snks, recreate_cig_ep, match_data);
+	queue_foreach(session->srcs, recreate_cig_ep, match_data);
+}
+
+static void recreate_cig(struct bap_ep *ep)
+{
+	struct bap_data *data = ep->data;
+	struct cig_busy_data info;
+
+	info.adapter = device_get_adapter(data->device);
+	info.cig = ep->qos.cig_id;
+
+	DBG("adapter %p ep %p recreate CIG %d", info.adapter, ep, info.cig);
+
+	if (ep->qos.cig_id == BT_ISO_QOS_CIG_UNSET) {
+		recreate_cig_ep(ep, &info);
+		return;
+	}
+
+	queue_foreach(sessions, recreate_cig_session, &info);
+}
+
 static gboolean bap_io_disconnected(GIOChannel *io, GIOCondition cond,
 							gpointer user_data)
 {
@@ -865,10 +949,8 @@ static gboolean bap_io_disconnected(GIOChannel *io, GIOCondition cond,
 	bap_io_close(ep);
 
 	/* Check if connecting recreate IO */
-	if (ep->recreate) {
-		ep->recreate = false;
-		ep->io_id = g_idle_add(bap_io_recreate, ep);
-	}
+	if (!is_cig_busy(ep->data, ep->qos.cig_id))
+		recreate_cig(ep);
 
 	return FALSE;
 }
@@ -893,18 +975,22 @@ static void bap_connect_io(struct bap_data *data, struct bap_ep *ep,
 	int fd;
 
 	/* If IO already set skip creating it again */
-	if (bt_bap_stream_get_io(stream))
+	if (bt_bap_stream_get_io(stream)) {
+		DBG("ep %p stream %p has existing io", ep, stream);
 		return;
+	}
 
 	if (bt_bap_stream_io_is_connecting(stream, &fd)) {
-		bap_accept_io(data, stream, fd, defer);
+		bap_accept_io(ep, stream, fd, defer);
 		return;
 	}
 
-	/* If IO channel still up wait for it to be disconnected and then
-	 * recreate.
+	/* If IO channel still up or CIG is busy, wait for it to be
+	 * disconnected and then recreate.
 	 */
-	if (ep->io) {
+	if (ep->io || is_cig_busy(data, ep->qos.cig_id)) {
+		DBG("ep %p stream %p defer %s wait recreate", ep, stream,
+						defer ? "true" : "false");
 		ep->recreate = true;
 		return;
 	}
@@ -937,6 +1023,7 @@ static void bap_connect_io(struct bap_data *data, struct bap_ep *ep,
 						bap_io_disconnected, ep);
 
 	ep->io = io;
+	ep->cig_active = !defer;
 
 	bt_bap_stream_io_connecting(stream, g_io_channel_unix_get_fd(io));
 }
-- 
2.40.1

RE: [BlueZ,1/2] shared/bap: detach io for source ASEs at QoS when disabling

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1602 bytes --]

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=751683

---Test result---

Test Summary:
CheckPatch                    PASS      1.17 seconds
GitLint                       FAIL      0.92 seconds
BuildEll                      PASS      27.52 seconds
BluezMake                     PASS      912.77 seconds
MakeCheck                     PASS      11.56 seconds
MakeDistcheck                 PASS      158.68 seconds
CheckValgrind                 PASS      259.57 seconds
CheckSmatch                   PASS      348.36 seconds
bluezmakeextell               PASS      104.18 seconds
IncrementalBuild              PASS      1530.25 seconds
ScanBuild                     PASS      1061.78 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,2/2] bap: wait for CIG to become configurable before recreating CIS

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
25: B2 Line has trailing whitespace: "    "
30: B2 Line has trailing whitespace: "    "
33: B2 Line has trailing whitespace: "    "


---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ 1/2] shared/bap: detach io for source ASEs at QoS when disabling

[Luiz Augusto von Dentz]

Hi Pauli,

On Sun, May 28, 2023 at 10:47 AM Pauli Virtanen <pav@iki.fi> wrote:
>
> The Client may terminate a CIS when sink is in QOS and source in
> Disabling states (BAP v1.0.1 Sec 5.6.5).  It may also terminate it when
> Receiver Stop Ready has completed successfully (BAP v1.0.1 Sec 5.6.5.1).
> On successful Receiver Stop Ready the Server shall transition the ASE
> back to QoS state (ASCS v1.0 Sec 5.6).
>
> It appears Samsung Galaxy Buds2 Pro (R510XXUOAWA5) ignores the Receiver
> Stop Ready command if CIS is already disconnected, and then gets stuck
> in disabling state. It works if CIS is disconnected after Receiver Stop
> Ready.
>
> For better compatibility, disconnect CIS only after the source ASE is
> back in the QoS state. This is what we also do with sinks.
>
> Link: https://github.com/bluez/bluez/issues/516
> ---
>  src/shared/bap.c | 20 ++------------------
>  1 file changed, 2 insertions(+), 18 deletions(-)
>
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index f194f466f..16a9cec5b 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -1115,18 +1115,6 @@ static bool match_stream_io(const void *data, const void *user_data)
>         return stream->io == io;
>  }
>
> -static void stream_stop_disabling(void *data, void *user_data)
> -{
> -       struct bt_bap_stream *stream = data;
> -
> -       if (stream->io || stream->ep->state != BT_ASCS_ASE_STATE_DISABLING)
> -               return;
> -
> -       DBG(stream->bap, "stream %p", stream);
> -
> -       bt_bap_stream_stop(stream, NULL, NULL);
> -}
> -
>  static bool bap_stream_io_detach(struct bt_bap_stream *stream)
>  {
>         struct bt_bap_stream *link;
> @@ -1145,9 +1133,6 @@ static bool bap_stream_io_detach(struct bt_bap_stream *stream)
>                 /* Detach link if in QoS state */
>                 if (link->ep->state == BT_ASCS_ASE_STATE_QOS)
>                         bap_stream_io_detach(link);
> -       } else {
> -               /* Links without IO on disabling state shall be stopped. */
> -               queue_foreach(stream->links, stream_stop_disabling, NULL);
>         }
>
>         stream_io_unref(io);
> @@ -1218,7 +1203,6 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream)
>                 bap_stream_update_io_links(stream);
>                 break;
>         case BT_ASCS_ASE_STATE_DISABLING:
> -               bap_stream_io_detach(stream);
>                 break;
>         case BT_ASCS_ASE_STATE_QOS:
>                 if (stream->io && !stream->io->connecting)
> @@ -1252,8 +1236,8 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream)
>                         bt_bap_stream_start(stream, NULL, NULL);
>                 break;
>         case BT_ASCS_ASE_STATE_DISABLING:
> -               if (!bt_bap_stream_get_io(stream))
> -                       bt_bap_stream_stop(stream, NULL, NULL);
> +               /* IO is detached when back in QOS */
> +               bt_bap_stream_stop(stream, NULL, NULL);
>                 break;

Note that doing this way makes our peripheral/server role detach by
itself because it will end up calling stream_stop, so perhaps we need
to add a check if we acting as a client or not, that said if we do it
late don't we risk the server not sending QOS until ISO is dropped? So
perhaps we also need to detect that somehow and drop the ISO socket if
the peripheral stays on DISABLING for too long (e.g. 2 sec) after
stop.

>         }
>
> --
> 2.40.1
>


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 1/2] shared/bap: detach io for source ASEs at QoS when disabling

[Pauli Virtanen]

Hi Luiz,

ti, 2023-05-30 kello 10:35 -0700, Luiz Augusto von Dentz kirjoitti:
> On Sun, May 28, 2023 at 10:47 AM Pauli Virtanen <pav@iki.fi> wrote:
> > 
> > The Client may terminate a CIS when sink is in QOS and source in
> > Disabling states (BAP v1.0.1 Sec 5.6.5).  It may also terminate it when
> > Receiver Stop Ready has completed successfully (BAP v1.0.1 Sec 5.6.5.1).
> > On successful Receiver Stop Ready the Server shall transition the ASE
> > back to QoS state (ASCS v1.0 Sec 5.6).
> > 
> > It appears Samsung Galaxy Buds2 Pro (R510XXUOAWA5) ignores the Receiver
> > Stop Ready command if CIS is already disconnected, and then gets stuck
> > in disabling state. It works if CIS is disconnected after Receiver Stop
> > Ready.
> > 
> > For better compatibility, disconnect CIS only after the source ASE is
> > back in the QoS state. This is what we also do with sinks.
> > 
> > Link: https://github.com/bluez/bluez/issues/516
> > ---
> >  src/shared/bap.c | 20 ++------------------
> >  1 file changed, 2 insertions(+), 18 deletions(-)
> > 
> > diff --git a/src/shared/bap.c b/src/shared/bap.c
> > index f194f466f..16a9cec5b 100644
> > --- a/src/shared/bap.c
> > +++ b/src/shared/bap.c
> > @@ -1115,18 +1115,6 @@ static bool match_stream_io(const void *data, const void *user_data)
> >         return stream->io == io;
> >  }
> > 
> > -static void stream_stop_disabling(void *data, void *user_data)
> > -{
> > -       struct bt_bap_stream *stream = data;
> > -
> > -       if (stream->io || stream->ep->state != BT_ASCS_ASE_STATE_DISABLING)
> > -               return;
> > -
> > -       DBG(stream->bap, "stream %p", stream);
> > -
> > -       bt_bap_stream_stop(stream, NULL, NULL);
> > -}
> > -
> >  static bool bap_stream_io_detach(struct bt_bap_stream *stream)
> >  {
> >         struct bt_bap_stream *link;
> > @@ -1145,9 +1133,6 @@ static bool bap_stream_io_detach(struct bt_bap_stream *stream)
> >                 /* Detach link if in QoS state */
> >                 if (link->ep->state == BT_ASCS_ASE_STATE_QOS)
> >                         bap_stream_io_detach(link);
> > -       } else {
> > -               /* Links without IO on disabling state shall be stopped. */
> > -               queue_foreach(stream->links, stream_stop_disabling, NULL);
> >         }
> > 
> >         stream_io_unref(io);
> > @@ -1218,7 +1203,6 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream)
> >                 bap_stream_update_io_links(stream);
> >                 break;
> >         case BT_ASCS_ASE_STATE_DISABLING:
> > -               bap_stream_io_detach(stream);
> >                 break;
> >         case BT_ASCS_ASE_STATE_QOS:
> >                 if (stream->io && !stream->io->connecting)
> > @@ -1252,8 +1236,8 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream)
> >                         bt_bap_stream_start(stream, NULL, NULL);
> >                 break;
> >         case BT_ASCS_ASE_STATE_DISABLING:
> > -               if (!bt_bap_stream_get_io(stream))
> > -                       bt_bap_stream_stop(stream, NULL, NULL);
> > +               /* IO is detached when back in QOS */
> > +               bt_bap_stream_stop(stream, NULL, NULL);
> >                 break;
> 
> Note that doing this way makes our peripheral/server role detach by
> itself because it will end up calling stream_stop, so perhaps we need
> to add a check if we acting as a client or not, that said if we do it

Ack, as server we shall not transition to stop here. -> For v2

> late don't we risk the server not sending QOS until ISO is dropped? So
> perhaps we also need to detect that somehow and drop the ISO socket if
> the peripheral stays on DISABLING for too long (e.g. 2 sec) after
> stop.

BAP does not appear to require terminating the CIS, either after
entering disabling or after sending stop. My reading of the spec is
that it is explicitly allowed to do it after Stop Ready.

But it's possible some devices misbehave in the opposite way and
require terminating CIS in Disabling and not after it. I can in add
some timeout mechanism for stalled transitions in v2, and use it here
to detach first and then send another Stop Ready.

Relevant parts:

BAP (Sec. 5.6.5):
"""
If a Source ASE is in the Disabling state, and/or if a Sink ASE is in
the QoS Configured state, the Unicast Client or the Unicast Server may
terminate a CIS established for that ASE by following the Connected
Isochronous Stream Terminate procedure defined in Volume 3, Part C,
Section 9.3.15 in [1]. Termination of the CIS is not required.
"""

BAP (Sec. 5.6.5.1):
"""
If the Receiver Stop Ready operation has completed successfully for a
Source ASE, the Unicast Client or the Unicast Server may terminate a
CIS established for that Source ASE by following the Connected
Isochronous Stream Terminate procedure defined in Volume 3, Part C,
Section 9.3.15 in [1]. Termination of the CIS is not required.
"""

ASCS (Sec 5.6):
"""
If the server accepts the requested Receiver Stop Ready operation
parameter values for a Source ASE, the server shall transition the ASE
to the QoS Configured state and write a value of 0x02 (QoS
Configured) to the ASE_State field, and the server shall write the
previously accepted Config QoS operation parameter values to the
matching Additional_ASE_Parameters fields defined in Table 4.5.
"""

-- 
Pauli Virtanen

Re: [PATCH BlueZ 1/2] shared/bap: detach io for source ASEs at QoS when disabling

[Luiz Augusto von Dentz]

Hi Pauli,

On Tue, May 30, 2023 at 10:59 AM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi Luiz,
>
> ti, 2023-05-30 kello 10:35 -0700, Luiz Augusto von Dentz kirjoitti:
> > On Sun, May 28, 2023 at 10:47 AM Pauli Virtanen <pav@iki.fi> wrote:
> > >
> > > The Client may terminate a CIS when sink is in QOS and source in
> > > Disabling states (BAP v1.0.1 Sec 5.6.5).  It may also terminate it when
> > > Receiver Stop Ready has completed successfully (BAP v1.0.1 Sec 5.6.5.1).
> > > On successful Receiver Stop Ready the Server shall transition the ASE
> > > back to QoS state (ASCS v1.0 Sec 5.6).
> > >
> > > It appears Samsung Galaxy Buds2 Pro (R510XXUOAWA5) ignores the Receiver
> > > Stop Ready command if CIS is already disconnected, and then gets stuck
> > > in disabling state. It works if CIS is disconnected after Receiver Stop
> > > Ready.
> > >
> > > For better compatibility, disconnect CIS only after the source ASE is
> > > back in the QoS state. This is what we also do with sinks.
> > >
> > > Link: https://github.com/bluez/bluez/issues/516
> > > ---
> > >  src/shared/bap.c | 20 ++------------------
> > >  1 file changed, 2 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/src/shared/bap.c b/src/shared/bap.c
> > > index f194f466f..16a9cec5b 100644
> > > --- a/src/shared/bap.c
> > > +++ b/src/shared/bap.c
> > > @@ -1115,18 +1115,6 @@ static bool match_stream_io(const void *data, const void *user_data)
> > >         return stream->io == io;
> > >  }
> > >
> > > -static void stream_stop_disabling(void *data, void *user_data)
> > > -{
> > > -       struct bt_bap_stream *stream = data;
> > > -
> > > -       if (stream->io || stream->ep->state != BT_ASCS_ASE_STATE_DISABLING)
> > > -               return;
> > > -
> > > -       DBG(stream->bap, "stream %p", stream);
> > > -
> > > -       bt_bap_stream_stop(stream, NULL, NULL);
> > > -}
> > > -
> > >  static bool bap_stream_io_detach(struct bt_bap_stream *stream)
> > >  {
> > >         struct bt_bap_stream *link;
> > > @@ -1145,9 +1133,6 @@ static bool bap_stream_io_detach(struct bt_bap_stream *stream)
> > >                 /* Detach link if in QoS state */
> > >                 if (link->ep->state == BT_ASCS_ASE_STATE_QOS)
> > >                         bap_stream_io_detach(link);
> > > -       } else {
> > > -               /* Links without IO on disabling state shall be stopped. */
> > > -               queue_foreach(stream->links, stream_stop_disabling, NULL);
> > >         }
> > >
> > >         stream_io_unref(io);
> > > @@ -1218,7 +1203,6 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream)
> > >                 bap_stream_update_io_links(stream);
> > >                 break;
> > >         case BT_ASCS_ASE_STATE_DISABLING:
> > > -               bap_stream_io_detach(stream);
> > >                 break;
> > >         case BT_ASCS_ASE_STATE_QOS:
> > >                 if (stream->io && !stream->io->connecting)
> > > @@ -1252,8 +1236,8 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream)
> > >                         bt_bap_stream_start(stream, NULL, NULL);
> > >                 break;
> > >         case BT_ASCS_ASE_STATE_DISABLING:
> > > -               if (!bt_bap_stream_get_io(stream))
> > > -                       bt_bap_stream_stop(stream, NULL, NULL);
> > > +               /* IO is detached when back in QOS */
> > > +               bt_bap_stream_stop(stream, NULL, NULL);
> > >                 break;
> >
> > Note that doing this way makes our peripheral/server role detach by
> > itself because it will end up calling stream_stop, so perhaps we need
> > to add a check if we acting as a client or not, that said if we do it
>
> Ack, as server we shall not transition to stop here. -> For v2
>
> > late don't we risk the server not sending QOS until ISO is dropped? So
> > perhaps we also need to detect that somehow and drop the ISO socket if
> > the peripheral stays on DISABLING for too long (e.g. 2 sec) after
> > stop.
>
> BAP does not appear to require terminating the CIS, either after
> entering disabling or after sending stop. My reading of the spec is
> that it is explicitly allowed to do it after Stop Ready.
>
> But it's possible some devices misbehave in the opposite way and
> require terminating CIS in Disabling and not after it. I can in add
> some timeout mechanism for stalled transitions in v2, and use it here
> to detach first and then send another Stop Ready.
>
> Relevant parts:
>
> BAP (Sec. 5.6.5):
> """
> If a Source ASE is in the Disabling state, and/or if a Sink ASE is in
> the QoS Configured state, the Unicast Client or the Unicast Server may
> terminate a CIS established for that ASE by following the Connected
> Isochronous Stream Terminate procedure defined in Volume 3, Part C,
> Section 9.3.15 in [1]. Termination of the CIS is not required.
> """
>
> BAP (Sec. 5.6.5.1):
> """
> If the Receiver Stop Ready operation has completed successfully for a
> Source ASE, the Unicast Client or the Unicast Server may terminate a
> CIS established for that Source ASE by following the Connected
> Isochronous Stream Terminate procedure defined in Volume 3, Part C,
> Section 9.3.15 in [1]. Termination of the CIS is not required.
> """
>
> ASCS (Sec 5.6):
> """
> If the server accepts the requested Receiver Stop Ready operation
> parameter values for a Source ASE, the server shall transition the ASE
> to the QoS Configured state and write a value of 0x02 (QoS
> Configured) to the ASE_State field, and the server shall write the
> previously accepted Config QoS operation parameter values to the
> matching Additional_ASE_Parameters fields defined in Table 4.5.
> """

The best option would be to follow the test specification with respect
to Disable tests:

'4.8.6 Unicast Client Initiates Disable Operation

1. The Upper Tester orders the IUT to execute the GATT Write Without
Response sub-procedure
for the ASE Control Point characteristic with the opcode set to 0x05
(Disable) and:
• Number_of_ASEs set to 1
• ASE_ID[0] set using the value from the Initial Condition
2. The Lower Tester sends the IUT a notification of the ASE Control
Point characteristic.
3. If the Lower Tester is in the Audio Source role:
a. The Lower Tester sends the IUT a notification of the ASE
characteristic that corresponds to
the ASE_ID that was set in step 1, with ASE_State set to Disabling.
b. The Upper Tester orders the IUT to execute the GATT Write Without
Response sub-
procedure for the ASE Control Point characteristic with the opcode set
to 0x06 (Receiver
Stop Ready) and:
•
•
Number_of_ASEs set to 1
ASE_ID[0] set using the value from the Initial Condition
4. The Lower Tester sends the IUT a notification of the ASE Control
Point characteristic.
5. The Lower Tester sends the IUT a notification of the ASE
characteristic that corresponds to the
ASE_ID that was set in step 4, with ASE_State set to QoS Configured.'

Unfortunately it doesn't say what behavior it expects for the ISO
transport itself so we need to check how PTS implemented it. In fact
if we look into the server test case there is:

'4.9.10 Unicast Server Initiates Disable While Streaming on Loss of CIS

Pass verdict
The IUT sends a notification of the ASE characteristic with the
ASE_State field set to 0x02 (QoS
Configured).'

I suspect this is valid also for Disabling state, meaning if the CIS
is lost while Disabling the server shall move to QoS state, so the
fact that the Buds is not doing it may be a qualification issue if
that is covered in the test specification in the future, anyway that
is not really our problem and shouldn't stop us to perform the
disconnection after Stop.

-- 
Luiz Augusto von Dentz