Wait for state to become idle when releasing A2DP

src/ba-transport.c

@@ -107,7 +107,7 @@ static void transport_threads_cancel(struct ba_transport *t) {
 	t->stopping = false;
 	pthread_mutex_unlock(&t->bt_fd_mtx);
 
-	pthread_cond_broadcast(&t->stopped);
+	pthread_cond_broadcast(&t->stopped_cond);
 
 }
 
@@ -250,7 +250,7 @@ static struct ba_transport *transport_new(
 	pthread_mutex_init(&t->codec_select_client_mtx, NULL);
 	pthread_mutex_init(&t->bt_fd_mtx, NULL);
 	pthread_mutex_init(&t->acquisition_mtx, NULL);
-	pthread_cond_init(&t->stopped, NULL);
+	pthread_cond_init(&t->stopped_cond, NULL);
 
 	t->bt_fd = -1;
 
@@ -364,6 +364,17 @@ static int transport_release_bt_a2dp(struct ba_transport *t) {
 			else
 				goto fail;
 		}
+		else {
+
+			/* When A2DP transport is released, its state is set to idle. Such
+			 * change is notified by BlueZ via D-Bus asynchronous D-Bus signal.
+			 * We have to wait for the state change here, because otherwise we
+			 * might receive this state change signal in the middle of transport
+			 * acquisition, which would lead us to an undefined state. */
+			while (t->a2dp.state != BLUEZ_A2DP_TRANSPORT_STATE_IDLE)
+				pthread_cond_wait(&t->a2dp.state_changed_cond, &t->bt_fd_mtx);
+
+		}
 
 	}
 
@@ -403,9 +414,11 @@ struct ba_transport *ba_transport_new_a2dp(
 
 	t->profile = profile;
 
+	pthread_cond_init(&t->a2dp.state_changed_cond, NULL);
+	t->a2dp.state = BLUEZ_A2DP_TRANSPORT_STATE_IDLE;
+
 	t->a2dp.codec = codec;
 	memcpy(&t->a2dp.configuration, configuration, codec->capabilities_size);
-	t->a2dp.state = BLUEZ_A2DP_TRANSPORT_STATE_IDLE;
 
 	err |= transport_pcm_init(&t->a2dp.pcm,
 			is_sink ? BA_TRANSPORT_PCM_MODE_SOURCE : BA_TRANSPORT_PCM_MODE_SINK,
@@ -752,6 +765,9 @@ void ba_transport_destroy(struct ba_transport *t) {
 	if (t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP) {
 		bluealsa_dbus_pcm_unregister(&t->a2dp.pcm);
 		bluealsa_dbus_pcm_unregister(&t->a2dp.pcm_bc);
+		/* Make sure that the transport A2DP state is set to idle
+		 * prior to stopping the IO threads. */
+		ba_transport_set_a2dp_state(t, BLUEZ_A2DP_TRANSPORT_STATE_IDLE);
 	}
 	else if (t->profile & BA_TRANSPORT_PROFILE_MASK_SCO) {
 		bluealsa_dbus_pcm_unregister(&t->sco.pcm_spk);
@@ -841,6 +857,7 @@ void ba_transport_unref(struct ba_transport *t) {
 	if (t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP) {
 		transport_pcm_free(&t->a2dp.pcm);
 		transport_pcm_free(&t->a2dp.pcm_bc);
+		pthread_cond_destroy(&t->a2dp.state_changed_cond);
 	}
 	else if (t->profile & BA_TRANSPORT_PROFILE_MASK_SCO) {
 		if (t->sco.rfcomm != NULL)
@@ -864,7 +881,7 @@ void ba_transport_unref(struct ba_transport *t) {
 	if (t->thread_manager_pipe[1] != -1)
 		close(t->thread_manager_pipe[1]);
 
-	pthread_cond_destroy(&t->stopped);
+	pthread_cond_destroy(&t->stopped_cond);
 	pthread_mutex_destroy(&t->bt_fd_mtx);
 	pthread_mutex_destroy(&t->acquisition_mtx);
 	pthread_mutex_destroy(&t->codec_select_client_mtx);
@@ -1095,7 +1112,7 @@ int ba_transport_stop(struct ba_transport *t) {
 		goto fail;
 
 	while (t->stopping)
-		pthread_cond_wait(&t->stopped, &t->bt_fd_mtx);
+		pthread_cond_wait(&t->stopped_cond, &t->bt_fd_mtx);
 
 fail:
 	pthread_mutex_unlock(&t->bt_fd_mtx);
@@ -1169,7 +1186,7 @@ int ba_transport_acquire(struct ba_transport *t) {
 	/* If we are in the middle of IO threads stopping, wait until all resources
 	 * are reclaimed, so we can acquire them in a clean way once more. */
 	while (t->stopping)
-		pthread_cond_wait(&t->stopped, &t->bt_fd_mtx);
+		pthread_cond_wait(&t->stopped_cond, &t->bt_fd_mtx);
 
 	/* If BT socket file descriptor is still valid, we
 	 * can safely reuse it (e.g. in a keep-alive mode). */
@@ -1239,18 +1256,34 @@ int ba_transport_release(struct ba_transport *t) {
 int ba_transport_set_a2dp_state(
 		struct ba_transport *t,
 		enum bluez_a2dp_transport_state state) {
-	switch (t->a2dp.state = state) {
-	case BLUEZ_A2DP_TRANSPORT_STATE_PENDING:
-		/* When transport is marked as pending, try to acquire transport, but only
-		 * if we are handing A2DP sink profile. For source profile, transport has
-		 * to be acquired by our controller (during the PCM open request). */
-		if (t->profile == BA_TRANSPORT_PROFILE_A2DP_SINK)
-			return ba_transport_acquire(t);
-		return 0;
-	case BLUEZ_A2DP_TRANSPORT_STATE_ACTIVE:
-		return ba_transport_start(t);
-	case BLUEZ_A2DP_TRANSPORT_STATE_IDLE:
-	default:
-		return ba_transport_stop(t);
+
+	pthread_mutex_lock(&t->bt_fd_mtx);
+
+	bool changed = t->a2dp.state != state;
+	t->a2dp.state = state;
+
+	pthread_mutex_unlock(&t->bt_fd_mtx);
+
+	if (changed) {
+
+		pthread_cond_signal(&t->a2dp.state_changed_cond);
+
+		switch (state) {
+		case BLUEZ_A2DP_TRANSPORT_STATE_PENDING:
+			/* When transport is marked as pending, try to acquire transport,
+			 * but only if we are handing A2DP sink profile. For source profile,
+			 * transport has to be acquired by our controller (during the PCM
+			 * open request). */
+			if (t->profile == BA_TRANSPORT_PROFILE_A2DP_SINK)
+				return ba_transport_acquire(t);
+			return 0;
+		case BLUEZ_A2DP_TRANSPORT_STATE_ACTIVE:
+			return ba_transport_start(t);
+		case BLUEZ_A2DP_TRANSPORT_STATE_IDLE:
+			return ba_transport_stop(t);
+		}
+
 	}
+
+	return 0;
 }

src/ba-transport.h

@@ -108,7 +108,7 @@ struct ba_transport {
 	int thread_manager_pipe[2];
 
 	/* indicates IO threads stopping */
-	pthread_cond_t stopped;
+	pthread_cond_t stopped_cond;
 	bool stopping;
 
 	union {
@@ -119,6 +119,7 @@ struct ba_transport {
 			const char *bluez_dbus_sep_path;
 
 			/* current state of the transport */
+			pthread_cond_t state_changed_cond;
 			enum bluez_a2dp_transport_state state;
 
 			/* audio codec configuration capabilities */

src/bluez.c

@@ -1620,7 +1620,7 @@ static unsigned int bluez_bus_watch_id = 0;
 
 /**
  * Subscribe to BlueZ signals. */
-static void bluez_signals_subscribe(void) {
+void bluez_signals_subscribe(void) {
 
 	bluez_sig_sub_id_iface_added = g_dbus_connection_signal_subscribe(config.dbus,
 			BLUEZ_SERVICE, DBUS_IFACE_OBJECT_MANAGER, "InterfacesAdded", NULL, NULL,

test/mock/Makefile.am

@@ -21,6 +21,8 @@ bluealsa_mock_SOURCES = \
 	../../src/ba-transport-pcm.c \
 	../../src/bluealsa-dbus.c \
 	../../src/bluealsa-iface.c \
+	../../src/bluez.c \
+	../../src/bluez-iface.c \
 	../../src/codec-sbc.c \
 	../../src/dbus.c \
 	../../src/h2.c \
@@ -121,6 +123,7 @@ endif
 if ENABLE_MIDI
 bluealsa_mock_SOURCES += \
 	../../src/ble-midi.c \
+	../../src/bluez-midi.c \
 	../../src/midi.c
 endif
 

test/mock/mock-bluealsa.c

@@ -94,18 +94,7 @@ static const a2dp_faststream_t config_faststream_44100_16000 = {
 };
 #endif
 
-bool bluez_a2dp_set_configuration(const char *current_dbus_sep_path,
-		const struct a2dp_sep *sep, GError **error) {
-	debug("%s: %s", __func__, current_dbus_sep_path);
-	(void)current_dbus_sep_path; (void)sep;
-	*error = g_error_new(G_DBUS_ERROR, G_DBUS_ERROR_NOT_SUPPORTED, "Not supported");
-	return false;
-}
-
-void bluez_battery_provider_update(struct ba_device *device) {
-	debug("%s: %p", __func__, device);
-	(void)device;
-}
+void bluez_signals_subscribe(void);
 
 int ofono_call_volume_update(struct ba_transport *transport) {
 	debug("%s: %p", __func__, transport);
@@ -505,6 +494,8 @@ static void mock_ba_dbus_name_acquired(G_GNUC_UNUSED GDBusConnection *conn,
 
 	/* register D-Bus interfaces */
 	bluealsa_dbus_register();
+	/* subscribe for BlueZ signals */
+	bluez_signals_subscribe();
 
 	fprintf(stderr, "BLUEALSA_DBUS_SERVICE_NAME=%s\n", name);
 	mock_sem_signal(userdata);

test/mock/mock-bluez.c

@@ -19,9 +19,11 @@
 
 #include "ba-config.h"
 #include "bluez-iface.h"
-#include "mock-bluez-iface.h"
+#include "dbus.h"
 #include "shared/defs.h"
 
+#include "mock-bluez-iface.h"
+
 /* Bluetooth device name mappings in form of "MAC:name". */
 static const char * devices[8] = { NULL };
 /* Global BlueZ mock server object manager. */
@@ -68,6 +70,16 @@ static void mock_bluez_device_add(const char *path, const char *adapter, const c
 static gboolean mock_bluez_media_transport_release_handler(MockBluezMediaTransport1 *transport,
 		GDBusMethodInvocation *invocation, G_GNUC_UNUSED void *userdata) {
 	mock_bluez_media_transport1_complete_release(transport, invocation);
+
+	GVariantBuilder props;
+	g_variant_builder_init(&props, G_VARIANT_TYPE("a{sv}"));
+	g_variant_builder_add(&props, "{sv}", "State", g_variant_new_string("idle"));
+	GDBusConnection *conn = g_dbus_method_invocation_get_connection(invocation);
+	const char *path = g_dbus_method_invocation_get_object_path(invocation);
+	const char *iface = g_dbus_method_invocation_get_interface_name(invocation);
+	g_dbus_connection_emit_properties_changed(conn, path, iface,
+			g_variant_builder_end(&props), NULL, NULL);
+
 	return TRUE;
 }