Hi Vlad, On Fri, Jul 19, 2024 at 3:34 AM Vlad Pruteanu <vlad.pruteanu@xxxxxxx> wrote: > > Hello Luiz, > > > Hi Vlad, > > > > On Wed, Jul 10, 2024 at 4:13 AM Vlad Pruteanu <vlad.pruteanu@xxxxxxx> > > 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
