bluetooth: Synchronize AVRCP Absolute Volume with A2DP sink

Like the previous commit this handles `Volume` property changes but
applies them to an A2DP sink instead of source stream.  As mentioned in
the AVRCP spec v1.6.2 §5.8 the rendering device (A2DP sink) is
responsible for performing volume attenuation meaning PulseAudio should
pass through audio as-is without performing any attenuation in SW.
Setting a valid pointer to `set_sink_volume` and returning `true` from
`should_attenuate_volume` attaches a hardware callback to `pa_sink` such
that no volume attenuation is performed anymore.

In addition to receiving volume change notifications it is also possible
to control remote volume by writing a new value to the DBus property.
This is especially useful when playing back to in-ear audio devices
which usually lack physical buttons to adjust the final volume on the
sink.

While software volume (used before this patch) is generally fine it is
annoying to crank it up all the way to 100% when a previous connection
to a different device left saved volume on the peer at a low volume.
Providing this bidirectional synchronization is most natural to users
who wish to use physical controls on their headphones, are used to this
from their smartphone, or aforementioned volume mismatches where both PA
as source and the peer as sink/rendering device are performing
attenutation.

Part-of: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/239>

diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
index 4453e87..38ab13a 100644 (file)
--- a/src/modules/bluetooth/bluez5-util.c
+++ b/src/modules/bluetooth/bluez5-util.c
@@ -114,6 +114,19 @@ static pa_volume_t a2dp_gain_to_volume(uint16_t gain) {
     return volume;
 }
 
+static uint16_t volume_to_a2dp_gain(pa_volume_t volume) {
+    uint16_t gain = (uint16_t) ((
+        volume * A2DP_MAX_GAIN
+        /* Round to closest by adding half the denominator */
+        + PA_VOLUME_NORM / 2
+    ) / PA_VOLUME_NORM);
+
+    if (gain > A2DP_MAX_GAIN)
+        gain = A2DP_MAX_GAIN;
+
+    return gain;
+}
+
 struct pa_bluetooth_discovery {
     PA_REFCNT_DECLARE;
 
@@ -187,6 +200,9 @@ pa_bluetooth_transport *pa_bluetooth_transport_new(pa_bluetooth_device *d, const
     t->path = pa_xstrdup(path);
     t->profile = p;
     t->config_size = size;
+    /* Always force initial volume to be set/propagated correctly */
+    t->sink_volume = PA_VOLUME_INVALID;
+    t->source_volume = PA_VOLUME_INVALID;
 
     if (size > 0) {
         t->config = pa_xnew(uint8_t, size);
@@ -509,22 +525,81 @@ void pa_bluetooth_transport_set_state(pa_bluetooth_transport *t, pa_bluetooth_tr
     }
 }
 
-static void pa_bluetooth_transport_remote_volume_changed(pa_bluetooth_transport *t, uint16_t gain) {
-    pa_volume_t volume;
+static pa_volume_t pa_bluetooth_transport_set_sink_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;
+    DBusMessageIter iter;
+    uint16_t gain;
 
     pa_assert(t);
+    pa_assert(t->device);
+    pa_assert(t->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK || t->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE);
+    pa_assert(t->device->discovery);
 
+    gain = volume_to_a2dp_gain(volume);
+    /* Propagate rounding and bound checks */
     volume = a2dp_gain_to_volume(gain);
 
-    /* increment volume by one to correct rounding errors */
-    if (volume < PA_VOLUME_NORM)
-        volume++;
+    pa_assert(t->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK);
 
-    if (t->source_volume == volume)
-        return;
+    if (t->sink_volume == volume)
+        return volume;
+
+    t->sink_volume = volume;
+
+    pa_log_debug("Sending A2DP volume %d/127 to peer", gain);
+
+    pa_assert_se(m = dbus_message_new_method_call(BLUEZ_SERVICE, t->path, DBUS_INTERFACE_PROPERTIES, "Set"));
+
+    dbus_message_iter_init_append(m, &iter);
+    pa_assert_se(dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &mediatransport_str));
+    pa_assert_se(dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &volume_str));
+    pa_dbus_append_basic_variant(&iter, DBUS_TYPE_UINT16, &gain);
+
+    /* Ignore replies, wait for the Volume property to change (generally arrives
+     * before this function replies).
+     *
+     * In an ideal world BlueZ exposes a function to change volume, that returns
+     * with the actual volume set by the peer as returned by the SetAbsoluteVolume
+     * AVRCP command.  That is required later to perform software volume compensation
+     * based on actual playback volume.
+     */
+    dbus_message_set_no_reply(m, true);
+    pa_assert_se(dbus_connection_send(pa_dbus_connection_get(t->device->discovery->connection), m, NULL));
+    dbus_message_unref(m);
 
-    t->source_volume = volume;
-    pa_hook_fire(pa_bluetooth_discovery_hook(t->device->discovery, PA_BLUETOOTH_HOOK_TRANSPORT_SOURCE_VOLUME_CHANGED), t);
+    return 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;
+    char volume_str[PA_VOLUME_SNPRINT_MAX];
+
+    pa_assert(t);
+
+    is_source = t->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE;
+
+    if (is_source) {
+        if (t->source_volume == volume)
+            return;
+        t->source_volume = volume;
+        hook = PA_BLUETOOTH_HOOK_TRANSPORT_SOURCE_VOLUME_CHANGED;
+    } else if (t->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK) {
+        if (t->sink_volume == volume)
+            return;
+        t->sink_volume = volume;
+        hook = PA_BLUETOOTH_HOOK_TRANSPORT_SINK_VOLUME_CHANGED;
+    } else {
+        pa_assert_not_reached();
+    }
+
+    pa_log_debug("Reporting volume change %s for %s",
+                 pa_volume_snprint(volume_str, sizeof(volume_str), volume),
+                 is_source ? "source" : "sink");
+
+    pa_hook_fire(pa_bluetooth_discovery_hook(t->device->discovery, hook), t);
 }
 
 void pa_bluetooth_transport_put(pa_bluetooth_transport *t) {
@@ -740,8 +815,8 @@ static void parse_transport_property(pa_bluetooth_transport *t, DBusMessageIter
             dbus_message_iter_get_basic(&variant_i, &value);
 
             if (pa_streq(key, "Volume")) {
-                if (t->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE)
-                    pa_bluetooth_transport_remote_volume_changed(t, value);
+                pa_volume_t volume = a2dp_gain_to_volume(value);
+                pa_bluetooth_transport_remote_volume_changed(t, volume);
             }
             break;
         }
@@ -1870,8 +1945,7 @@ const char *pa_bluetooth_profile_to_string(pa_bluetooth_profile_t profile) {
 bool pa_bluetooth_profile_should_attenuate_volume(pa_bluetooth_profile_t peer_profile) {
     switch(peer_profile) {
         case PA_BLUETOOTH_PROFILE_A2DP_SINK:
-            /* Will be set to false when A2DP absolute volume is supported */
-            return true;
+            return false;
         case PA_BLUETOOTH_PROFILE_A2DP_SOURCE:
             return true;
         case PA_BLUETOOTH_PROFILE_HFP_HF:
@@ -2025,6 +2099,8 @@ static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage
     t = pa_bluetooth_transport_new(d, sender, path, p, config, size);
     t->acquire = bluez5_transport_acquire_cb;
     t->release = bluez5_transport_release_cb;
+    t->set_sink_volume = pa_bluetooth_transport_set_sink_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 2f937e6..c712e86 100644 (file)
--- a/src/modules/bluetooth/module-bluez5-device.c
+++ b/src/modules/bluetooth/module-bluez5-device.c
@@ -1126,6 +1126,13 @@ static void sink_setup_volume_callback(pa_sink *s) {
         return;
 
     if (pa_bluetooth_profile_should_attenuate_volume(u->profile)) {
+        /* It is yet unknown how (if at all) volume is synchronized for bidirectional
+         * A2DP codecs.  Disallow attaching hooks to a pa_sink if the peer is in
+         * A2DP_SOURCE role.  This assert should be replaced with the proper logic
+         * when bidirectional codecs are implemented.
+         */
+        pa_assert(u->profile != PA_BLUETOOTH_PROFILE_A2DP_SOURCE);
+
         if (u->sink_volume_changed_slot)
             return;
 
@@ -1146,7 +1153,11 @@ static void sink_setup_volume_callback(pa_sink *s) {
         pa_sink_set_soft_volume(s, NULL);
 
         pa_sink_set_set_volume_callback(s, sink_set_volume_cb);
-        s->n_volume_steps = HSP_MAX_GAIN + 1;
+
+        if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK)
+            s->n_volume_steps = A2DP_MAX_GAIN + 1;
+        else
+            s->n_volume_steps = HSP_MAX_GAIN + 1;
     }
 }