From f7e303e55da7d002805e2fc7d052c3639e1ed92d Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Sun, 21 Nov 2021 10:33:54 +0100 Subject: [PATCH] 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 | 58 +++++++++++++++++++++++++++++++++++++----- src/ba-transport-pcm.h | 7 +++-- src/ba-transport.h | 6 +++++ src/bluealsa-dbus.c | 6 +++-- test/test-rfcomm.c | 2 +- test/test-utils-ctl.c | 10 ++++---- utils/aplay/aplay.c | 23 +++++++++++++++-- 7 files changed, 93 insertions(+), 19 deletions(-) diff --git a/src/ba-transport-pcm.c b/src/ba-transport-pcm.c index 3c7e19ff1..85872e3dd 100644 --- a/src/ba-transport-pcm.c +++ b/src/ba-transport-pcm.c @@ -24,6 +24,7 @@ #include #include +#include #include #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) { diff --git a/src/ba-transport-pcm.h b/src/ba-transport-pcm.h index 73d7fbe14..45ae097ab 100644 --- a/src/ba-transport-pcm.h +++ b/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); diff --git a/src/ba-transport.h b/src/ba-transport.h index 18e1dda9a..c17486dfa 100644 --- a/src/ba-transport.h +++ b/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 { diff --git a/src/bluealsa-dbus.c b/src/bluealsa-dbus.c index 27269a3f7..a059aca8c 100644 --- a/src/bluealsa-dbus.c +++ b/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; } diff --git a/test/test-rfcomm.c b/test/test-rfcomm.c index 4374dd9fb..0c14901fd 100644 --- a/test/test-rfcomm.c +++ b/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); diff --git a/test/test-utils-ctl.c b/test/test-utils-ctl.c index 9d6d327a7..63a0a4f94 100644 --- a/test/test-utils-ctl.c +++ b/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); diff --git a/utils/aplay/aplay.c b/utils/aplay/aplay.c index a3bc8aa60..199ff6bb5 100644 --- a/utils/aplay/aplay.c +++ b/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: