[pulseaudio-discuss] [PATCH v5 2/4] bluetooth: separate HSP and HFP

[Rodrigo Araujo]

Hello again.

On 09/25/2017 03:30 PM, Rodrigo Araujo wrote:
> Hi.
>
> On 09/25/2017 02:37 PM, James Bottomley wrote:
>> On Sat, 2017-09-23 at 23:33 +0100, Rodrigo Araujo wrote:
>>> Hi.
>>>
>>> First, just to say that your patches are going great. Finally I can
>>> use
>>> the microphone of my HFP only headset (a version of a Bluedio T2+).
>>>
>>> So far, I've only encontered one problem: the auto_switch option of
>>> module_bluetooth_policy stops working. Dug through the code and I
>>> think
>>> you missed a few spots were you have to hangle the new
>>> headset_handsfree
>>> profile in module_bluetooth_policy.c
>>>
>>> Applying the following after applying your v5 patches fixed the issue
>>> for me, now when I start making a VOIP call the profile switches to
>>> headset_handsfree and the mic works automatically, and when the call
>>> finishes it reverts back to a2dp.
>> That's great, thanks!  Sad to say I didn't think automatic profile
>> switching worked in pulseaudio, so I hadn't thought to investigate it.
>> Did you want me to merge this into patch 2 or add it as a new patch
>> with you as the author?
>>
>> James
> Seems to me it makes sense to merge into patch 2. Would simplify things
> also, too many patches and lots of work for you guys already ;)
>
> If you want to mention my name as contributor in that patch, also go
> ahead :)
>
> Best regards.

Meanwhile I found another issue: setting the volume (gain) of the
speaker and microphone doesn't work. It's easily reproducible when using
pavucontrol.

This happens because of a clash with the HS role, and because we are a
HFP AG we are falling on the reversed logic and sending mic commands
when we want to send speaker commands, and vice-versa.

The following patch is simple and fixes that:

--- a/src/modules/bluetooth/backend-native.c	2017-09-23 23:12:35.220031951 +0100
+++ b/src/modules/bluetooth/backend-native.c	2017-09-25 22:18:29.094127139 +0100
@@ -565,7 +565,8 @@
     /* If we are in the AG role, we send a command to the head set to change
      * the speaker gain. In the HS role, source and sink are swapped, so
      * in this case we notify the AG that the microphone gain has changed */
-    if (t->profile == PA_BLUETOOTH_PROFILE_HSP_HS) {
+    if (t->profile == PA_BLUETOOTH_PROFILE_HSP_HS ||
+	t->profile == PA_BLUETOOTH_PROFILE_HFP_HF) {
         len = sprintf(buf, "\r\n+VGS=%d\r\n", gain);
         pa_log_debug("RFCOMM >> +VGS=%d", gain);
     } else {
@@ -592,7 +593,8 @@
     /* If we are in the AG role, we send a command to the head set to change
      * the microphone gain. In the HS role, source and sink are swapped, so
      * in this case we notify the AG that the speaker gain has changed */
-    if (t->profile == PA_BLUETOOTH_PROFILE_HSP_HS) {
+    if (t->profile == PA_BLUETOOTH_PROFILE_HSP_HS ||
+	t->profile == PA_BLUETOOTH_PROFILE_HFP_HF) {
         len = sprintf(buf, "\r\n+VGM=%d\r\n", gain);
         pa_log_debug("RFCOMM >> +VGM=%d", gain);
     } else {


Can you also merge that in you patch? :)

This fixes when we explicitly set the volume, but also should help when
the system tries to set it because it needs to.

However, audio quality still leaves to be desired when compared with
what is achieved using the HSP role (the voice is still too "metallic"
and with excessively poor quality). This will surely be fixed when
implementing wide band audio, but still should be better as it is now
since the audio encoding is mostly the same. I will try to look into
into too and if I find that it has to do with any of the other patches,
I will reply to the respective thread. I will also see if I can help
with implementing wide band audio, but not I'm promising anything since
this is not my area.

Best regards and keep up the good work.
Rodrigo Araujo