Cleanup old PCM when InterfacesRemoved is received

In order not to grow internal PCM cache indefinitely, we have to cleanup
old/stale entries when BlueALSA reports PCM as removed. Also, not doing
so might lead to incorrect PCM properties being used when the properties
changed signal is received. This might lead to playback malfunctions -
e.g. incorrect sampling rate being selected.

Closes #644

test/mock/mock-bluealsa.c

@@ -415,6 +415,13 @@ static void *mock_bluealsa_service_thread(void *userdata) {
 		if (mock_fuzzing_ms)
 			ba_transport_set_codec(t, HFP_CODEC_CVSD);
 
+#if ENABLE_MSBC
+		if (mock_fuzzing_ms) {
+			usleep(mock_fuzzing_ms * 1000);
+			ba_transport_set_codec(t, HFP_CODEC_MSBC);
+		}
+#endif
+
 	}
 
 	if (config.profile.hsp_ag) {

test/test-alsa-ctl.c

@@ -497,15 +497,15 @@ CK_START_TEST(test_notifications) {
 
 	size_t events_update_codec = 0;
 #if ENABLE_MSBC
-	events_update_codec += 4;
+	events_update_codec += 8;
 #endif
 
 	/* Processed events:
 	 * - 0 removes; 2 new elems (12:34:... A2DP)
 	 * - 2 removes; 4 new elems (12:34:... A2DP, 23:45:... A2DP)
 	 * - 4 removes; 7 new elems (2x A2DP, SCO playback, battery)
 	 * - 7 removes; 9 new elems (2x A2DP, SCO playback/capture, battery)
-	 * - 4 updates (SCO codec update if mSBC is supported)
+	 * - 8 updates (SCO codec updates if mSBC is supported)
 	 *
 	 * XXX: It is possible that the battery element (RFCOMM D-Bus path) will not
 	 *      be exported in time. In such case, the number of events will be less

test/test-utils-aplay.c

@@ -174,7 +174,7 @@ CK_START_TEST(test_play_all) {
 				NULL), -1);
 	spawn_terminate(&sp_ba_aplay, 500);
 
-	char output[4096] = "";
+	char output[8192] = "";
 	ck_assert_int_gt(fread(output, 1, sizeof(output) - 1, sp_ba_aplay.f_stderr), 0);
 	fprintf(stderr, "%s", output);
 
@@ -206,7 +206,7 @@ CK_START_TEST(test_play_single_audio) {
 				NULL), -1);
 	spawn_terminate(&sp_ba_aplay, 500);
 
-	char output[4096] = "";
+	char output[8192] = "";
 	ck_assert_int_gt(fread(output, 1, sizeof(output) - 1, sp_ba_aplay.f_stderr), 0);
 	fprintf(stderr, "%s", output);
 
@@ -248,7 +248,7 @@ CK_START_TEST(test_play_mixer_setup) {
 				NULL), -1);
 	spawn_terminate(&sp_ba_aplay, 500);
 
-	char output[4096] = "";
+	char output[8192] = "";
 	ck_assert_int_gt(fread(output, 1, sizeof(output) - 1, sp_ba_aplay.f_stderr), 0);
 	fprintf(stderr, "%s", output);
 
@@ -265,6 +265,53 @@ CK_START_TEST(test_play_mixer_setup) {
 
 } CK_END_TEST
 
+CK_START_TEST(test_play_dbus_signals) {
+
+	struct spawn_process sp_ba_mock;
+	ck_assert_int_ne(spawn_bluealsa_mock(&sp_ba_mock, NULL, false,
+				"--timeout=0",
+				"--profile=hfp-ag",
+				"--fuzzing=250",
+				NULL), -1);
+
+	struct spawn_process sp_ba_aplay;
+	ck_assert_int_ne(spawn_bluealsa_aplay(&sp_ba_aplay,
+				"--profile-sco",
+				"--pcm=null",
+				"-vv",
+				NULL), -1);
+	spawn_terminate(&sp_ba_aplay, 1500);
+
+	char output[8192] = "";
+	ck_assert_int_gt(fread(output, 1, sizeof(output) - 1, sp_ba_aplay.f_stderr), 0);
+	fprintf(stderr, "%s", output);
+
+#if ENABLE_MSBC && DEBUG
+	/* with mSBC support, codec is not selected right away */
+	ck_assert_ptr_ne(strstr(output,
+				"Skipping SCO with codec not selected"), NULL);
+#endif
+
+	ck_assert_ptr_ne(strstr(output,
+				"Used configuration for 12:34:56:78:9A:BC"), NULL);
+	/* check proper sampling rate for CVSD codec */
+	ck_assert_ptr_ne(strstr(output,
+				"Sampling rate: 8000 Hz"), NULL);
+
+#if ENABLE_MSBC
+	ck_assert_ptr_ne(strstr(output,
+				"Used configuration for 12:34:56:78:9A:BC"), NULL);
+	/* check proper sampling rate for mSBC codec */
+	ck_assert_ptr_ne(strstr(output,
+				"Sampling rate: 16000 Hz"), NULL);
+#endif
+
+	spawn_close(&sp_ba_aplay, NULL);
+	spawn_terminate(&sp_ba_mock, 0);
+	spawn_close(&sp_ba_mock, NULL);
+
+} CK_END_TEST
+
 int main(int argc, char *argv[], char *envp[]) {
 	preload(argc, argv, envp, ".libs/aloader.so");
 
@@ -289,6 +336,7 @@ int main(int argc, char *argv[], char *envp[]) {
 	tcase_add_test(tc, test_play_all);
 	tcase_add_test(tc, test_play_single_audio);
 	tcase_add_test(tc, test_play_mixer_setup);
+	tcase_add_test(tc, test_play_dbus_signals);
 
 	srunner_run_all(sr, CK_ENV);
 	int nf = srunner_ntests_failed(sr);

utils/aplay/aplay.c

@@ -246,17 +246,32 @@ static void print_bt_pcm_list(void) {
 
 }
 
-static struct ba_pcm *get_ba_pcm(const char *path) {
-
-	size_t i;
+static struct ba_pcm *ba_pcm_add(const struct ba_pcm *pcm) {
+	struct ba_pcm *tmp;
+	if ((tmp = realloc(ba_pcms, (ba_pcms_count + 1) * sizeof(*ba_pcms))) == NULL)
+		return NULL;
+	ba_pcms = tmp;
+	memcpy(&ba_pcms[ba_pcms_count], pcm, sizeof(*ba_pcms));
+	return &ba_pcms[ba_pcms_count++];
+}
 
-	for (i = 0; i < ba_pcms_count; i++)
+static struct ba_pcm *ba_pcm_get(const char *path) {
+	for (size_t i = 0; i < ba_pcms_count; i++)
 		if (strcmp(ba_pcms[i].pcm_path, path) == 0)
 			return &ba_pcms[i];
-
 	return NULL;
 }
 
+static void ba_pcm_remove(const char *path) {
+	for (size_t i = 0; i < ba_pcms_count; i++)
+		if (strcmp(ba_pcms[i].pcm_path, path) == 0) {
+			memmove(&ba_pcms[i], &ba_pcms[i + 1],
+					(ba_pcms_count - i - 1) * sizeof(*ba_pcms));
+			ba_pcms_count--;
+			break;
+		}
+}
+
 static struct io_worker *get_active_io_worker(void) {
 
 	struct io_worker *w = NULL;
@@ -911,13 +926,10 @@ static DBusHandlerResult dbus_signal_handler(DBusConnection *conn, DBusMessage *
 			}
 			if (pcm.transport == BA_PCM_TRANSPORT_NONE)
 				goto fail;
-			struct ba_pcm *tmp = ba_pcms;
-			if ((ba_pcms = realloc(ba_pcms, (ba_pcms_count + 1) * sizeof(*ba_pcms))) == NULL) {
-				error("Couldn't add new BlueALSA PCM: %s", strerror(ENOMEM));
-				ba_pcms = tmp;
+			if (ba_pcm_add(&pcm) == NULL) {
+				error("Couldn't add new BlueALSA PCM: %s", strerror(errno));
 				goto fail;
 			}
-			memcpy(&ba_pcms[ba_pcms_count++], &pcm, sizeof(*ba_pcms));
 			supervise_io_worker(&pcm);
 			return DBUS_HANDLER_RESULT_HANDLED;
 		}
@@ -930,17 +942,18 @@ static DBusHandlerResult dbus_signal_handler(DBusConnection *conn, DBusMessage *
 			}
 			dbus_message_iter_get_basic(&iter, &path);
 			struct ba_pcm *pcm;
-			if ((pcm = get_ba_pcm(path)) == NULL)
+			if ((pcm = ba_pcm_get(path)) == NULL)
 				goto fail;
 			supervise_io_worker_stop(pcm);
+			ba_pcm_remove(path);
 			return DBUS_HANDLER_RESULT_HANDLED;
 		}
 
 	}
 
 	if (strcmp(interface, DBUS_INTERFACE_PROPERTIES) == 0) {
 		struct ba_pcm *pcm;
-		if ((pcm = get_ba_pcm(path)) == NULL)
+		if ((pcm = ba_pcm_get(path)) == NULL)
 			goto fail;
 		if (!dbus_message_iter_init(message, &iter) ||
 				dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING) {