bluetooth: Report a2dp_source volume changes to the source device
authorMarijn Suijten <marijns95@gmail.com>
Thu, 16 Jan 2020 14:39:41 +0000 (15:39 +0100)
committerPulseAudio Marge Bot <pulseaudio-maintainers@lists.freedesktop.org>
Mon, 17 May 2021 14:50:03 +0000 (14:50 +0000)
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: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/239>

src/modules/bluetooth/bluez5-util.c
src/modules/bluetooth/module-bluez5-device.c

index 38ab13a..c442ba6 100644 (file)
@@ -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);
index c712e86..fa29ef4 100644 (file)
@@ -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;