From 9f8855705157e839b87818e7e94f5d26597b07b8 Mon Sep 17 00:00:00 2001
From: borine <32966433+borine@users.noreply.github.com>
Date: Tue, 29 Oct 2024 09:29:00 +0000
Subject: [PATCH] Enable D-Bus delay signals for processing delay

Make sure that clients are informed of significant changes to the
decoder internal processing delay, even when used with remote
devices that do not report their own delay.
---
 doc/a2dpconf.1.rst     |  2 +-
 src/a2dp-aac.c         |  2 ++
 src/a2dp-aptx-hd.c     |  2 ++
 src/a2dp-aptx.c        |  2 ++
 src/a2dp-faststream.c  |  2 ++
 src/a2dp-lc3plus.c     |  2 ++
 src/a2dp-ldac.c        |  2 ++
 src/a2dp-lhdc.c        |  2 ++
 src/a2dp-mpeg.c        |  2 ++
 src/a2dp-opus.c        |  2 ++
 src/a2dp-sbc.c         |  2 ++
 src/ba-transport-pcm.c | 65 +++++++++++++++++++++++++++---------------
 src/ba-transport-pcm.h |  3 ++
 src/bluealsa-dbus.c    |  5 ++++
 src/sco-lc3-swb.c      |  2 ++
 src/sco-msbc.c         |  2 ++
 test/test-a2dp.c       |  2 ++
 17 files changed, 77 insertions(+), 24 deletions(-)

diff --git a/doc/a2dpconf.1.rst b/doc/a2dpconf.1.rst
index 4c7cd6049..53720e564 100644
--- a/doc/a2dpconf.1.rst
+++ b/doc/a2dpconf.1.rst
@@ -36,7 +36,7 @@ OPTIONS
     Print version and exit.
 
 -v, --verbose
-    Sow verbose bit-stream details.
+    Show verbose bit-stream details.
     Display each field as a binary mask with each bit represented by a single
     character.
 
diff --git a/src/a2dp-aac.c b/src/a2dp-aac.c
index 102e66e21..17d23bcdd 100644
--- a/src/a2dp-aac.c
+++ b/src/a2dp-aac.c
@@ -30,6 +30,7 @@
 #include "ba-config.h"
 #include "ba-transport.h"
 #include "ba-transport-pcm.h"
+#include "bluealsa-dbus.h"
 #include "io.h"
 #include "rtp.h"
 #include "utils.h"
@@ -377,6 +378,7 @@ void *a2dp_aac_enc_thread(struct ba_transport_pcm *t_pcm) {
 					if (!io.initiated) {
 						/* Get the delay due to codec processing. */
 						t_pcm->processing_delay_dms = asrsync_get_dms_since_last_sync(&io.asrs);
+						ba_transport_pcm_delay_sync(t_pcm, BA_DBUS_PCM_UPDATE_DELAY);
 						io.initiated = true;
 					}
 
diff --git a/src/a2dp-aptx-hd.c b/src/a2dp-aptx-hd.c
index 1763b0bec..963b91583 100644
--- a/src/a2dp-aptx-hd.c
+++ b/src/a2dp-aptx-hd.c
@@ -26,6 +26,7 @@
 #include "ba-config.h"
 #include "ba-transport.h"
 #include "ba-transport-pcm.h"
+#include "bluealsa-dbus.h"
 #include "codec-aptx.h"
 #include "io.h"
 #include "rtp.h"
@@ -208,6 +209,7 @@ void *a2dp_aptx_hd_enc_thread(struct ba_transport_pcm *t_pcm) {
 			if (!io.initiated) {
 				/* Get the delay due to codec processing. */
 				t_pcm->processing_delay_dms = asrsync_get_dms_since_last_sync(&io.asrs);
+				ba_transport_pcm_delay_sync(t_pcm, BA_DBUS_PCM_UPDATE_DELAY);
 				io.initiated = true;
 			}
 
diff --git a/src/a2dp-aptx.c b/src/a2dp-aptx.c
index 5170e3c12..7945f9125 100644
--- a/src/a2dp-aptx.c
+++ b/src/a2dp-aptx.c
@@ -25,6 +25,7 @@
 #include "ba-config.h"
 #include "ba-transport.h"
 #include "ba-transport-pcm.h"
+#include "bluealsa-dbus.h"
 #include "codec-aptx.h"
 #include "io.h"
 #include "shared/a2dp-codecs.h"
@@ -192,6 +193,7 @@ void *a2dp_aptx_enc_thread(struct ba_transport_pcm *t_pcm) {
 			if (!io.initiated) {
 				/* Get the delay due to codec processing. */
 				t_pcm->processing_delay_dms = asrsync_get_dms_since_last_sync(&io.asrs);
+				ba_transport_pcm_delay_sync(t_pcm, BA_DBUS_PCM_UPDATE_DELAY);
 				io.initiated = true;
 			}
 
diff --git a/src/a2dp-faststream.c b/src/a2dp-faststream.c
index 2b0d090e1..e073a8e04 100644
--- a/src/a2dp-faststream.c
+++ b/src/a2dp-faststream.c
@@ -24,6 +24,7 @@
 #include "ba-config.h"
 #include "ba-transport.h"
 #include "ba-transport-pcm.h"
+#include "bluealsa-dbus.h"
 #include "codec-sbc.h"
 #include "io.h"
 #include "shared/a2dp-codecs.h"
@@ -216,6 +217,7 @@ void *a2dp_fs_enc_thread(struct ba_transport_pcm *t_pcm) {
 			if (!io.initiated) {
 				/* Get the delay due to codec processing. */
 				t_pcm->processing_delay_dms = asrsync_get_dms_since_last_sync(&io.asrs);
+				ba_transport_pcm_delay_sync(t_pcm, BA_DBUS_PCM_UPDATE_DELAY);
 				io.initiated = true;
 			}
 
diff --git a/src/a2dp-lc3plus.c b/src/a2dp-lc3plus.c
index a86827398..998f980dd 100644
--- a/src/a2dp-lc3plus.c
+++ b/src/a2dp-lc3plus.c
@@ -30,6 +30,7 @@
 #include "ba-config.h"
 #include "ba-transport.h"
 #include "ba-transport-pcm.h"
+#include "bluealsa-dbus.h"
 #include "io.h"
 #include "rtp.h"
 #include "utils.h"
@@ -356,6 +357,7 @@ void *a2dp_lc3plus_enc_thread(struct ba_transport_pcm *t_pcm) {
 				if (!io.initiated) {
 					/* Get the delay due to codec processing. */
 					t_pcm->processing_delay_dms = asrsync_get_dms_since_last_sync(&io.asrs);
+					ba_transport_pcm_delay_sync(t_pcm, BA_DBUS_PCM_UPDATE_DELAY);
 					io.initiated = true;
 				}
 
diff --git a/src/a2dp-ldac.c b/src/a2dp-ldac.c
index 9ecc2e10e..66709b75f 100644
--- a/src/a2dp-ldac.c
+++ b/src/a2dp-ldac.c
@@ -27,6 +27,7 @@
 #include "ba-transport.h"
 #include "ba-transport-pcm.h"
 #include "ba-config.h"
+#include "bluealsa-dbus.h"
 #include "io.h"
 #include "rtp.h"
 #include "utils.h"
@@ -247,6 +248,7 @@ void *a2dp_ldac_enc_thread(struct ba_transport_pcm *t_pcm) {
 				if (!io.initiated) {
 					/* Get the delay due to codec processing. */
 					t_pcm->processing_delay_dms = asrsync_get_dms_since_last_sync(&io.asrs);
+					ba_transport_pcm_delay_sync(t_pcm, BA_DBUS_PCM_UPDATE_DELAY);
 					io.initiated = true;
 				}
 
diff --git a/src/a2dp-lhdc.c b/src/a2dp-lhdc.c
index 804ed47d6..41aba0f81 100644
--- a/src/a2dp-lhdc.c
+++ b/src/a2dp-lhdc.c
@@ -29,6 +29,7 @@
 #include "ba-transport.h"
 #include "ba-transport-pcm.h"
 #include "ba-config.h"
+#include "bluealsa-dbus.h"
 #include "io.h"
 #include "rtp.h"
 #include "utils.h"
@@ -278,6 +279,7 @@ void *a2dp_lhdc_enc_thread(struct ba_transport_pcm *t_pcm) {
 				if (!io.initiated) {
 					/* Get the delay due to codec processing. */
 					t_pcm->processing_delay_dms = asrsync_get_dms_since_last_sync(&io.asrs);
+					ba_transport_pcm_delay_sync(t_pcm, BA_DBUS_PCM_UPDATE_DELAY);
 					io.initiated = true;
 				}
 
diff --git a/src/a2dp-mpeg.c b/src/a2dp-mpeg.c
index 4bbb81fd9..5ed2bcbf0 100644
--- a/src/a2dp-mpeg.c
+++ b/src/a2dp-mpeg.c
@@ -35,6 +35,7 @@
 #include "ba-config.h"
 #include "ba-transport.h"
 #include "ba-transport-pcm.h"
+#include "bluealsa-dbus.h"
 #include "io.h"
 #include "rtp.h"
 #include "utils.h"
@@ -297,6 +298,7 @@ void *a2dp_mp3_enc_thread(struct ba_transport_pcm *t_pcm) {
 				if (!io.initiated) {
 					/* Get the delay due to codec processing. */
 					t_pcm->processing_delay_dms = asrsync_get_dms_since_last_sync(&io.asrs);
+					ba_transport_pcm_delay_sync(t_pcm, BA_DBUS_PCM_UPDATE_DELAY);
 					io.initiated = true;
 				}
 
diff --git a/src/a2dp-opus.c b/src/a2dp-opus.c
index 41140d143..6dd6a4f21 100644
--- a/src/a2dp-opus.c
+++ b/src/a2dp-opus.c
@@ -28,6 +28,7 @@
 #include "ba-config.h"
 #include "ba-transport.h"
 #include "ba-transport-pcm.h"
+#include "bluealsa-dbus.h"
 #include "io.h"
 #include "rtp.h"
 #include "shared/a2dp-codecs.h"
@@ -233,6 +234,7 @@ void *a2dp_opus_enc_thread(struct ba_transport_pcm *t_pcm) {
 			if (!io.initiated) {
 				/* Get the delay due to codec processing. */
 				t_pcm->processing_delay_dms = asrsync_get_dms_since_last_sync(&io.asrs);
+				ba_transport_pcm_delay_sync(t_pcm, BA_DBUS_PCM_UPDATE_DELAY);
 				io.initiated = true;
 			}
 
diff --git a/src/a2dp-sbc.c b/src/a2dp-sbc.c
index 53bf667e3..1d7962263 100644
--- a/src/a2dp-sbc.c
+++ b/src/a2dp-sbc.c
@@ -29,6 +29,7 @@
 #include "ba-transport.h"
 #include "ba-transport-pcm.h"
 #include "ba-config.h"
+#include "bluealsa-dbus.h"
 #include "codec-sbc.h"
 #include "io.h"
 #include "rtp.h"
@@ -267,6 +268,7 @@ void *a2dp_sbc_enc_thread(struct ba_transport_pcm *t_pcm) {
 				 * frame is written, the BT sink can start decoding and playing
 				 * audio in a continuous fashion. */
 				t_pcm->processing_delay_dms = asrsync_get_dms_since_last_sync(&io.asrs);
+				ba_transport_pcm_delay_sync(t_pcm, BA_DBUS_PCM_UPDATE_DELAY);
 				io.initiated = true;
 			}
 
diff --git a/src/ba-transport-pcm.c b/src/ba-transport-pcm.c
index d9306aa14..7bf4917e9 100644
--- a/src/ba-transport-pcm.c
+++ b/src/ba-transport-pcm.c
@@ -750,39 +750,58 @@ int ba_transport_pcm_delay_get(const struct ba_transport_pcm *pcm) {
 int ba_transport_pcm_delay_sync(struct ba_transport_pcm *pcm, unsigned int update_mask) {
 
 	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.delay_reporting &&
-			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) {
-			if (err->code == G_DBUS_ERROR_PROPERTY_READ_ONLY)
-				/* 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 report value any more. */
-				t->a2dp.delay_reporting = false;
-			warn("Couldn't set A2DP transport delay: %s", err->message);
-			g_error_free(err);
+	if (t->profile == BA_TRANSPORT_PROFILE_A2DP_SINK) {
+
+		int delay = 0;
+		delay += pcm->codec_delay_dms;
+		delay += pcm->processing_delay_dms;
+		delay += pcm->client_delay_dms;
+
+		if (t->a2dp.delay_reporting &&
+					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) {
+				if (err->code == G_DBUS_ERROR_PROPERTY_READ_ONLY)
+					/* 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 report
+					 * value any more. */
+					t->a2dp.delay_reporting = false;
+				warn("Couldn't set A2DP transport delay: %s", err->message);
+				g_error_free(err);
+			}
+
 		}
+	}
 
+	/* If the remote device does not provide delay update reports we can still
+	 * inform local D-Bus clients of our internal processing delay. */
+	if ((t->profile & BA_TRANSPORT_PROFILE_MASK_A2DP) == 0 ||
+				!t->a2dp.delay_reporting) {
+		if (update_mask == BA_DBUS_PCM_UPDATE_DELAY) {
+			/* To avoid creating a flood of D-Bus signals, we only notify clients
+			 * when the codec + processing value changes by more than 10ms. */
+			int delay = pcm->codec_delay_dms + pcm->processing_delay_dms;
+			if (abs(delay - (int)pcm->reported_codec_delay_dms) < 100 /* 10ms */)
+				goto final;
+			pcm->reported_codec_delay_dms = delay;
+		}
 	}
 
 	/* Notify all connected D-Bus clients. */
 	bluealsa_dbus_pcm_update(pcm, update_mask);
+
+final:
 	return 0;
 }
 
diff --git a/src/ba-transport-pcm.h b/src/ba-transport-pcm.h
index acd1e9d85..7ff4f436c 100644
--- a/src/ba-transport-pcm.h
+++ b/src/ba-transport-pcm.h
@@ -129,6 +129,9 @@ struct ba_transport_pcm {
 	 * the host computational power. It is used to compensate for the time
 	 * required to encode or decode audio. */
 	unsigned int processing_delay_dms;
+	/* The last reported total codec + processing delay. It is used to limit
+	 * the rate at which changes are reported via D-Bus. */
+	unsigned int reported_codec_delay_dms;
 	/* Positive (or negative) delay reported by the client. */
 	int client_delay_dms;
 
diff --git a/src/bluealsa-dbus.c b/src/bluealsa-dbus.c
index 431b1199d..f6283c244 100644
--- a/src/bluealsa-dbus.c
+++ b/src/bluealsa-dbus.c
@@ -538,6 +538,11 @@ static void bluealsa_pcm_open(GDBusMethodInvocation *inv, void *userdata) {
 			goto fail;
 		}
 
+		/* If the transport thread has updated its codec delay value during its
+		 * start procedure then we inform clients that the delay has changed. */
+		if (pcm->codec_delay_dms > 0)
+			bluealsa_dbus_pcm_update(pcm, BA_DBUS_PCM_UPDATE_DELAY);
+
 	}
 
 	pthread_mutex_lock(&pcm->mutex);
diff --git a/src/sco-lc3-swb.c b/src/sco-lc3-swb.c
index a1e312327..41713c8ed 100644
--- a/src/sco-lc3-swb.c
+++ b/src/sco-lc3-swb.c
@@ -22,6 +22,7 @@
 
 #include "ba-transport.h"
 #include "ba-transport-pcm.h"
+#include "bluealsa-dbus.h"
 #include "codec-lc3-swb.h"
 #include "io.h"
 #include "shared/defs.h"
@@ -80,6 +81,7 @@ void *sco_lc3_swb_enc_thread(struct ba_transport_pcm *t_pcm) {
 				if (!io.initiated) {
 					/* Get the delay due to codec processing. */
 					t_pcm->processing_delay_dms = asrsync_get_dms_since_last_sync(&io.asrs);
+					ba_transport_pcm_delay_sync(t_pcm, BA_DBUS_PCM_UPDATE_DELAY);
 					io.initiated = true;
 				}
 
diff --git a/src/sco-msbc.c b/src/sco-msbc.c
index 5b8dda6a4..669e7b4e1 100644
--- a/src/sco-msbc.c
+++ b/src/sco-msbc.c
@@ -19,6 +19,7 @@
 
 #include "ba-transport.h"
 #include "ba-transport-pcm.h"
+#include "bluealsa-dbus.h"
 #include "codec-msbc.h"
 #include "io.h"
 #include "shared/defs.h"
@@ -87,6 +88,7 @@ void *sco_msbc_enc_thread(struct ba_transport_pcm *t_pcm) {
 				if (!io.initiated) {
 					/* Get the delay due to codec processing. */
 					t_pcm->processing_delay_dms = asrsync_get_dms_since_last_sync(&io.asrs);
+					ba_transport_pcm_delay_sync(t_pcm, BA_DBUS_PCM_UPDATE_DELAY);
 					io.initiated = true;
 				}
 
diff --git a/test/test-a2dp.c b/test/test-a2dp.c
index c69f9828e..0c1c95d70 100644
--- a/test/test-a2dp.c
+++ b/test/test-a2dp.c
@@ -51,6 +51,8 @@ int ba_transport_pcm_state_set(struct ba_transport_pcm *pcm,
 enum ba_transport_pcm_signal ba_transport_pcm_signal_recv(struct ba_transport_pcm *pcm) {
 	(void)pcm; return -1; }
 void ba_transport_pcm_thread_cleanup(struct ba_transport_pcm *pcm) { (void)pcm; }
+int ba_transport_pcm_delay_sync(struct ba_transport_pcm *pcm, unsigned int update_mask) {
+	(void)pcm; (void)update_mask; return -1; }
 
 CK_START_TEST(test_a2dp_codecs_codec_id_from_string) {
 	ck_assert_uint_eq(a2dp_codecs_codec_id_from_string("SBC"), A2DP_CODEC_SBC);