set_configuration creates a transport before calling SetConfiguration on
the MediaEndpoint1 DBus interface. If this DBus call fails the
transport sticks around while it should instead be cleaned up.
When the peer retries or reconnects (in case of BlueZ: See
avdtp_parse_rej for SET_CONFIGURATION returning TRUE on failure,
resulting in connection_lost with EIO) set_configuration finds this old
transport and returns FALSE. The peer will never succeed this call
unless it randomly decides to call clear_configuration or BlueZ is
restarted.
---
Hi all,
This issue was found while playing around with broken configurations in
PulseAudio. All attempts to set a configuration after the first failure
are rejected. Note that BlueZ disconnects if SET_CONFIGURATION fails,
is that expected? I did not analyze any of this code yet but it seems
rather harsh for something that's not fatal and perhaps better
propagated to the calling application?
As far as I know Pali's A2DP codec additions to PulseAudio expect this
this behaviour; if SetConfiguration fails it'll simply attempt the next
profile until finding one that succeeds.
I moved the functions up above endpoint_reply instead of
forward-declaring them as they were pretty close already and aren't that
many line changes, let me know if that's okay.
profiles/audio/media.c | 77 +++++++++++++++++++++++++-----------------
1 file changed, 46 insertions(+), 31 deletions(-)
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 74064d398..0632fbe8a 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -271,6 +271,37 @@ static void clear_endpoint(struct media_endpoint *endpoint)
clear_configuration(endpoint, endpoint->transports->data);
}
+static int transport_device_cmp(gconstpointer data, gconstpointer user_data)
+{
+ struct media_transport *transport = (struct media_transport *) data;
+ const struct btd_device *device = user_data;
+ const struct btd_device *dev = media_transport_get_dev(transport);
+
+ if (device == dev)
+ return 0;
+
+ return -1;
+}
+
+static struct media_transport *find_device_transport(
+ struct media_endpoint *endpoint,
+ struct btd_device *device)
+{
+ GSList *match;
+
+ match = g_slist_find_custom(endpoint->transports, device,
+ transport_device_cmp);
+ if (match == NULL)
+ return NULL;
+
+ return match->data;
+}
+
+struct a2dp_config_data {
+ struct a2dp_setup *setup;
+ a2dp_endpoint_config_t cb;
+};
+
static void endpoint_reply(DBusPendingCall *call, void *user_data)
{
struct endpoint_request *request = user_data;
@@ -298,6 +329,21 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
return;
}
+ if (dbus_message_is_method_call(request->msg,
+ MEDIA_ENDPOINT_INTERFACE,
+ "SetConfiguration")) {
+ struct a2dp_config_data *data = request->user_data;
+ struct btd_device *device =
+ a2dp_setup_get_device(data->setup);
+ struct media_transport *transport =
+ find_device_transport(endpoint, device);
+
+ if (transport == NULL)
+ error("Expected to destroy transport");
+ else
+ media_transport_destroy(transport);
+ }
+
dbus_error_free(&err);
goto done;
}
@@ -396,37 +442,6 @@ static gboolean select_configuration(struct media_endpoint *endpoint,
return media_endpoint_async_call(msg, endpoint, cb, user_data, destroy);
}
-static int transport_device_cmp(gconstpointer data, gconstpointer user_data)
-{
- struct media_transport *transport = (struct media_transport *) data;
- const struct btd_device *device = user_data;
- const struct btd_device *dev = media_transport_get_dev(transport);
-
- if (device == dev)
- return 0;
-
- return -1;
-}
-
-static struct media_transport *find_device_transport(
- struct media_endpoint *endpoint,
- struct btd_device *device)
-{
- GSList *match;
-
- match = g_slist_find_custom(endpoint->transports, device,
- transport_device_cmp);
- if (match == NULL)
- return NULL;
-
- return match->data;
-}
-
-struct a2dp_config_data {
- struct a2dp_setup *setup;
- a2dp_endpoint_config_t cb;
-};
-
int8_t media_player_get_device_volume(struct btd_device *device)
{
struct avrcp_player *target_player;
--
2.29.1
Marijn Suijten
