Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
From:   Pauli Virtanen <pav@iki.fi>
To:     linux-bluetooth@vger.kernel.org
Cc:     Pauli Virtanen <pav@iki.fi>
Subject: [PATCH BlueZ 1/4] doc: extend MediaEndpoint1 API with SelectQoS
Date:   Sat, 28 Oct 2023 17:39:26 +0300
Message-ID: <6f03ad1eaaa00f84db8cd6a4a4b88ee83078951d.1698503903.git.pav@iki.fi>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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.
---

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.
    
    ***
    
    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).
    
    ***
    
    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