From c6b771537ef3c31c13e902ccceb2ae16949695bb Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Thu, 16 Jan 2020 15:39:41 +0100 Subject: [PATCH] bluetooth: Report a2dp_source volume changes to the source device Write the current volume to the `Volume` DBus property to keep the volume on the remote in sync. Without this the remote device shows the wrong volume, and any attempts to change it will cause an unexpected jump when the local volume has also been adjusted. Thanks to prior investments to improve volume synchronization, setting up callbacks and sending initial volume to the peer for HFP/HSP implementing this feature is as easy as unconditionally assigning a valid function to `set_source_volume`. `source_setup_volume_callback` is already responsible for attaching a `SOURCE_VOLUME_CHANGED` hook and sending initial (restored) volume to the peer (signifying support for Absolute Volume - if not derived from the presence of FEATURE_CATEGORY_2 on the profile yet). Part-of: --- src/modules/bluetooth/bluez5-util.c | 31 +++++++++++++++++++++++----- src/modules/bluetooth/module-bluez5-device.c | 9 ++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c index 38ab13a..c442ba6 100644 --- a/src/modules/bluetooth/bluez5-util.c +++ b/src/modules/bluetooth/bluez5-util.c @@ -525,7 +525,7 @@ void pa_bluetooth_transport_set_state(pa_bluetooth_transport *t, pa_bluetooth_tr } } -static pa_volume_t pa_bluetooth_transport_set_sink_volume(pa_bluetooth_transport *t, pa_volume_t volume) { +static pa_volume_t pa_bluetooth_transport_set_volume(pa_bluetooth_transport *t, pa_volume_t volume) { static const char *volume_str = "Volume"; static const char *mediatransport_str = BLUEZ_MEDIA_TRANSPORT_INTERFACE; DBusMessage *m; @@ -541,12 +541,15 @@ static pa_volume_t pa_bluetooth_transport_set_sink_volume(pa_bluetooth_transport /* Propagate rounding and bound checks */ volume = a2dp_gain_to_volume(gain); - pa_assert(t->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK); - - if (t->sink_volume == volume) + if (t->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE && t->source_volume == volume) + return volume; + else if (t->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK && t->sink_volume == volume) return volume; - t->sink_volume = volume; + if (t->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE) + t->source_volume = volume; + else if (t->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK) + t->sink_volume = volume; pa_log_debug("Sending A2DP volume %d/127 to peer", gain); @@ -572,6 +575,18 @@ static pa_volume_t pa_bluetooth_transport_set_sink_volume(pa_bluetooth_transport return volume; } +static pa_volume_t pa_bluetooth_transport_set_sink_volume(pa_bluetooth_transport *t, pa_volume_t volume) { + pa_assert(t); + pa_assert(t->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK); + return pa_bluetooth_transport_set_volume(t, volume); +} + +static pa_volume_t pa_bluetooth_transport_set_source_volume(pa_bluetooth_transport *t, pa_volume_t volume) { + pa_assert(t); + pa_assert(t->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE); + return pa_bluetooth_transport_set_volume(t, volume); +} + static void pa_bluetooth_transport_remote_volume_changed(pa_bluetooth_transport *t, pa_volume_t volume) { pa_bluetooth_hook_t hook; bool is_source; @@ -2100,6 +2115,12 @@ static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage t->acquire = bluez5_transport_acquire_cb; t->release = bluez5_transport_release_cb; t->set_sink_volume = pa_bluetooth_transport_set_sink_volume; + /* A2DP Absolute Volume is optional but BlueZ unconditionally reports + * feature category 2, meaning supporting it is mandatory. + * PulseAudio can and should perform the attenuation anyway in + * the source role as it is the audio rendering device. + */ + t->set_source_volume = pa_bluetooth_transport_set_source_volume; pa_bluetooth_transport_reconfigure(t, &endpoint_conf->bt_codec, a2dp_transport_write, NULL); pa_bluetooth_transport_put(t); diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c index c712e86..fa29ef4 100644 --- a/src/modules/bluetooth/module-bluez5-device.c +++ b/src/modules/bluetooth/module-bluez5-device.c @@ -905,6 +905,8 @@ static void source_setup_volume_callback(pa_source *s) { * If the peer is an AG however backend-native unconditionally provides this * function, PA in the role of HS/HF is responsible for signalling support * by emitting an initial volume command. + * For A2DP bluez-util also unconditionally provides this function to keep + * the peer informed about volume changes. */ if (!u->transport->set_source_volume) return; @@ -921,6 +923,13 @@ static void source_setup_volume_callback(pa_source *s) { /* Send initial volume to peer, signalling support for volume control */ u->transport->set_source_volume(u->transport, pa_cvolume_max(&s->real_volume)); } else { + /* It is yet unknown how (if at all) volume is synchronized for bidirectional + * A2DP codecs. Disallow attaching callbacks (and using HFP n_volume_steps) + * below to a pa_source if the peer is in A2DP_SINK role. This assert should + * be replaced with the proper logic when bidirectional codecs are implemented. + */ + pa_assert(u->profile != PA_BLUETOOTH_PROFILE_A2DP_SINK); + if (s->set_volume == source_set_volume_cb) return; -- 2.7.4