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).
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 as client for this device, and since it
shouldn't matter for us in which order we do it, disconnect CIS after
completion of Receiver Stop Ready, instead of immediately in Disabling.
We disconnect also if Receiver Stop Ready fails, given that
disconnecting in Disabled state should be OK.
Link: https://github.com/bluez/bluez/issues/516
---
Notes:
v2: Disconnect when Receiver Stop has completed.
Keep BAP server behavior unchanged.
I don't have access to PTS, so if it's needed to verify what it does,
someone else must do it.
Assuming this device was PTS tested, I'd guess its test case does not
disconnect CIS immediately in Disabling.
src/shared/bap.c | 33 +++++++++++++++------------------
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/src/shared/bap.c b/src/shared/bap.c
index cf5d810bb..13c76afe6 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -1168,18 +1168,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;
@@ -1198,9 +1186,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);
@@ -1244,6 +1229,15 @@ static struct bt_bap *bt_bap_ref_safe(struct bt_bap *bap)
return bt_bap_ref(bap);
}
+static void stream_stop_complete(struct bt_bap_stream *stream, uint8_t code,
+ uint8_t reason, void *user_data)
+{
+ DBG(stream->bap, "stream %p stop 0x%02x 0x%02x", stream, code, reason);
+
+ if (stream->ep->state == BT_ASCS_ASE_STATE_DISABLING)
+ bap_stream_io_detach(stream);
+}
+
static void bap_stream_state_changed(struct bt_bap_stream *stream)
{
struct bt_bap *bap = stream->bap;
@@ -1271,7 +1265,9 @@ 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);
+ /* As client, we detach after Receiver Stop Ready */
+ if (!stream->client)
+ bap_stream_io_detach(stream);
break;
case BT_ASCS_ASE_STATE_QOS:
if (stream->io && !stream->io->connecting)
@@ -1305,8 +1301,9 @@ 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);
+ /* Send Stop Ready, and detach IO after remote replies */
+ if (stream->client)
+ bt_bap_stream_stop(stream, stream_stop_complete, NULL);
break;
}
--
2.41.0
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:
v2: no changes
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 8e2fc1556..d7ce9e038 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -68,6 +68,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;
@@ -525,6 +526,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,
@@ -988,7 +990,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;
@@ -1025,12 +1027,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.ucast.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);
@@ -1047,6 +1089,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.ucast.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.ucast.cig_id;
+
+ DBG("adapter %p ep %p recreate CIG %d", info.adapter, ep, info.cig);
+
+ if (ep->qos.ucast.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)
{
@@ -1059,10 +1143,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.ucast.cig_id))
+ recreate_cig(ep);
return FALSE;
}
@@ -1087,18 +1169,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.ucast.cig_id)) {
+ DBG("ep %p stream %p defer %s wait recreate", ep, stream,
+ defer ? "true" : "false");
ep->recreate = true;
return;
}
@@ -1131,6 +1217,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.41.0
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=761842
---Test result---
Test Summary:
CheckPatch PASS 1.04 seconds
GitLint FAIL 0.88 seconds
BuildEll PASS 26.33 seconds
BluezMake PASS 754.04 seconds
MakeCheck PASS 11.14 seconds
MakeDistcheck PASS 152.44 seconds
CheckValgrind PASS 246.87 seconds
CheckSmatch PASS 331.78 seconds
bluezmakeextell PASS 100.51 seconds
IncrementalBuild PASS 1272.53 seconds
ScanBuild PASS 992.76 seconds
Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,v2,1/2] shared/bap: detach io for source ASEs only after Stop Ready
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: " "
28: B2 Line has trailing whitespace: " "
---
Regards,
Linux Bluetooth
Hello:
This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Sun, 2 Jul 2023 21:43:04 +0300 you 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).
>
> 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.
>
> [...]
Here is the summary with links:
- [BlueZ,v2,1/2] shared/bap: detach io for source ASEs only after Stop Ready
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=7b10e72de6f4
- [BlueZ,v2,2/2] bap: wait for CIG to become configurable before recreating CIS
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=8c3170190d6f
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Hi Pauli,
On Sun, Sep 10, 2023 at 9:31 AM Pauli Virtanen <[email protected]> wrote:
>
> Hi Luiz,
>
> to, 2023-08-31 kello 14:04 -0700, Luiz Augusto von Dentz kirjoitti:
> > On Wed, Jul 5, 2023 at 11:18 AM <[email protected]> wrote:
> > >
> > > Hello:
> > >
> > > This series was applied to bluetooth/bluez.git (master)
> > > by Luiz Augusto von Dentz <[email protected]>:
> > >
> > > On Sun, 2 Jul 2023 21:43:04 +0300 you 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).
> > > >
> > > > 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.
> > > >
> > > > [...]
> > >
> > > Here is the summary with links:
> > > - [BlueZ,v2,1/2] shared/bap: detach io for source ASEs only after Stop Ready
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=7b10e72de6f4
> > > - [BlueZ,v2,2/2] bap: wait for CIG to become configurable before recreating CIS
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=8c3170190d6f
> > >
> > > You are awesome, thank you!
> > > --
> > > Deet-doot-dot, I am a bot.
> > > https://korg.docs.kernel.org/patchwork/pwbot.html
> >
> > Looks like this one introduces a problem when using the emulator:
> >
> > https://gist.github.com/Vudentz/5c7ef940fc97b054227559dcd47b99f7?permalink_comment_id=4677775#gistcomment-4677775
> >
> > If I try to release then acquire then it won't trigger recreate logic,
> > I suspect this is due to bap_io_disconnected being called ahead of
> > states changes.
>
> Sorry for the delay. Was this fixed by d06b912df5ab ("bap: Fix not
> always calling bap_io_close on disconnect")? If not, I'll try to
> reproduce.
Yeah, that should have been fixed already.
> --
> Pauli Virtanen
--
Luiz Augusto von Dentz