[PATCH BlueZ] media: use custom DBus timeouts only when remote side is waiting

[PATCH BlueZ] media: use custom DBus timeouts only when remote side is waiting

[Pauli Virtanen]

Under high system load (VM instance on boot) it's observed the 3 sec
timeout BlueZ uses for BAP broadcast SetConfiguration may be missed by
Wireplumber, as these are set up immediately on startup together with
any other setup (eg ALSA) that may need time.

There's no actual need for using a short custom timeout in BlueZ for
this, as in this case there is no remote side that is waiting for a reply.

Fix by limiting custom timeouts to cases where there is a waiting
remote.  A2DP-specific timeout should be used only for A2DP
SetConfiguration.  Similarly, using timeout for BAP only makes sense for
unicast, and should derive from the ATT timeout.
---
 profiles/audio/media.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index cdaafb04e..0297e4c79 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -71,7 +71,11 @@
 #define MEDIA_ENDPOINT_INTERFACE "org.bluez.MediaEndpoint1"
 #define MEDIA_PLAYER_INTERFACE "org.mpris.MediaPlayer2.Player"
 
-#define REQUEST_TIMEOUT (3 * 1000)		/* 3 seconds */
+/* Timeout should be less than avdtp request timeout (4 seconds) */
+#define A2DP_REQUEST_TIMEOUT_MSEC	(3 * 1000)
+
+/* Timeout should be less than ATT timeout (30 seconds) */
+#define BAP_REQUEST_TIMEOUT_MSEC	(20 * 1000)
 
 struct media_app {
 	struct media_adapter	*adapter;
@@ -465,16 +469,16 @@ static gboolean media_endpoint_async_call(DBusMessage *msg,
 					struct media_transport *transport,
 					media_endpoint_cb_t cb,
 					void *user_data,
-					GDestroyNotify destroy)
+					GDestroyNotify destroy,
+					int timeout_msec)
 {
 	struct endpoint_request *request;
 
 	request = g_new0(struct endpoint_request, 1);
 
-	/* Timeout should be less than avdtp request timeout (4 seconds) */
 	if (g_dbus_send_message_with_reply(btd_get_dbus_connection(),
 						msg, &request->call,
-						REQUEST_TIMEOUT) == FALSE) {
+						timeout_msec) == FALSE) {
 		error("D-Bus send failed");
 		g_free(request);
 		return FALSE;
@@ -521,7 +525,7 @@ static gboolean select_configuration(struct media_endpoint *endpoint,
 					DBUS_TYPE_INVALID);
 
 	return media_endpoint_async_call(msg, endpoint, NULL,
-						cb, user_data, destroy);
+						cb, user_data, destroy, -1);
 }
 
 static int transport_device_cmp(gconstpointer data, gconstpointer user_data)
@@ -604,7 +608,8 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
 	g_dbus_get_properties(conn, path, "org.bluez.MediaTransport1", &iter);
 
 	return media_endpoint_async_call(msg, endpoint, transport,
-						cb, user_data, destroy);
+						cb, user_data, destroy,
+						A2DP_REQUEST_TIMEOUT_MSEC);
 }
 #endif
 
@@ -1093,7 +1098,7 @@ static int pac_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
 	dbus_message_iter_close_container(&iter, &dict);
 
 	if (!media_endpoint_async_call(msg, endpoint, NULL, pac_select_cb,
-								data, free))
+								data, free, -1))
 		return -EIO;
 
 	return 0;
@@ -1233,6 +1238,7 @@ static int pac_config(struct bt_bap_stream *stream, struct iovec *cfg,
 	DBusMessage *msg;
 	DBusMessageIter iter;
 	const char *path;
+	int timeout_msec;
 
 	DBG("endpoint %p stream %p", endpoint, stream);
 
@@ -1243,9 +1249,11 @@ static int pac_config(struct bt_bap_stream *stream, struct iovec *cfg,
 	switch (bt_bap_stream_get_type(stream)) {
 	case BT_BAP_STREAM_TYPE_UCAST:
 		transport = pac_ucast_config(stream, cfg, endpoint);
+		timeout_msec = BAP_REQUEST_TIMEOUT_MSEC;
 		break;
 	case BT_BAP_STREAM_TYPE_BCAST:
 		transport = pac_bcast_config(stream, cfg, endpoint);
+		timeout_msec = -1;
 		break;
 	default:
 		transport = NULL;
@@ -1279,7 +1287,8 @@ static int pac_config(struct bt_bap_stream *stream, struct iovec *cfg,
 	g_dbus_get_properties(conn, path, "org.bluez.MediaTransport1", &iter);
 
 	if (!media_endpoint_async_call(msg, endpoint, transport,
-						pac_config_cb, data, free))
+						pac_config_cb, data, free,
+						timeout_msec))
 		return -EIO;
 
 	return 0;
-- 
2.54.0

RE: [BlueZ] media: use custom DBus timeouts only when remote side is waiting

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1339 bytes --]

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

---Test result---

Test Summary:
CheckPatch                    PASS      0.32 seconds
GitLint                       PASS      0.20 seconds
BuildEll                      PASS      20.21 seconds
BluezMake                     PASS      660.37 seconds
MakeCheck                     PASS      3.56 seconds
MakeDistcheck                 FAIL      230.19 seconds
CheckValgrind                 PASS      225.47 seconds
CheckSmatch                   PASS      351.66 seconds
bluezmakeextell               PASS      182.49 seconds
IncrementalBuild              PASS      670.58 seconds
ScanBuild                     PASS      1032.73 seconds

Details
##############################
Test: MakeDistcheck - FAIL
Desc: Run Bluez Make Distcheck
Output:

make[4]: *** [Makefile:10239: test-suite.log] Error 1
make[3]: *** [Makefile:10347: check-TESTS] Error 2
make[2]: *** [Makefile:10818: check-am] Error 2
make[1]: *** [Makefile:10820: check] Error 2
make: *** [Makefile:10741: distcheck] Error 1


https://github.com/bluez/bluez/pull/2129

---
Regards,
Linux Bluetooth