[BlueZ,v2,1/5] doc/media: Add 'broadcasting' state and 'Select' method

This adds a new state for transports created by the Broadcast
Sink. Such transports will remain  in the 'idle' state until the
user calls 'Select' on them, at which point they will be moved to
'broadcasting'. This allows the user to select the desired BIS as
the audio server automatically acquires transports that are in this
state.
---
 doc/org.bluez.MediaTransport.rst | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

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=873820

---Test result---

Test Summary:
CheckPatch                    FAIL      1.78 seconds
GitLint                       PASS      1.47 seconds
BuildEll                      PASS      24.43 seconds
BluezMake                     PASS      1678.94 seconds
MakeCheck                     PASS      13.63 seconds
MakeDistcheck                 PASS      176.04 seconds
CheckValgrind                 PASS      249.84 seconds
CheckSmatch                   PASS      352.51 seconds
bluezmakeextell               PASS      118.54 seconds
IncrementalBuild              PASS      7869.54 seconds
ScanBuild                     PASS      986.14 seconds

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

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

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

/github/workspace/src/src/13741852.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/13741852.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

Hi,

to, 2024-07-25 kello 14:58 +0300, Vlad Pruteanu kirjoitti:
> This adds a new state for transports created by the Broadcast
> Sink. Such transports will remain  in the 'idle' state until the
> user calls 'Select' on them, at which point they will be moved to
> 'broadcasting'. This allows the user to select the desired BIS as
> the audio server automatically acquires transports that are in this
> state.
> ---
>  doc/org.bluez.MediaTransport.rst | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/org.bluez.MediaTransport.rst b/doc/org.bluez.MediaTransport.rst
> index 6e95df8f2..47346d36b 100644
> --- a/doc/org.bluez.MediaTransport.rst
> +++ b/doc/org.bluez.MediaTransport.rst
> @@ -7,7 +7,7 @@ BlueZ D-Bus MediaTransport API documentation
>  --------------------------------------------
>  
>  :Version: BlueZ
> -:Date: September 2023
> +:Date: July 2024
>  :Manual section: 5
>  :Manual group: Linux System Administration
>  
> @@ -51,6 +51,18 @@ void Release()
>  
>  	Releases file descriptor.
>  
> +void Select()
> +``````````````
> +
> +	Applicable only for transports created by a broadcast sink. This moves
> +	the transport from 'idle' to 'broadcasting'. Since the audio server
> +	automatically acquires transports that are in this state, the user can
> +	thus select which BISes he wishes to sync to.
> +
> +	Possible Errors:
> +
> +	:org.bluez.Error.NotAuthorized:

Should there also be Unselect() that forces the transport back to
"idle"? 

IIRC, BlueZ as A2DP Sink/ACP behaves like that --- when remote INT
stops audio stream the transport goes back to "idle". So it would be
similar, with the difference that a local application --- instead of
the remote side --- is controlling whether the stream is "playing".

I guess the design question here is whether local apps shall to talk to
BlueZ or the sound server to enable specific audio streams.

Having the "enable" flag either in BlueZ or on audio server side is
possible, esp. if it is question of transport acquire.

As audio server, we could e.g. expose it as the device "on/off" profile
state, which user can turn on/off e.g. in pavucontrol.

***

Note that the current Pipewire BAP Broadcast behavior I think is work
in progress. Acquiring the transport on PENDING I think is maybe
accidental --- it is using the BAP Unicast Server code path, and in one
of the bcast sink patches from NXP I see there seems to be intent to
use BAP Unicast Client like behavior, but probably it's not quite right
if you see the acquire-on-pending behavior.

(As "server" Pipewire will expose audio streams similarly as
application audio streams, and by default direct them to available
audio speakers. As "client" Pipewire exposes the audio streams as
virtual microphone devices, which the user can manually record from or
link to speakers.)

> +
>  Properties
>  ----------
>  
> @@ -84,6 +96,8 @@ string State [readonly]
>  
>  	:"idle": not streaming
>  	:"pending": streaming but not acquired
> +	:"broadcasting": streaming but not acquired, applicable only for transports
> +		created by a broadcast sink
>  	:"active": streaming and acquired
>  
>  uint16 Delay [readwrite, optional]

Hello,

> -----Original Message-----
> From: Pauli Virtanen <pav@iki.fi>
> Sent: Thursday, July 25, 2024 7:46 PM
> To: Vlad Pruteanu <vlad.pruteanu@nxp.com>; linux-
> bluetooth@vger.kernel.org
> Cc: Mihai-Octavian Urzica <mihai-octavian.urzica@nxp.com>; Iulia Tanasescu
> <iulia.tanasescu@nxp.com>; Andrei Istodorescu
> <andrei.istodorescu@nxp.com>; luiz.dentz@gmail.com
> Subject: [EXT] Re: [PATCH BlueZ v2 1/5] doc/media: Add 'broadcasting' state
> and 'Select' method
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> Hi,
> 
> to, 2024-07-25 kello 14:58 +0300, Vlad Pruteanu kirjoitti:
> > This adds a new state for transports created by the Broadcast
> > Sink. Such transports will remain  in the 'idle' state until the
> > user calls 'Select' on them, at which point they will be moved to
> > 'broadcasting'. This allows the user to select the desired BIS as
> > the audio server automatically acquires transports that are in this
> > state.
> > ---
> >  doc/org.bluez.MediaTransport.rst | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/doc/org.bluez.MediaTransport.rst
> b/doc/org.bluez.MediaTransport.rst
> > index 6e95df8f2..47346d36b 100644
> > --- a/doc/org.bluez.MediaTransport.rst
> > +++ b/doc/org.bluez.MediaTransport.rst
> > @@ -7,7 +7,7 @@ BlueZ D-Bus MediaTransport API documentation
> >  --------------------------------------------
> >
> >  :Version: BlueZ
> > -:Date: September 2023
> > +:Date: July 2024
> >  :Manual section: 5
> >  :Manual group: Linux System Administration
> >
> > @@ -51,6 +51,18 @@ void Release()
> >
> >       Releases file descriptor.
> >
> > +void Select()
> > +``````````````
> > +
> > +     Applicable only for transports created by a broadcast sink. This moves
> > +     the transport from 'idle' to 'broadcasting'. Since the audio server
> > +     automatically acquires transports that are in this state, the user can
> > +     thus select which BISes he wishes to sync to.
> > +
> > +     Possible Errors:
> > +
> > +     :org.bluez.Error.NotAuthorized:
> 
> Should there also be Unselect() that forces the transport back to
> "idle"?
> 
Yes we also need this command. I will send an updated patch to also
include this method.

> IIRC, BlueZ as A2DP Sink/ACP behaves like that --- when remote INT
> stops audio stream the transport goes back to "idle". So it would be
> similar, with the difference that a local application --- instead of
> the remote side --- is controlling whether the stream is "playing".
> 
> I guess the design question here is whether local apps shall to talk to
> BlueZ or the sound server to enable specific audio streams.
> 
I think that at least for discovering Broadcast Sources local apps
should talk to BlueZ, since, after all, it's a matter of scanning for
a Bluetooth device that is advertising and acting according to the
profile specification. To me, the process is similar to A2DP, without
the actual connection process, and since for A2DP the local apps first
talk to BlueZ, I believe that this should be the case here as well.  

> Having the "enable" flag either in BlueZ or on audio server side is
> possible, esp. if it is question of transport acquire.
> 
> As audio server, we could e.g. expose it as the device "on/off" profile
> state, which user can turn on/off e.g. in pavucontrol.
> 
> ***
> 
> Note that the current Pipewire BAP Broadcast behavior I think is work
> in progress. Acquiring the transport on PENDING I think is maybe
> accidental --- it is using the BAP Unicast Server code path, and in one
> of the bcast sink patches from NXP I see there seems to be intent to
> use BAP Unicast Client like behavior, but probably it's not quite right
> if you see the acquire-on-pending behavior.
> 
> (As "server" Pipewire will expose audio streams similarly as
> application audio streams, and by default direct them to available
> audio speakers. As "client" Pipewire exposes the audio streams as
> virtual microphone devices, which the user can manually record from or
> link to speakers.)
> 
> > +
> >  Properties
> >  ----------
> >
> > @@ -84,6 +96,8 @@ string State [readonly]
> >
> >       :"idle": not streaming
> >       :"pending": streaming but not acquired
> > +     :"broadcasting": streaming but not acquired, applicable only for
> transports
> > +             created by a broadcast sink
> >       :"active": streaming and acquired
> >
> >  uint16 Delay [readwrite, optional]
> 
> --
> Pauli Virtanen


Regards,
Vlad