[PATCH BlueZ 0/2] Remove POLLOUT check before bcast defer accept

[PATCH BlueZ 0/2] Remove POLLOUT check before bcast defer accept

[Iulia Tanasescu]

This patch removes the POLLOUT check on a broadcast child socket before
calling "read" to accept defer setup. The check was inherited from
unicast, but it is unnecessary for broadcast, since the POLLOUT event
is never generated on a broadcast child socket.

Iulia Tanasescu (2):
  btio: Remove POLLOUT check from bt_io_bcast_accept
  iso-tester: Separate iso_defer_accept into dedicated functions

 btio/btio.c        | 18 +++---------------
 tools/iso-tester.c | 45 +++++++++++++++++++++++++++++++++++----------
 2 files changed, 38 insertions(+), 25 deletions(-)


base-commit: 41d6c4e1c92fc6e0757b0f71ca5062671ff55235
-- 
2.39.2

[PATCH BlueZ 1/2] btio: Remove POLLOUT check from bt_io_bcast_accept

[Iulia Tanasescu]

For the Broadcast Sink defer setup scenario, there is no need to check
for the POLLOUT event on the PA sync socket before calling "read" to
issue the Create BIG sync command. This check has been inherited from
unicast, but it is unnecessary for broadcast, since currently after
accept and read, the event to signal BIG sync established is G_IO_IN.
---
 btio/btio.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/btio/btio.c b/btio/btio.c
index d30cfcac7..2d277e409 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -5,7 +5,7 @@
  *
  *  Copyright (C) 2009-2010  Marcel Holtmann <marcel@holtmann.org>
  *  Copyright (C) 2009-2010  Nokia Corporation
- *  Copyright 2023 NXP
+ *  Copyright 2023-2024 NXP
  *
  *
  */
@@ -1800,7 +1800,6 @@ gboolean bt_io_bcast_accept(GIOChannel *io, BtIOConnect connect,
 {
 	int sock;
 	char c;
-	struct pollfd pfd;
 	va_list args;
 	struct sockaddr_iso *addr = NULL;
 	uint8_t bc_num_bis = 0;
@@ -1857,22 +1856,11 @@ gboolean bt_io_bcast_accept(GIOChannel *io, BtIOConnect connect,
 			return FALSE;
 	}
 
-	memset(&pfd, 0, sizeof(pfd));
-	pfd.fd = sock;
-	pfd.events = POLLOUT;
-
-	if (poll(&pfd, 1, 0) < 0) {
-		ERROR_FAILED(err, "poll", errno);
+	if (read(sock, &c, 1) < 0) {
+		ERROR_FAILED(err, "read", errno);
 		return FALSE;
 	}
 
-	if (!(pfd.revents & POLLOUT)) {
-		if (read(sock, &c, 1) < 0) {
-			ERROR_FAILED(err, "read", errno);
-			return FALSE;
-		}
-	}
-
 	server_add(io, connect, NULL, user_data, destroy);
 
 	return TRUE;
-- 
2.39.2

RE: Remove POLLOUT check before bcast defer accept

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 949 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=838961

---Test result---

Test Summary:
CheckPatch                    PASS      0.90 seconds
GitLint                       PASS      0.61 seconds
BuildEll                      PASS      24.41 seconds
BluezMake                     PASS      1708.45 seconds
MakeCheck                     PASS      12.78 seconds
MakeDistcheck                 PASS      180.35 seconds
CheckValgrind                 PASS      245.99 seconds
CheckSmatch                   PASS      362.14 seconds
bluezmakeextell               PASS      120.19 seconds
IncrementalBuild              PASS      3072.32 seconds
ScanBuild                     PASS      1022.92 seconds



---
Regards,
Linux Bluetooth

[PATCH BlueZ 2/2] iso-tester: Separate iso_defer_accept into dedicated functions

[Iulia Tanasescu]

This separates the iso_defer_accept function into dedicated ones for
unicast/broadcast, since the flow is different for each scenario:
For unicast, POLLOUT is checked on socket before calling read and
adding a G_IO_OUT watch. Checking for POLLOUT is not necessary for
broadcast, since currently this event is never generated on the
child socket. Instead, G_IO_IN is generated after BIG sync is
established and a BIS socket is ready to be accepted.
---
 tools/iso-tester.c | 45 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/tools/iso-tester.c b/tools/iso-tester.c
index 1864b9e9d..60afef301 100644
--- a/tools/iso-tester.c
+++ b/tools/iso-tester.c
@@ -4,7 +4,7 @@
  *  BlueZ - Bluetooth protocol stack for Linux
  *
  *  Copyright (C) 2022  Intel Corporation.
- *  Copyright 2023 NXP
+ *  Copyright 2023-2024 NXP
  *
  */
 
@@ -489,6 +489,8 @@ struct iso_client_data {
 	bool pa_bind;
 };
 
+typedef bool (*iso_defer_accept_t)(struct test_data *data, GIOChannel *io);
+
 static void mgmt_debug(const char *str, void *user_data)
 {
 	const char *prefix = user_data;
@@ -2582,11 +2584,10 @@ static void setup_listen(struct test_data *data, uint8_t num, GIOFunc func)
 	}
 }
 
-static bool iso_defer_accept(struct test_data *data, GIOChannel *io)
+static bool iso_defer_accept_bcast(struct test_data *data, GIOChannel *io)
 {
 	int sk;
 	char c;
-	struct pollfd pfd;
 	const struct iso_client_data *isodata = data->test_data;
 	struct sockaddr_iso *addr = NULL;
 
@@ -2610,6 +2611,31 @@ static bool iso_defer_accept(struct test_data *data, GIOChannel *io)
 		free(addr);
 	}
 
+	if (read(sk, &c, 1) < 0) {
+		tester_warn("read: %s (%d)", strerror(errno), errno);
+		return false;
+	}
+
+	tester_print("Accept deferred setup");
+
+	data->io_queue = queue_new();
+	if (data->io_queue)
+		queue_push_tail(data->io_queue, io);
+
+	data->io_id[0] = g_io_add_watch(io, G_IO_IN,
+				iso_accept_cb, NULL);
+
+	return true;
+}
+
+static bool iso_defer_accept_ucast(struct test_data *data, GIOChannel *io)
+{
+	int sk;
+	char c;
+	struct pollfd pfd;
+
+	sk = g_io_channel_unix_get_fd(io);
+
 	memset(&pfd, 0, sizeof(pfd));
 	pfd.fd = sk;
 	pfd.events = POLLOUT;
@@ -2632,12 +2658,8 @@ static bool iso_defer_accept(struct test_data *data, GIOChannel *io)
 	if (data->io_queue)
 		queue_push_tail(data->io_queue, io);
 
-	if (isodata->bcast)
-		data->io_id[0] = g_io_add_watch(io, G_IO_IN,
-					iso_accept_cb, NULL);
-	else
-		data->io_id[0] = g_io_add_watch(io, G_IO_OUT,
-					iso_connect_cb, NULL);
+	data->io_id[0] = g_io_add_watch(io, G_IO_OUT,
+				iso_connect_cb, NULL);
 
 	return true;
 }
@@ -2648,6 +2670,9 @@ static gboolean iso_accept_cb(GIOChannel *io, GIOCondition cond,
 	struct test_data *data = tester_get_data();
 	const struct iso_client_data *isodata = data->test_data;
 	int sk, new_sk;
+	iso_defer_accept_t iso_accept = isodata->bcast ?
+						iso_defer_accept_bcast :
+						iso_defer_accept_ucast;
 
 	data->io_id[0] = 0;
 
@@ -2676,7 +2701,7 @@ static gboolean iso_accept_cb(GIOChannel *io, GIOCondition cond,
 				return false;
 		}
 
-		if (!iso_defer_accept(data, io)) {
+		if (!iso_accept(data, io)) {
 			tester_warn("Unable to accept deferred setup");
 			tester_test_failed();
 		}
-- 
2.39.2

Re: [PATCH BlueZ 2/2] iso-tester: Separate iso_defer_accept into dedicated functions

[Luiz Augusto von Dentz]

Hi Iulia,

On Wed, Mar 27, 2024 at 12:10 PM Iulia Tanasescu
<iulia.tanasescu@nxp.com> wrote:
>
> This separates the iso_defer_accept function into dedicated ones for
> unicast/broadcast, since the flow is different for each scenario:
> For unicast, POLLOUT is checked on socket before calling read and
> adding a G_IO_OUT watch. Checking for POLLOUT is not necessary for
> broadcast, since currently this event is never generated on the
> child socket. Instead, G_IO_IN is generated after BIG sync is
> established and a BIS socket is ready to be accepted.

@Pauli Virtanen Is this inline with TX Timestamping though? Or that
only cares for POLLERR?

Also it would be great to know what are the plans of PW with respect
to broadcast, I was thinking something like this:

1. Broadcast Source: Have some dedicated card that can be configured
via configuration file or a dedicated app that performs the
configuration which can then select what streams shall be broadcasted,
since broadcasting things like system audio notifications probably
doesn't make much sense.
2. Broadcast Sink: This shall be a little bit easier since we now
enumerate the BASE found over the air, so it should work very closely
to unicast, but perhaps you may also want to start a scan session
while the card selection dialog is open so the user don't need to use
yet another app to trigger it, or perhaps this should really be done
at Bluetooth system settings that way PW don't show any Broadcast Sink
until the user selects it at Bluetooth APP, this way we don't clutter
PW interface with unsynchronized Broadcast Sinks. Note that in the
future we are going to remove the step of creating a MediaEndpoint for
Sinks since they are already configured over the air they shall appear
as MediaTransports ready to be Acquired.

> ---
>  tools/iso-tester.c | 45 +++++++++++++++++++++++++++++++++++----------
>  1 file changed, 35 insertions(+), 10 deletions(-)
>
> diff --git a/tools/iso-tester.c b/tools/iso-tester.c
> index 1864b9e9d..60afef301 100644
> --- a/tools/iso-tester.c
> +++ b/tools/iso-tester.c
> @@ -4,7 +4,7 @@
>   *  BlueZ - Bluetooth protocol stack for Linux
>   *
>   *  Copyright (C) 2022  Intel Corporation.
> - *  Copyright 2023 NXP
> + *  Copyright 2023-2024 NXP
>   *
>   */
>
> @@ -489,6 +489,8 @@ struct iso_client_data {
>         bool pa_bind;
>  };
>
> +typedef bool (*iso_defer_accept_t)(struct test_data *data, GIOChannel *io);
> +
>  static void mgmt_debug(const char *str, void *user_data)
>  {
>         const char *prefix = user_data;
> @@ -2582,11 +2584,10 @@ static void setup_listen(struct test_data *data, uint8_t num, GIOFunc func)
>         }
>  }
>
> -static bool iso_defer_accept(struct test_data *data, GIOChannel *io)
> +static bool iso_defer_accept_bcast(struct test_data *data, GIOChannel *io)
>  {
>         int sk;
>         char c;
> -       struct pollfd pfd;
>         const struct iso_client_data *isodata = data->test_data;
>         struct sockaddr_iso *addr = NULL;
>
> @@ -2610,6 +2611,31 @@ static bool iso_defer_accept(struct test_data *data, GIOChannel *io)
>                 free(addr);
>         }
>
> +       if (read(sk, &c, 1) < 0) {
> +               tester_warn("read: %s (%d)", strerror(errno), errno);
> +               return false;
> +       }
> +
> +       tester_print("Accept deferred setup");
> +
> +       data->io_queue = queue_new();
> +       if (data->io_queue)
> +               queue_push_tail(data->io_queue, io);
> +
> +       data->io_id[0] = g_io_add_watch(io, G_IO_IN,
> +                               iso_accept_cb, NULL);
> +
> +       return true;
> +}
> +
> +static bool iso_defer_accept_ucast(struct test_data *data, GIOChannel *io)
> +{
> +       int sk;
> +       char c;
> +       struct pollfd pfd;
> +
> +       sk = g_io_channel_unix_get_fd(io);
> +
>         memset(&pfd, 0, sizeof(pfd));
>         pfd.fd = sk;
>         pfd.events = POLLOUT;
> @@ -2632,12 +2658,8 @@ static bool iso_defer_accept(struct test_data *data, GIOChannel *io)
>         if (data->io_queue)
>                 queue_push_tail(data->io_queue, io);
>
> -       if (isodata->bcast)
> -               data->io_id[0] = g_io_add_watch(io, G_IO_IN,
> -                                       iso_accept_cb, NULL);
> -       else
> -               data->io_id[0] = g_io_add_watch(io, G_IO_OUT,
> -                                       iso_connect_cb, NULL);
> +       data->io_id[0] = g_io_add_watch(io, G_IO_OUT,
> +                               iso_connect_cb, NULL);
>
>         return true;
>  }
> @@ -2648,6 +2670,9 @@ static gboolean iso_accept_cb(GIOChannel *io, GIOCondition cond,
>         struct test_data *data = tester_get_data();
>         const struct iso_client_data *isodata = data->test_data;
>         int sk, new_sk;
> +       iso_defer_accept_t iso_accept = isodata->bcast ?
> +                                               iso_defer_accept_bcast :
> +                                               iso_defer_accept_ucast;
>
>         data->io_id[0] = 0;
>
> @@ -2676,7 +2701,7 @@ static gboolean iso_accept_cb(GIOChannel *io, GIOCondition cond,
>                                 return false;
>                 }
>
> -               if (!iso_defer_accept(data, io)) {
> +               if (!iso_accept(data, io)) {
>                         tester_warn("Unable to accept deferred setup");
>                         tester_test_failed();
>                 }
> --
> 2.39.2
>


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 2/2] iso-tester: Separate iso_defer_accept into dedicated functions

[Pauli Virtanen]

Hi Luiz & al.,

ke, 2024-03-27 kello 12:50 -0400, Luiz Augusto von Dentz kirjoitti:
> On Wed, Mar 27, 2024 at 12:10 PM Iulia Tanasescu
> <iulia.tanasescu@nxp.com> wrote:
> > 
> > This separates the iso_defer_accept function into dedicated ones for
> > unicast/broadcast, since the flow is different for each scenario:
> > For unicast, POLLOUT is checked on socket before calling read and
> > adding a G_IO_OUT watch. Checking for POLLOUT is not necessary for
> > broadcast, since currently this event is never generated on the
> > child socket. Instead, G_IO_IN is generated after BIG sync is
> > established and a BIS socket is ready to be accepted.
> 
> @Pauli Virtanen Is this inline with TX Timestamping though? Or that
> only cares for POLLERR?

AFAICS this won't interact with TX timestamping. The timestamping only
cares about MSG_ERRQUEUE which is not touched here, and also the setup
here seems to concern what happens before user sends payloads, so
before there is going to be any timestamps.

> Also it would be great to know what are the plans of PW with respect
> to broadcast, I was thinking something like this:
> 
> 1. Broadcast Source: Have some dedicated card that can be configured
> via configuration file or a dedicated app that performs the
> configuration which can then select what streams shall be broadcasted,
> since broadcasting things like system audio notifications probably
> doesn't make much sense.
> 2. Broadcast Sink: This shall be a little bit easier since we now
> enumerate the BASE found over the air, so it should work very closely
> to unicast, but perhaps you may also want to start a scan session
> while the card selection dialog is open so the user don't need to use
> yet another app to trigger it, or perhaps this should really be done
> at Bluetooth system settings that way PW don't show any Broadcast Sink
> until the user selects it at Bluetooth APP, this way we don't clutter
> PW interface with unsynchronized Broadcast Sinks. Note that in the
> future we are going to remove the step of creating a MediaEndpoint for
> Sinks since they are already configured over the air they shall appear
> as MediaTransports ready to be Acquired.

I've been waiting for the BlueZ side of it to settle down a bit. (I
note the BlueZ broadcast API is not documented, would be great if the
.rst docs would be updated too in the patches.) It would also be great
if there is some way to test all this locally, we've merged patches
from Silviu for broadcast sink, but I have myself only looked that it
should be OK in theory.


For Broadcast Source:

The plan here would be to add first some support that we can create and
destroy broadcast source MediaEndpoints on the fly, and then wire it up
to the runtime setting system we have, so user can create and remove
endpoints without having to restart daemons to reload configs.

Generally they would appear to user applications in similar way as
network audio sinks, there's established way how all this goes.

The fact that it has to be via the same DBus connection as the rest is
a bit inconvenient, but can be dealt with.

Ideally the BlueZ DBus API would be such that there could also be a
dedicated "broadcast" application that can send streams, without
needing to interact with the sound server, and also allowing a sound
server to be running its own broadcast sources if any simultaneously.


For Broadcast Sink:

We are going to expose whatever we see over the DBus API to the user as
audio sources, which can be connected where they want to, and will be
acquired when connected.

For controlling the scanning of the broadcast streams, and which
streams are going to be synced to: It feels a bit orthogonal to the job
of a sound server. We could in principle do it. The BT Controller would
appear a sound card object, and it would have additional properties
that would list the available streams, and one poke e.g. with
Pipewire's command line tools to sync to specific streams.

This would be inventing an API to do something that might in principle
also be DBus API in BlueZ.

I guess a question for both source and sink is whether the sound server
should do the whole thing, or only be responsible for the routing of
audio, with things like configuring BASE or selecting which streams to
sync being an orthogonal DBus API (which could then be controlled by
the sound server, or by a dedicated app which could then either
implement its own audio transports or leave it the audio transport part
to the sound server).

> 
> > ---
> >  tools/iso-tester.c | 45 +++++++++++++++++++++++++++++++++++----------
> >  1 file changed, 35 insertions(+), 10 deletions(-)
> > 
> > diff --git a/tools/iso-tester.c b/tools/iso-tester.c
> > index 1864b9e9d..60afef301 100644
> > --- a/tools/iso-tester.c
> > +++ b/tools/iso-tester.c
> > @@ -4,7 +4,7 @@
> >   *  BlueZ - Bluetooth protocol stack for Linux
> >   *
> >   *  Copyright (C) 2022  Intel Corporation.
> > - *  Copyright 2023 NXP
> > + *  Copyright 2023-2024 NXP
> >   *
> >   */
> > 
> > @@ -489,6 +489,8 @@ struct iso_client_data {
> >         bool pa_bind;
> >  };
> > 
> > +typedef bool (*iso_defer_accept_t)(struct test_data *data, GIOChannel *io);
> > +
> >  static void mgmt_debug(const char *str, void *user_data)
> >  {
> >         const char *prefix = user_data;
> > @@ -2582,11 +2584,10 @@ static void setup_listen(struct test_data *data, uint8_t num, GIOFunc func)
> >         }
> >  }
> > 
> > -static bool iso_defer_accept(struct test_data *data, GIOChannel *io)
> > +static bool iso_defer_accept_bcast(struct test_data *data, GIOChannel *io)
> >  {
> >         int sk;
> >         char c;
> > -       struct pollfd pfd;
> >         const struct iso_client_data *isodata = data->test_data;
> >         struct sockaddr_iso *addr = NULL;
> > 
> > @@ -2610,6 +2611,31 @@ static bool iso_defer_accept(struct test_data *data, GIOChannel *io)
> >                 free(addr);
> >         }
> > 
> > +       if (read(sk, &c, 1) < 0) {
> > +               tester_warn("read: %s (%d)", strerror(errno), errno);
> > +               return false;
> > +       }
> > +
> > +       tester_print("Accept deferred setup");
> > +
> > +       data->io_queue = queue_new();
> > +       if (data->io_queue)
> > +               queue_push_tail(data->io_queue, io);
> > +
> > +       data->io_id[0] = g_io_add_watch(io, G_IO_IN,
> > +                               iso_accept_cb, NULL);
> > +
> > +       return true;
> > +}
> > +
> > +static bool iso_defer_accept_ucast(struct test_data *data, GIOChannel *io)
> > +{
> > +       int sk;
> > +       char c;
> > +       struct pollfd pfd;
> > +
> > +       sk = g_io_channel_unix_get_fd(io);
> > +
> >         memset(&pfd, 0, sizeof(pfd));
> >         pfd.fd = sk;
> >         pfd.events = POLLOUT;
> > @@ -2632,12 +2658,8 @@ static bool iso_defer_accept(struct test_data *data, GIOChannel *io)
> >         if (data->io_queue)
> >                 queue_push_tail(data->io_queue, io);
> > 
> > -       if (isodata->bcast)
> > -               data->io_id[0] = g_io_add_watch(io, G_IO_IN,
> > -                                       iso_accept_cb, NULL);
> > -       else
> > -               data->io_id[0] = g_io_add_watch(io, G_IO_OUT,
> > -                                       iso_connect_cb, NULL);
> > +       data->io_id[0] = g_io_add_watch(io, G_IO_OUT,
> > +                               iso_connect_cb, NULL);
> > 
> >         return true;
> >  }
> > @@ -2648,6 +2670,9 @@ static gboolean iso_accept_cb(GIOChannel *io, GIOCondition cond,
> >         struct test_data *data = tester_get_data();
> >         const struct iso_client_data *isodata = data->test_data;
> >         int sk, new_sk;
> > +       iso_defer_accept_t iso_accept = isodata->bcast ?
> > +                                               iso_defer_accept_bcast :
> > +                                               iso_defer_accept_ucast;
> > 
> >         data->io_id[0] = 0;
> > 
> > @@ -2676,7 +2701,7 @@ static gboolean iso_accept_cb(GIOChannel *io, GIOCondition cond,
> >                                 return false;
> >                 }
> > 
> > -               if (!iso_defer_accept(data, io)) {
> > +               if (!iso_accept(data, io)) {
> >                         tester_warn("Unable to accept deferred setup");
> >                         tester_test_failed();
> >                 }
> > --
> > 2.39.2
> > 
> 
> 

-- 
Pauli Virtanen

Re: [PATCH BlueZ 2/2] iso-tester: Separate iso_defer_accept into dedicated functions

[Iulia Tanasescu]

Hi Luiz, Pauli

> -----Original Message-----
> From: Pauli Virtanen <pav@iki.fi>
> Sent: Wednesday, March 27, 2024 7:48 PM
> To: Luiz Augusto von Dentz <luiz.dentz@gmail.com>; Iulia Tanasescu
> <iulia.tanasescu@nxp.com>
> Cc: linux-bluetooth@vger.kernel.org; Claudia Cristina Draghicescu
> <claudia.rosu@nxp.com>; Mihai-Octavian Urzica <mihai-
> octavian.urzica@nxp.com>; Silviu Florian Barbulescu
> <silviu.barbulescu@nxp.com>; Vlad Pruteanu <vlad.pruteanu@nxp.com>;
> Andrei Istodorescu <andrei.istodorescu@nxp.com>
> Subject: [EXT] Re: [PATCH BlueZ 2/2] iso-tester: Separate iso_defer_accept into
> dedicated functions
> 
> Hi Luiz & al.,
> 
> ke, 2024-03-27 kello 12:50 -0400, Luiz Augusto von Dentz kirjoitti:
> > On Wed, Mar 27, 2024 at 12:10 PM Iulia Tanasescu
> > <iulia.tanasescu@nxp.com> wrote:
> > >
> > > This separates the iso_defer_accept function into dedicated ones for
> > > unicast/broadcast, since the flow is different for each scenario:
> > > For unicast, POLLOUT is checked on socket before calling read and
> > > adding a G_IO_OUT watch. Checking for POLLOUT is not necessary for
> > > broadcast, since currently this event is never generated on the
> > > child socket. Instead, G_IO_IN is generated after BIG sync is
> > > established and a BIS socket is ready to be accepted.
> >
> > @Pauli Virtanen Is this inline with TX Timestamping though? Or that
> > only cares for POLLERR?
> 
> AFAICS this won't interact with TX timestamping. The timestamping only
> cares about MSG_ERRQUEUE which is not touched here, and also the setup
> here seems to concern what happens before user sends payloads, so
> before there is going to be any timestamps.
> 

This is right.

To better explain, I decided to remove the POLLOUT check at broadcast
defer accept because it is unnecessary with the current kernel
implementation (POLLOUT has no part in broadcast receiver setup and
so the decision to accept it should not be done based on this condition).

I am actually working on a kernel update on the broadcast receiver side
that would make use of POLLOUT:

Currently, a broadcast listening socket with defer setup is woken up after
a BIGInfo report is received:

https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/hci_event.c#n7084

If a Broadcast Source only has PA enabled but does not have an active BIG,
a Sink will be able to synchronize to the PA and receive PA reports, but
no BIGInfo reports will be generated. In this case, the userspace will
never be woken up to enumerate BASE from PA reports.

This can be fixed by allocating the PA sync hcon directly on PA sync
established event, and once the BASE has been extracted from PA reports,
the userspace can be woken up to accept the child socket and get BASE.

However, the userspace needs to be informed if BIGInfo reports are also
generated - I was thinking by generating POLLOUT on child socket. This
will not work with the current BlueZ implementation where deferred setup
is accepted only if POLLOUT is not detected on child socket.

> > Also it would be great to know what are the plans of PW with respect
> > to broadcast, I was thinking something like this:
> >
> > 1. Broadcast Source: Have some dedicated card that can be configured
> > via configuration file or a dedicated app that performs the
> > configuration which can then select what streams shall be broadcasted,
> > since broadcasting things like system audio notifications probably
> > doesn't make much sense.
> > 2. Broadcast Sink: This shall be a little bit easier since we now
> > enumerate the BASE found over the air, so it should work very closely
> > to unicast, but perhaps you may also want to start a scan session
> > while the card selection dialog is open so the user don't need to use
> > yet another app to trigger it, or perhaps this should really be done
> > at Bluetooth system settings that way PW don't show any Broadcast Sink
> > until the user selects it at Bluetooth APP, this way we don't clutter
> > PW interface with unsynchronized Broadcast Sinks. Note that in the
> > future we are going to remove the step of creating a MediaEndpoint for
> > Sinks since they are already configured over the air they shall appear
> > as MediaTransports ready to be Acquired.
> 
> I've been waiting for the BlueZ side of it to settle down a bit. (I
> note the BlueZ broadcast API is not documented, would be great if the
> .rst docs would be updated too in the patches.) It would also be great
> if there is some way to test all this locally, we've merged patches
> from Silviu for broadcast sink, but I have myself only looked that it
> should be OK in theory.
> 
> 
> For Broadcast Source:
> 
> The plan here would be to add first some support that we can create and
> destroy broadcast source MediaEndpoints on the fly, and then wire it up
> to the runtime setting system we have, so user can create and remove
> endpoints without having to restart daemons to reload configs.
> 
> Generally they would appear to user applications in similar way as
> network audio sinks, there's established way how all this goes.
> 
> The fact that it has to be via the same DBus connection as the rest is
> a bit inconvenient, but can be dealt with.
> 
> Ideally the BlueZ DBus API would be such that there could also be a
> dedicated "broadcast" application that can send streams, without
> needing to interact with the sound server, and also allowing a sound
> server to be running its own broadcast sources if any simultaneously.
> 
> 
> For Broadcast Sink:
> 
> We are going to expose whatever we see over the DBus API to the user as
> audio sources, which can be connected where they want to, and will be
> acquired when connected.
> 
> For controlling the scanning of the broadcast streams, and which
> streams are going to be synced to: It feels a bit orthogonal to the job
> of a sound server. We could in principle do it. The BT Controller would
> appear a sound card object, and it would have additional properties
> that would list the available streams, and one poke e.g. with
> Pipewire's command line tools to sync to specific streams.
> 
> This would be inventing an API to do something that might in principle
> also be DBus API in BlueZ.
> 
> I guess a question for both source and sink is whether the sound server
> should do the whole thing, or only be responsible for the routing of
> audio, with things like configuring BASE or selecting which streams to
> sync being an orthogonal DBus API (which could then be controlled by
> the sound server, or by a dedicated app which could then either
> implement its own audio transports or leave it the audio transport part
> to the sound server).
> 
> >
> > > ---
> > >  tools/iso-tester.c | 45 +++++++++++++++++++++++++++++++++++----------
> > >  1 file changed, 35 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/tools/iso-tester.c b/tools/iso-tester.c
> > > index 1864b9e9d..60afef301 100644
> > > --- a/tools/iso-tester.c
> > > +++ b/tools/iso-tester.c
> > > @@ -4,7 +4,7 @@
> > >   *  BlueZ - Bluetooth protocol stack for Linux
> > >   *
> > >   *  Copyright (C) 2022  Intel Corporation.
> > > - *  Copyright 2023 NXP
> > > + *  Copyright 2023-2024 NXP
> > >   *
> > >   */
> > >
> > > @@ -489,6 +489,8 @@ struct iso_client_data {
> > >         bool pa_bind;
> > >  };
> > >
> > > +typedef bool (*iso_defer_accept_t)(struct test_data *data, GIOChannel *io);
> > > +
> > >  static void mgmt_debug(const char *str, void *user_data)
> > >  {
> > >         const char *prefix = user_data;
> > > @@ -2582,11 +2584,10 @@ static void setup_listen(struct test_data *data,
> uint8_t num, GIOFunc func)
> > >         }
> > >  }
> > >
> > > -static bool iso_defer_accept(struct test_data *data, GIOChannel *io)
> > > +static bool iso_defer_accept_bcast(struct test_data *data, GIOChannel *io)
> > >  {
> > >         int sk;
> > >         char c;
> > > -       struct pollfd pfd;
> > >         const struct iso_client_data *isodata = data->test_data;
> > >         struct sockaddr_iso *addr = NULL;
> > >
> > > @@ -2610,6 +2611,31 @@ static bool iso_defer_accept(struct test_data
> *data, GIOChannel *io)
> > >                 free(addr);
> > >         }
> > >
> > > +       if (read(sk, &c, 1) < 0) {
> > > +               tester_warn("read: %s (%d)", strerror(errno), errno);
> > > +               return false;
> > > +       }
> > > +
> > > +       tester_print("Accept deferred setup");
> > > +
> > > +       data->io_queue = queue_new();
> > > +       if (data->io_queue)
> > > +               queue_push_tail(data->io_queue, io);
> > > +
> > > +       data->io_id[0] = g_io_add_watch(io, G_IO_IN,
> > > +                               iso_accept_cb, NULL);
> > > +
> > > +       return true;
> > > +}
> > > +
> > > +static bool iso_defer_accept_ucast(struct test_data *data, GIOChannel *io)
> > > +{
> > > +       int sk;
> > > +       char c;
> > > +       struct pollfd pfd;
> > > +
> > > +       sk = g_io_channel_unix_get_fd(io);
> > > +
> > >         memset(&pfd, 0, sizeof(pfd));
> > >         pfd.fd = sk;
> > >         pfd.events = POLLOUT;
> > > @@ -2632,12 +2658,8 @@ static bool iso_defer_accept(struct test_data
> *data, GIOChannel *io)
> > >         if (data->io_queue)
> > >                 queue_push_tail(data->io_queue, io);
> > >
> > > -       if (isodata->bcast)
> > > -               data->io_id[0] = g_io_add_watch(io, G_IO_IN,
> > > -                                       iso_accept_cb, NULL);
> > > -       else
> > > -               data->io_id[0] = g_io_add_watch(io, G_IO_OUT,
> > > -                                       iso_connect_cb, NULL);
> > > +       data->io_id[0] = g_io_add_watch(io, G_IO_OUT,
> > > +                               iso_connect_cb, NULL);
> > >
> > >         return true;
> > >  }
> > > @@ -2648,6 +2670,9 @@ static gboolean iso_accept_cb(GIOChannel *io,
> GIOCondition cond,
> > >         struct test_data *data = tester_get_data();
> > >         const struct iso_client_data *isodata = data->test_data;
> > >         int sk, new_sk;
> > > +       iso_defer_accept_t iso_accept = isodata->bcast ?
> > > +                                               iso_defer_accept_bcast :
> > > +                                               iso_defer_accept_ucast;
> > >
> > >         data->io_id[0] = 0;
> > >
> > > @@ -2676,7 +2701,7 @@ static gboolean iso_accept_cb(GIOChannel *io,
> GIOCondition cond,
> > >                                 return false;
> > >                 }
> > >
> > > -               if (!iso_defer_accept(data, io)) {
> > > +               if (!iso_accept(data, io)) {
> > >                         tester_warn("Unable to accept deferred setup");
> > >                         tester_test_failed();
> > >                 }
> > > --
> > > 2.39.2
> > >
> >
> >
> 
> --
> Pauli Virtanen

Regards,
Iulia

Re: [PATCH BlueZ 0/2] Remove POLLOUT check before bcast defer accept

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Wed, 27 Mar 2024 18:10:17 +0200 you wrote:
> This patch removes the POLLOUT check on a broadcast child socket before
> calling "read" to accept defer setup. The check was inherited from
> unicast, but it is unnecessary for broadcast, since the POLLOUT event
> is never generated on a broadcast child socket.
> 
> Iulia Tanasescu (2):
>   btio: Remove POLLOUT check from bt_io_bcast_accept
>   iso-tester: Separate iso_defer_accept into dedicated functions
> 
> [...]

Here is the summary with links:
  - [BlueZ,1/2] btio: Remove POLLOUT check from bt_io_bcast_accept
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f4c40dc4107e
  - [BlueZ,2/2] iso-tester: Separate iso_defer_accept into dedicated functions
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=3403f65e266a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html