| Message ID | 20240725115854.234781-2-vlad.pruteanu@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add 'broadcasting' state | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
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
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: + 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]