| Message ID | 20240710081338.17262-1-vlad.pruteanu@nxp.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add transport.select method | expand |
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 >
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
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
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!