Hi Pauli, On Sat, Oct 28, 2023 at 10:42 AM Pauli Virtanen <pav@xxxxxx> wrote: > > Change unicast BAP configuration to proceed as: > > 0. SelectProperties(endpoint) > 1. ASCS Config Codec > 2. ASCS Server notifies ASE > 3. SelectQoS(configuration) [optional] > 4. ASCS Config QoS > 5. SetConfiguration(transport) > > Previously, SelectProperties had to return also the QoS > configuration. However, it is impossible for it to provide it properly, > because the values supported by the server are known only after ASCS > Config Codec. > > This resolves the issue by adding a new method call, which is supposed > to return suitable QoS values. > > Remove the QoS input parameter from the SelectProperties() call, as the > server supported QoS settings may depend on the codec configuration, and > are not known yet at that point. > > For convenience, e.g. when mandatory QoS presets are used, the endpoint > does not need to implement SelectQoS(). In this case the QoS values > returned by SelectProperties are used. Id leave the endpoint the option to return the QoS on SelectProperties, if it doesn't then we call SelectQoS, that said this also means we need to wait until the endpoint respond with the QoS in order to create the socket or we have to create the socket without any QoS which afaik is required to bind. > --- > > Notes: > Alternative to this is calling SelectProperties() twice at the > two different stages of the ASE setup steps. > > However, if the second SelectProperties() call returns a different > Capabilities configuration, we'd need to either (i) do Config Codec again > or (ii) fail the configuration. > > Doing Config Codec again introduces a chance of getting stuck looping, > if client is not behaving correctly, which doesn't sound like good > design. Failing the configuration raises question why have the > Capabilities as return parameters at all. So instead, make it a separate > method. Well it is also possible that the server returns non-optiomal preferences compared to a different codec configuration but we can only know that once the codec is configured, so we need to be prepared for the headset to misbehave like many do in case of A2DP, but in case of BAP they can messed up in both Codec configuration and QoS stages. > > *** > > If two methods is too much, we could in principle get rid of the > SelectProperties() call and leave only SelectQoS. > > Instead, the sound server would call SetConfiguration() on a remote > endpoint it chooses, and provide the configuration parameters there. > IIUC, this is how it is supposed to work for BAP Broadcast currently. > This might need some sort of "Ready" property on the Device1 DBus object > or elsewhere (e.g. the endpoints), so that it's simple for the sound > server to wait until all endpoints have been exposed in DBus. > > This might also be preferable way to do it, since only the component > closer to the user i.e. the sound server knows which endpoint the user > wanted to use, and when BlueZ guesses wrong it avoids needing to tear > down the old configuration and reconfigure (which we have to do for > A2DP). In principle Im nore in favor of keeping the logic of bluetoothd calling into the endpoint object when it is necessary, we also may apply the same logic as for A2DP that use the last configuration when reconnecting, so there is no guessing but more of a policy of using the last configuration to make the reconnections as quick as possible. If the user wants to change the configuration he can do via SetConfiguration, but this normally requires a user interaction, also if the idea would be to have the reconnection policy on top of bluetoothd Id say it is probably not recommended as that would probably be slower and it may not know all the endpoints registered in order to properly infer what is the best configuration to use. > *** > > This series was tested also vs. this > https://gitlab.freedesktop.org/pvir/pipewire/-/commits/bap-selectqos > > doc/org.bluez.MediaEndpoint.rst | 66 +++++++++++++++++++++++++++++---- > 1 file changed, 58 insertions(+), 8 deletions(-) > > diff --git a/doc/org.bluez.MediaEndpoint.rst b/doc/org.bluez.MediaEndpoint.rst > index 6754d6e3b..4ffe6951c 100644 > --- a/doc/org.bluez.MediaEndpoint.rst > +++ b/doc/org.bluez.MediaEndpoint.rst > @@ -66,6 +66,8 @@ array{byte} SelectConfiguration(array{byte} capabilities) > Note: There is no need to cache the selected configuration since on > success the configuration is send back as parameter of SetConfiguration. > > +.. _SelectProperties: > + > dict SelectProperties(dict capabilities) > ```````````````````````````````````````` > > @@ -79,8 +81,58 @@ dict SelectProperties(dict capabilities) > > :uint32 Locations: > > + See `MediaEndpoint Properties`_ for their possible values. > + > + Returns a configuration which can be used to setup a transport: > + > + :array{byte} Capabilities: > + > + See **org.bluez.MediaTransport(5)**. > + > + :array{byte} Metadata [optional]: > + > + See **org.bluez.MediaTransport(5)**. > + > + :dict QoS: > + > + See **org.bluez.MediaTransport(5)**. > + > + The following fields shall be provided: > + > + :byte TargetLatency: > + :byte PHY: > + > + If `SelectQoS`_ is not implemented, then values for > + all other ``QoS`` fields are also determined by the > + value returned here. > + > + Note: There is no need to cache the selected properties since > + on success the configuration is sent back as parameter of > + `SetConfiguration`_ and `SelectQoS`_. > + > +.. _SelectQoS: > + > +dict SelectQoS(dict configuration) > +`````````````````````````````````` > + > + Select BAP unicast QoS to be used for a transport, based on > + server capabilities and selected configuration. > + > + :object Endpoint: > + > + :array{byte} Capabilities: > + > + The configuration, as returned by `SelectProperties`_. > + > + :array{byte} Metadata [optional]: > + > + The metadata, as returned by `SelectProperties`_. > + > :dict QoS: > > + Server endpoint supported and preferred values. See > + `MediaEndpoint Properties`_ for their possible values. > + > :byte Framing: > :byte PHY: > :uint16 MaximumLatency: > @@ -89,18 +141,16 @@ dict SelectProperties(dict capabilities) > :uint32 PreferredMinimumDelay: > :uint32 PreferredMaximumDelay: > > - See `MediaEndpoint Properties`_ for their possible values. > + Returns a QoS configuration which can be used to setup a transport: > > - Returns a configuration which can be used to setup a transport: > - > - :array{byte} Capabilities: > - :array{byte} Metadata [optional]: > :dict QoS: > > - See `SetConfiguration`_ for their possible values. > + See **org.bluez.MediaTransport(5)** QoS property for > + possible values. > > - Note: There is no need to cache the selected properties since on > - success the configuration is send back as parameter of SetConfiguration. > + Note: There is no need to cache the selected properties since > + on success the configuration is sent back as parameter of > + `SetConfiguration`_. > > void ClearConfiguration(object transport) > ````````````````````````````````````````` > -- > 2.41.0 > -- Luiz Augusto von Dentz
