[PATCH BlueZ 0/3] Add transport.select method

[PATCH BlueZ 0/3] Add transport.select method

[Vlad Pruteanu]

This series adds a new "select" method that can be called by
the user on the broadcast sink side, to select the stream for
which synchronization is desired. This is achieved by changing
the states flow so that transport are not set to pending automatically
anymore. Instead, the transport must first be selected by the user,
which will update it's state from idle to pending. Any pending
transport will be acquired by PipeWire.

Furthermore, this method will be used for setting the broadcast code
of a stream on the sink side. If the encryption flag is set, the
transport.select call in bluetoothctl will prompt the user to enter
the code

Vlad Pruteanu (3):
  transport: Add "select" method
  client/player: Expose transport "select" method to the user
  transport: Broadcast sink: wait for user to select transport

 client/player.c            | 52 ++++++++++++++++++++++++++++++++++++++
 profiles/audio/transport.c | 29 ++++++++++++++++++---
 2 files changed, 77 insertions(+), 4 deletions(-)

-- 
2.40.1

[PATCH BlueZ 1/3] transport: Add "select" method

[Vlad Pruteanu]

This adds the "select" method for Broadcast transports. It's role
is to change the transport's state from idle to pending. This
allows the user to select the desired stream when running the setup
with PipeWire since it acquires any transport that is pending.
---
 profiles/audio/transport.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 922911cf3..d6493edd0 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -963,6 +963,9 @@ static gboolean get_endpoint(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
+static DBusMessage *select_transport(DBusConnection *conn, DBusMessage *msg,
+					void *data);
+
 static const GDBusMethodTable transport_methods[] = {
 	{ GDBUS_ASYNC_METHOD("Acquire",
 			NULL,
@@ -975,6 +978,8 @@ static const GDBusMethodTable transport_methods[] = {
 							{ "mtu_w", "q" }),
 			try_acquire) },
 	{ GDBUS_ASYNC_METHOD("Release", NULL, NULL, release) },
+	{ GDBUS_ASYNC_METHOD("Select",
+			NULL, NULL, select_transport) },
 	{ },
 };
 
@@ -1292,6 +1297,25 @@ static void transport_update_playing(struct media_transport *transport,
 		transport_set_state(transport, TRANSPORT_STATE_PENDING);
 }
 
+static DBusMessage *select_transport(DBusConnection *conn, DBusMessage *msg,
+					void *data)
+{
+	struct media_transport *transport = data;
+
+	if (transport->owner != NULL)
+		return btd_error_not_authorized(msg);
+
+	if (transport->state >= TRANSPORT_STATE_REQUESTING)
+		return btd_error_not_authorized(msg);
+
+	if (!strcmp(media_endpoint_get_uuid(transport->endpoint),
+						BAA_SERVICE_UUID)) {
+		transport_update_playing(transport, TRUE);
+	}
+
+	return NULL;
+}
+
 static void sink_state_changed(struct btd_service *service,
 						sink_state_t old_state,
 						sink_state_t new_state,
-- 
2.40.1

RE: Add transport.select method

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    FAIL      1.26 seconds
GitLint                       PASS      0.64 seconds
BuildEll                      PASS      24.52 seconds
BluezMake                     PASS      1739.85 seconds
MakeCheck                     PASS      13.02 seconds
MakeDistcheck                 PASS      178.35 seconds
CheckValgrind                 PASS      257.44 seconds
CheckSmatch                   PASS      356.38 seconds
bluezmakeextell               PASS      120.24 seconds
IncrementalBuild              PASS      4541.04 seconds
ScanBuild                     PASS      998.06 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,1/3] transport: Add "select" method
ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#127: FILE: profiles/audio/transport.c:966:
+static DBusMessage *select_transport(DBusConnection *conn, DBusMessage *msg,
                    ^

ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#127: FILE: profiles/audio/transport.c:966:
+static DBusMessage *select_transport(DBusConnection *conn, DBusMessage *msg,
                                                     ^

ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#127: FILE: profiles/audio/transport.c:966:
+static DBusMessage *select_transport(DBusConnection *conn, DBusMessage *msg,
                                                                        ^

/github/workspace/src/src/13728985.patch total: 3 errors, 0 warnings, 42 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13728985.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.




---
Regards,
Linux Bluetooth

[PATCH BlueZ 2/3] client/player: Expose transport "select" method to the user

[Vlad Pruteanu]

This exposes the "select" method for Broadcast transports. This
allows the user to select the desired stream when running the setup
with PipeWire since it acquires any transport that is pending.
---
 client/player.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/client/player.c b/client/player.c
index 0d031e4b0..58805c26a 100644
--- a/client/player.c
+++ b/client/player.c
@@ -4628,6 +4628,23 @@ static void acquire_reply(DBusMessage *message, void *user_data)
 	return bt_shell_noninteractive_quit(EXIT_FAILURE);
 }
 
+static void select_reply(DBusMessage *message, void *user_data)
+{
+	DBusError error;
+
+	dbus_error_init(&error);
+
+	if (dbus_set_error_from_message(&error, message) == TRUE) {
+		bt_shell_printf("Failed to select: %s\n", error.name);
+		dbus_error_free(&error);
+		return bt_shell_noninteractive_quit(EXIT_FAILURE);
+	}
+
+	bt_shell_printf("Select successful");
+
+	return bt_shell_noninteractive_quit(EXIT_SUCCESS);
+}
+
 static void prompt_acquire(const char *input, void *user_data)
 {
 	GDBusProxy *proxy = user_data;
@@ -4849,6 +4866,38 @@ static void cmd_acquire_transport(int argc, char *argv[])
 	return bt_shell_noninteractive_quit(EXIT_SUCCESS);
 }
 
+static void transport_select(GDBusProxy *proxy, bool prompt)
+{
+	if (!g_dbus_proxy_method_call(proxy, "Select", NULL,
+					select_reply, proxy, NULL)) {
+		bt_shell_printf("Failed select transport\n");
+		return;
+	}
+}
+
+static void cmd_select_transport(int argc, char *argv[])
+{
+	GDBusProxy *proxy;
+	int i;
+
+	for (i = 1; i < argc; i++) {
+		proxy = g_dbus_proxy_lookup(transports, NULL, argv[i],
+					BLUEZ_MEDIA_TRANSPORT_INTERFACE);
+		if (!proxy) {
+			bt_shell_printf("Transport %s not found\n", argv[i]);
+			return bt_shell_noninteractive_quit(EXIT_FAILURE);
+		}
+
+		if (find_transport(proxy)) {
+			bt_shell_printf("Transport %s already acquired\n",
+					argv[i]);
+			return bt_shell_noninteractive_quit(EXIT_FAILURE);
+		}
+
+		transport_select(proxy, false);
+	}
+}
+
 static void release_reply(DBusMessage *message, void *user_data)
 {
 	struct transport *transport = user_data;
@@ -5276,6 +5325,9 @@ static const struct bt_shell_menu transport_menu = {
 	{ "volume",      "<transport> [value]",	cmd_volume_transport,
 						"Get/Set transport volume",
 						transport_generator },
+	{ "select",     "<transport>", cmd_select_transport,
+						"Select Transport",
+						transport_generator },
 	{} },
 };
 
-- 
2.40.1

[PATCH BlueZ 3/3] transport: Broadcast sink: wait for user to select transport

[Vlad Pruteanu]

This changes the flow for transports created on broadcast sink side.
Transports are not automatically changed to pending anymore, instead
the user must first run transport.select on them. This allows for the
selection of the desired stream when running the setup with PipeWire,
which acquires any transport that is pending.
---
 profiles/audio/transport.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index d6493edd0..5ad1499b9 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -1676,10 +1676,7 @@ static void bap_state_changed(struct bt_bap_stream *stream, uint8_t old_state,
 			bap_update_qos(transport);
 		else if (bt_bap_stream_io_dir(stream) != BT_BAP_BCAST_SOURCE)
 			bap_update_bcast_qos(transport);
-		if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SOURCE)
-			transport_update_playing(transport, TRUE);
-		else
-			transport_update_playing(transport, FALSE);
+		transport_update_playing(transport, FALSE);
 		return;
 	case BT_BAP_STREAM_STATE_DISABLING:
 		return;
-- 
2.40.1

Re: [PATCH BlueZ 0/3] Add transport.select method

[Luiz Augusto von Dentz]

Hi Vlad,

On Wed, Jul 10, 2024 at 4:13 AM Vlad Pruteanu <vlad.pruteanu@nxp.com> wrote:
>
> This series adds a new "select" method that can be called by
> the user on the broadcast sink side, to select the stream for
> which synchronization is desired. This is achieved by changing
> the states flow so that transport are not set to pending automatically
> anymore. Instead, the transport must first be selected by the user,
> which will update it's state from idle to pending. Any pending
> transport will be acquired by PipeWire.

Hmm, perhaps it would have been better that PipeWire don't auto
pick-up transport on pending state if there are broadcasters, or we
could perhaps perhaps use a different state e.g. "broadcasting", since
pending doesn't really apply for broadcasters as far as streaming is
concerned.

> Furthermore, this method will be used for setting the broadcast code
> of a stream on the sink side. If the encryption flag is set, the
> transport.select call in bluetoothctl will prompt the user to enter
> the code

The roles of bluetoothctl is mostly to demonstrate how client can
implement the handling of D-Bus APIs, so we have to be careful not to
assume it will be used to replace things like Pipewire/audio settings,
so for instance the broadcast code shall be provided by the same
entity attempting to do Transport.Acquire otherwise things may get a
little too convoluted if we need different entities to set transport
up before it is ready to be acquired.

> Vlad Pruteanu (3):
>   transport: Add "select" method
>   client/player: Expose transport "select" method to the user
>   transport: Broadcast sink: wait for user to select transport
>
>  client/player.c            | 52 ++++++++++++++++++++++++++++++++++++++
>  profiles/audio/transport.c | 29 ++++++++++++++++++---
>  2 files changed, 77 insertions(+), 4 deletions(-)
>
> --
> 2.40.1
>


-- 
Luiz Augusto von Dentz

RE: [EXT] Re: [PATCH BlueZ 0/3] Add transport.select method

[Vlad Pruteanu]

Hello Luiz,

> Hi Vlad,
> 
> On Wed, Jul 10, 2024 at 4:13 AM Vlad Pruteanu <vlad.pruteanu@nxp.com>
> wrote:
> >
> > This series adds a new "select" method that can be called by
> > the user on the broadcast sink side, to select the stream for
> > which synchronization is desired. This is achieved by changing
> > the states flow so that transport are not set to pending automatically
> > anymore. Instead, the transport must first be selected by the user,
> > which will update it's state from idle to pending. Any pending
> > transport will be acquired by PipeWire.
> 
> Hmm, perhaps it would have been better that PipeWire don't auto
> pick-up transport on pending state if there are broadcasters, or we
> could perhaps perhaps use a different state e.g. "broadcasting", since
> pending doesn't really apply for broadcasters as far as streaming is
> concerned.
> 
What I propose with this patch is to slightly change how the PENDING
state is triggered on the Broadcast Sink side.
 
Currently:
A Sink scans a Source and automatically moves the associated transport
to PENDING. PipeWire sees this and acquires.
 
My implementation:
A Sink scans a Source,  the associated transport remains in IDLE. User
does transport.select, moving it to PENDING. PipeWire sees this and
acquires.
 
So I wouldn't be changing how the PENDING state is used, just how the
transport gets there. In any case, I think that everything is in line with
the comment for this state:
TRANSPORT_STATE_PENDING,	/* Playing but not acquired */
 
Nonetheless, if you think that the use of this state in the Broadcast context
can cause confusion I will add a comment clarifying it's meaning. Perhaps it
could be placed in the select_transport function.

> > Furthermore, this method will be used for setting the broadcast code
> > of a stream on the sink side. If the encryption flag is set, the
> > transport.select call in bluetoothctl will prompt the user to enter
> > the code
> 
> The roles of bluetoothctl is mostly to demonstrate how client can
> implement the handling of D-Bus APIs, so we have to be careful not to
> assume it will be used to replace things like Pipewire/audio settings,
> so for instance the broadcast code shall be provided by the same
> entity attempting to do Transport.Acquire otherwise things may get a
> little too convoluted if we need different entities to set transport
> up before it is ready to be acquired.
> 
> > Vlad Pruteanu (3):
> >   transport: Add "select" method
> >   client/player: Expose transport "select" method to the user
> >   transport: Broadcast sink: wait for user to select transport
> >
> >  client/player.c            | 52 ++++++++++++++++++++++++++++++++++++++
> >  profiles/audio/transport.c | 29 ++++++++++++++++++---
> >  2 files changed, 77 insertions(+), 4 deletions(-)
> >
> > --
> > 2.40.1
> >
> 
> 
> --
> Luiz Augusto von Dentz

Re: [EXT] Re: [PATCH BlueZ 0/3] Add transport.select method

[Luiz Augusto von Dentz]

Hi Vlad,

On Fri, Jul 19, 2024 at 3:34 AM Vlad Pruteanu <vlad.pruteanu@nxp.com> wrote:
>
> Hello Luiz,
>
> > Hi Vlad,
> >
> > On Wed, Jul 10, 2024 at 4:13 AM Vlad Pruteanu <vlad.pruteanu@nxp.com>
> > wrote:
> > >
> > > This series adds a new "select" method that can be called by
> > > the user on the broadcast sink side, to select the stream for
> > > which synchronization is desired. This is achieved by changing
> > > the states flow so that transport are not set to pending automatically
> > > anymore. Instead, the transport must first be selected by the user,
> > > which will update it's state from idle to pending. Any pending
> > > transport will be acquired by PipeWire.
> >
> > Hmm, perhaps it would have been better that PipeWire don't auto
> > pick-up transport on pending state if there are broadcasters, or we
> > could perhaps perhaps use a different state e.g. "broadcasting", since
> > pending doesn't really apply for broadcasters as far as streaming is
> > concerned.
> >
> What I propose with this patch is to slightly change how the PENDING
> state is triggered on the Broadcast Sink side.
>
> Currently:
> A Sink scans a Source and automatically moves the associated transport
> to PENDING. PipeWire sees this and acquires.
>
> My implementation:
> A Sink scans a Source,  the associated transport remains in IDLE. User
> does transport.select, moving it to PENDING. PipeWire sees this and
> acquires.
>
> So I wouldn't be changing how the PENDING state is used, just how the
> transport gets there. In any case, I think that everything is in line with
> the comment for this state:
> TRANSPORT_STATE_PENDING,        /* Playing but not acquired */
>
> Nonetheless, if you think that the use of this state in the Broadcast context
> can cause confusion I will add a comment clarifying it's meaning. Perhaps it
> could be placed in the select_transport function.

Got it, while this could work I think having a dedicated state for
broadcasters is better since we can document exactly the behavior
since these transports are not attached to any endpoint it need to
manually be acquired by the user rather than automatically like
unicast.

> > > Furthermore, this method will be used for setting the broadcast code
> > > of a stream on the sink side. If the encryption flag is set, the
> > > transport.select call in bluetoothctl will prompt the user to enter
> > > the code
> >
> > The roles of bluetoothctl is mostly to demonstrate how client can
> > implement the handling of D-Bus APIs, so we have to be careful not to
> > assume it will be used to replace things like Pipewire/audio settings,
> > so for instance the broadcast code shall be provided by the same
> > entity attempting to do Transport.Acquire otherwise things may get a
> > little too convoluted if we need different entities to set transport
> > up before it is ready to be acquired.
> >
> > > Vlad Pruteanu (3):
> > >   transport: Add "select" method
> > >   client/player: Expose transport "select" method to the user
> > >   transport: Broadcast sink: wait for user to select transport
> > >
> > >  client/player.c            | 52 ++++++++++++++++++++++++++++++++++++++
> > >  profiles/audio/transport.c | 29 ++++++++++++++++++---
> > >  2 files changed, 77 insertions(+), 4 deletions(-)
> > >
> > > --
> > > 2.40.1
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 0/3] Add transport.select method

[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, 10 Jul 2024 11:13:35 +0300 you wrote:
> This series adds a new "select" method that can be called by
> the user on the broadcast sink side, to select the stream for
> which synchronization is desired. This is achieved by changing
> the states flow so that transport are not set to pending automatically
> anymore. Instead, the transport must first be selected by the user,
> which will update it's state from idle to pending. Any pending
> transport will be acquired by PipeWire.
> 
> [...]

Here is the summary with links:
  - [BlueZ,1/3] transport: Add "select" method
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=083d1a7b66b5
  - [BlueZ,2/3] client/player: Expose transport "select" method to the user
    (no matching commit)
  - [BlueZ,3/3] transport: Broadcast sink: wait for user to select transport
    (no matching commit)

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