From 8db4dff2c5caffb1e30548e095b98c27aeb68f89 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Tue, 21 Jan 2020 00:41:43 +0100 Subject: [PATCH] bluetooth: Set up hardware gain control if init volume is received late Originally written for A2DP this rework of that patch enables late-bound hardware volume control on HFP and HSP. As per the specification the headphones (where gain control for both speaker and microphone could happen in hardware on the peer) are supposed to send initial values for these before the SCO connection is created; these `AT+VG[MS]` commands are also used to determine support for it. PA uses this information in `add_{sink,source}` to attach hardware volume callbacks, _if_ it is supported. Otherwise PA performs the attenuation in software. Unfortunately headphones like the WH-1000XM3's connect to A2DP initially and only send `AT+VGS` (microphone hardware gain is not supported) _during_ SCO connection when the user switches to the HFP profile afterwards; the callbacks set up dynamically in `rfcomm_io_callback` are written after the sink and source have been created (`add_{sink,source}`), leaving them without hardware volume callbacks and with software volume when adjusted on the PA side. (The headphones can still send volume updates resulting in abrupt changes if software and peer volume differ. Furthermore the same attenuation is applied twice - once in PA software, once on the peer). To solve this problem we simply check whether the callbacks have been attached whenever the peer sends a volume change, and if not attach the callbacks to the sink/source and reset software volume. Fixes: d510ddc7f ("bluetooth: Perform software attenuation until HF/HS reports gain control") Part-of: --- src/modules/bluetooth/module-bluez5-device.c | 98 ++++++++++++++++++-- 1 file changed, 88 insertions(+), 10 deletions(-) diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c index 449586135..6067b15a5 100644 --- a/src/modules/bluetooth/module-bluez5-device.c +++ b/src/modules/bluetooth/module-bluez5-device.c @@ -986,6 +986,41 @@ static void source_set_volume_cb(pa_source *s) { pa_cvolume_set(&s->soft_volume, u->decoder_sample_spec.channels, volume); } +static void source_setup_volume_callback(pa_source *s) { + struct userdata *u; + + pa_assert(s); + + if (s->set_volume == source_set_volume_cb) + return; + + pa_assert(s->core); + + u = s->userdata; + pa_assert(u); + pa_assert(u->source == s); + pa_assert(u->transport); + + /* Remote volume control/notifications have to be supported for + * the callback to make sense, otherwise this source should + * continue performing attenuation in software without HW_VOLUME_CTL. + */ + if (!u->transport->set_source_volume) + return; + + if (pa_bluetooth_profile_should_attenuate_volume(u->profile)) { + pa_log_debug("%s: Peer supports receiving volume update notifications", s->name); + } else { + pa_log_debug("%s: Resetting software volume for hardware attenuation by peer", s->name); + + /* Reset local attenuation */ + pa_source_set_soft_volume(s, NULL); + } + + pa_source_set_set_volume_callback(s, source_set_volume_cb); + s->n_volume_steps = 16; +} + /* Run from main thread */ static int add_source(struct userdata *u) { pa_source_new_data data; @@ -1041,11 +1076,8 @@ static int add_source(struct userdata *u) { u->source->parent.process_msg = source_process_msg; u->source->set_state_in_io_thread = source_set_state_in_io_thread_cb; - if (u->transport->set_source_volume) { - pa_log_debug("Peer supports microphone gain control"); - pa_source_set_set_volume_callback(u->source, source_set_volume_cb); - u->source->n_volume_steps = 16; - } + source_setup_volume_callback(u->source); + return 0; } @@ -1164,6 +1196,41 @@ static void sink_set_volume_cb(pa_sink *s) { pa_cvolume_set(&s->soft_volume, u->encoder_sample_spec.channels, volume); } +static void sink_setup_volume_callback(pa_sink *s) { + struct userdata *u; + + pa_assert(s); + + if (s->set_volume == sink_set_volume_cb) + return; + + pa_assert(s->core); + + u = s->userdata; + pa_assert(u); + pa_assert(u->sink == s); + pa_assert(u->transport); + + /* Remote volume control/notifications have to be supported for + * the callback to make sense, otherwise this sink should + * continue performing attenuation in software without HW_VOLUME_CTL. + */ + if (!u->transport->set_sink_volume) + return; + + if (pa_bluetooth_profile_should_attenuate_volume(u->profile)) { + pa_log_debug("%s: Peer supports receiving volume update notifications", s->name); + } else { + pa_log_debug("%s: Resetting software volume for hardware attenuation by peer", s->name); + + /* Reset local attenuation */ + pa_sink_set_soft_volume(s, NULL); + } + + pa_sink_set_set_volume_callback(s, sink_set_volume_cb); + s->n_volume_steps = 16; +} + /* Run from main thread */ static int add_sink(struct userdata *u) { pa_sink_new_data data; @@ -1220,11 +1287,8 @@ static int add_sink(struct userdata *u) { u->sink->parent.process_msg = sink_process_msg; u->sink->set_state_in_io_thread = sink_set_state_in_io_thread_cb; - if (u->transport->set_sink_volume) { - pa_log_debug("Peer supports speaker gain control"); - pa_sink_set_set_volume_callback(u->sink, sink_set_volume_cb); - u->sink->n_volume_steps = 16; - } + sink_setup_volume_callback(u->sink); + return 0; } @@ -2252,6 +2316,13 @@ static pa_hook_result_t transport_sink_volume_changed_cb(pa_bluetooth_discovery volume = t->sink_volume; + if (!u->sink) { + pa_log_warn("Received peer transport volume change without connected sink"); + return PA_HOOK_OK; + } + + sink_setup_volume_callback(u->sink); + pa_cvolume_set(&v, u->encoder_sample_spec.channels, volume); if (pa_bluetooth_profile_should_attenuate_volume(t->profile)) pa_sink_set_volume(u->sink, &v, true, true); @@ -2273,6 +2344,13 @@ static pa_hook_result_t transport_source_volume_changed_cb(pa_bluetooth_discover volume = t->source_volume; + if (!u->source) { + pa_log_warn("Received peer transport volume change without connected source"); + return PA_HOOK_OK; + } + + source_setup_volume_callback(u->source); + pa_cvolume_set(&v, u->decoder_sample_spec.channels, volume); if (pa_bluetooth_profile_should_attenuate_volume(t->profile))