PCM client delay reporting for A2DP sink profile

This feature will not work unless there will be a proper delay reporting
implementation in BlueZ (for A2DP sink profile).

src/ba-transport-pcm.c

@@ -24,6 +24,7 @@
 #include <string.h>
 #include <unistd.h>
 
+#include <gio/gio.h>
 #include <glib.h>
 
 #include "audio.h"
@@ -631,7 +632,9 @@ void ba_transport_pcm_volume_set(
 
 }
 
-int ba_transport_pcm_volume_update(struct ba_transport_pcm *pcm) {
+/**
+ * Synchronize PCM volume level with remote Bluetooth device. */
+int ba_transport_pcm_volume_sync(struct ba_transport_pcm *pcm) {
 
 	struct ba_transport *t = pcm->t;
 
@@ -684,8 +687,6 @@ int ba_transport_pcm_volume_update(struct ba_transport_pcm *pcm) {
 	}
 
 final:
-	/* notify connected clients (including requester) */
-	bluealsa_dbus_pcm_update(pcm, BA_DBUS_PCM_UPDATE_VOLUME);
 	return 0;
 }
 
@@ -716,20 +717,63 @@ int ba_transport_pcm_get_hardware_volume(
 	return 0;
 }
 
-int ba_transport_pcm_get_delay(const struct ba_transport_pcm *pcm) {
+/**
+ * Get PCM playback/capture cumulative delay. */
+int ba_transport_pcm_delay_get(const struct ba_transport_pcm *pcm) {
 
 	const struct ba_transport *t = pcm->t;
+	int delay = 0;
 
-	int delay = pcm->codec_delay_dms + pcm->processing_delay_dms;
+	delay += pcm->codec_delay_dms;
+	delay += pcm->processing_delay_dms;
 
-	if (t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP)
+	/* Add delay reported by BlueZ but only for A2DP Source profile. In case
+	 * of A2DP Sink, the BlueZ delay value is in fact our client delay. */
+	if (t->profile & BA_TRANSPORT_PROFILE_A2DP_SOURCE)
 		delay += t->a2dp.delay;
-	if (t->profile & BA_TRANSPORT_PROFILE_MASK_SCO)
+	/* HFP/HSP profiles do not provide any delay information. However, we can
+	 * assume some arbitrary value here - for now it will be 10 ms. */
+	else if (t->profile & BA_TRANSPORT_PROFILE_MASK_AG)
 		delay += 10;
 
 	return delay;
 }
 
+/**
+ * Synchronize PCM playback delay with remote Bluetooth device. */
+int ba_transport_pcm_delay_sync(struct ba_transport_pcm *pcm) {
+
+	struct ba_transport *t = pcm->t;
+	int delay = 0;
+
+	delay += pcm->codec_delay_dms;
+	delay += pcm->processing_delay_dms;
+	delay += pcm->client_delay_dms;
+
+	/* In case of A2DP Sink, update the delay property of the BlueZ media
+	 * transport interface. BlueZ should forward this value to the remote
+	 * device, so it can adjust audio/video synchronization. */
+	if (t->profile == BA_TRANSPORT_PROFILE_A2DP_SINK &&
+			!t->a2dp.read_only_delay_quirk &&
+			abs(delay - t->a2dp.delay) >= 100 /* 10ms */) {
+
+		GError *err = NULL;
+		t->a2dp.delay = delay;
+		g_dbus_set_property(config.dbus, t->bluez_dbus_owner, t->bluez_dbus_path,
+				BLUEZ_IFACE_MEDIA_TRANSPORT, "Delay", g_variant_new_uint16(delay), &err);
+
+		if (err != NULL) {
+			warn("Couldn't set A2DP transport delay: %s", err->message);
+			if (err->code == G_DBUS_ERROR_PROPERTY_READ_ONLY)
+				t->a2dp.read_only_delay_quirk = true;
+			g_error_free(err);
+		}
+
+	}
+
+	return 0;
+}
+
 const char *ba_transport_pcm_channel_to_string(
 		enum ba_transport_pcm_channel channel) {
 	switch (channel) {

src/ba-transport-pcm.h

@@ -247,15 +247,18 @@ void ba_transport_pcm_volume_set(
 		const bool *soft_mute,
 		const bool *hard_mute);
 
-int ba_transport_pcm_volume_update(
+int ba_transport_pcm_volume_sync(
 		struct ba_transport_pcm *pcm);
 
 int ba_transport_pcm_get_hardware_volume(
 		const struct ba_transport_pcm *pcm);
 
-int ba_transport_pcm_get_delay(
+int ba_transport_pcm_delay_get(
 		const struct ba_transport_pcm *pcm);
 
+int ba_transport_pcm_delay_sync(
+		struct ba_transport_pcm *pcm);
+
 const char *ba_transport_pcm_channel_to_string(
 		enum ba_transport_pcm_channel channel);
 

src/ba-transport.h

@@ -143,6 +143,12 @@ struct ba_transport {
 			 * subsequent ioctl() calls. */
 			int bt_fd_coutq_init;
 
+			/* Even though BlueZ documentation says that the Delay property is
+			 * read-write, it might not be true. In case when the delay write
+			 * operation fails with "not writable" error, we should not try to
+			 * update the delay value any more. */
+			bool read_only_delay_quirk;
+
 		} a2dp;
 
 		struct {

src/bluealsa-dbus.c

@@ -281,7 +281,7 @@ static GVariant *ba_variant_new_pcm_codec_config(const struct ba_transport_pcm *
 }
 
 static GVariant *ba_variant_new_pcm_delay(const struct ba_transport_pcm *pcm) {
-	return g_variant_new_uint16(ba_transport_pcm_get_delay(pcm));
+	return g_variant_new_uint16(ba_transport_pcm_delay_get(pcm));
 }
 
 static GVariant *ba_variant_new_pcm_client_delay(const struct ba_transport_pcm *pcm) {
@@ -962,6 +962,7 @@ static bool bluealsa_pcm_set_property(const char *property, GVariant *value,
 
 	if (strcmp(property, "ClientDelay") == 0) {
 		pcm->client_delay_dms = g_variant_get_int16(value);
+		ba_transport_pcm_delay_sync(pcm);
 		bluealsa_dbus_pcm_update(pcm, BA_DBUS_PCM_UPDATE_CLIENT_DELAY);
 		return TRUE;
 	}
@@ -1017,7 +1018,8 @@ static bool bluealsa_pcm_set_property(const char *property, GVariant *value,
 
 		pthread_mutex_unlock(&pcm->mutex);
 
-		ba_transport_pcm_volume_update(pcm);
+		ba_transport_pcm_volume_sync(pcm);
+		bluealsa_dbus_pcm_update(pcm, BA_DBUS_PCM_UPDATE_VOLUME);
 		return true;
 	}
 

test/test-rfcomm.c

@@ -327,7 +327,7 @@ CK_START_TEST(test_rfcomm_hfp_ag) {
 	ba_transport_pcm_volume_set(&pcm->volume[0], &level, NULL, NULL);
 	pthread_mutex_unlock(&pcm->mutex);
 	/* use internal API to update volume */
-	ba_transport_pcm_volume_update(pcm);
+	ba_transport_pcm_volume_sync(pcm);
 	ck_assert_rfcomm_recv(fd, "\r\n+VGS:7\r\n");
 
 	ba_transport_destroy(sco);

test/test-utils-ctl.c

@@ -264,7 +264,7 @@ CK_START_TEST(test_client_delay) {
 
 	struct spawn_process sp_ba_mock;
 	ck_assert_int_ne(spawn_bluealsa_mock(&sp_ba_mock, NULL, true,
-				"--profile=a2dp-source",
+				"--profile=a2dp-sink",
 				NULL), -1);
 
 	char output[4096];
@@ -276,21 +276,21 @@ CK_START_TEST(test_client_delay) {
 
 	/* check default client delay */
 	ck_assert_int_eq(run_bluealsactl(output, sizeof(output),
-				"client-delay", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsrc/sink",
+				"client-delay", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsnk/source",
 				NULL), 0);
 	ck_assert_ptr_ne(strstr(output, "ClientDelay: 0.0 ms"), NULL);
 
 	/* check setting client delay */
 	ck_assert_int_eq(run_bluealsactl(output, sizeof(output),
-				"client-delay", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsrc/sink", "-7.5",
+				"client-delay", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsnk/source", "-7.5",
 				NULL), 0);
 
 	/* check that setting client delay does not affect delay */
 	ck_assert_int_eq(run_bluealsactl(output, sizeof(output),
-				"info", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsrc/sink",
+				"info", "/org/bluealsa/hci11/dev_12_34_56_78_9A_BC/a2dpsnk/source",
 				NULL), 0);
 	ck_assert_ptr_ne(strstr(output, "ClientDelay: -7.5 ms"), NULL);
-	ck_assert_ptr_ne(strstr(output, "Delay: 10.0 ms"), NULL);
+	ck_assert_ptr_ne(strstr(output, "Delay: 0.0 ms"), NULL);
 
 	spawn_terminate(&sp_ba_mock, 0);
 	spawn_close(&sp_ba_mock, NULL);

utils/aplay/aplay.c

@@ -563,8 +563,7 @@ static void *io_worker_routine(struct io_worker *w) {
 				w->ba_pcm.pcm_path, softvol ? "software" : "pass-through");
 		if (softvol != w->ba_pcm.soft_volume) {
 			w->ba_pcm.soft_volume = softvol;
-			if (!ba_dbus_pcm_update(&dbus_ctx, &w->ba_pcm,
-						BLUEALSA_PCM_SOFT_VOLUME, &err)) {
+			if (!ba_dbus_pcm_update(&dbus_ctx, &w->ba_pcm, BLUEALSA_PCM_SOFT_VOLUME, &err)) {
 				error("Couldn't set BlueALSA source PCM volume mode: %s", err.message);
 				dbus_error_free(&err);
 				goto fail;
@@ -806,6 +805,26 @@ static void *io_worker_routine(struct io_worker *w) {
 		/* move leftovers to the beginning and reposition tail */
 		ffb_shift(&buffer, frames * w->ba_pcm.channels);
 
+		int ret;
+		snd_pcm_sframes_t delay_frames;
+		if ((ret = snd_pcm_delay(w->snd_pcm, &delay_frames)) != 0)
+			warn("Couldn't get PCM delay: %s", snd_strerror(ret));
+		else {
+
+			const int delay = delay_frames * 10000 / w->ba_pcm.rate;
+			if (abs(delay - w->ba_pcm.client_delay) >= 100 /* 10ms */) {
+
+				w->ba_pcm.client_delay = delay;
+				if (!ba_dbus_pcm_update(&dbus_ctx, &w->ba_pcm, BLUEALSA_PCM_CLIENT_DELAY, &err)) {
+					error("Couldn't update BlueALSA PCM client delay: %s", err.message);
+					dbus_error_free(&err);
+					goto fail;
+				}
+
+			}
+
+		}
+
 		continue;
 
 close_alsa: