On 21.09.2017 16:47, James Bottomley wrote:
On Thu, 2017-09-21 at 17:28 +0300, Tanu Kaskinen wrote:
On Thu, 2017-09-21 at 08:13 +0200, Georg Chini wrote:
On 21.09.2017 06:45, James Bottomley wrote:
On Thu, 2017-09-21 at 06:27 +0200, Georg Chini wrote:
On 20.09.2017 23:12, James Bottomley wrote:
On Wed, 2017-09-20 at 20:41 +0200, Georg Chini wrote:
On 20.09.2017 20:10, James Bottomley wrote:
On Wed, 2017-09-20 at 18:11 +0200, Georg Chini wrote:
On 20.09.2017 01:27, James Bottomley wrote:
This is round 4 of the initial bluetooth: separate
HSP and HFP patch.
It includes the review feedback and a global
on/off switch just in case there's a problem headset
with dual HFP/HSP but non-working HFP. This one now
includes a proper rfcomm negotiation (see patch
3). I've finally figured out a bug in the rfcomm
negotiation that was causing issues with my LG 900
headset, so I think it's now working for everything
(but testing would be welcome).
James Bottomley (3):
bluetooth: use consistent profile names
bluetooth: separate HSP and HFP
bluetooth: add correct HFP rfcomm negotiation
src/modules/bluetooth/backend-
native.c | 168
+++++++++++++++++++++---
src/modules/bluetooth/backend-
ofono.c | 4
+-
src/modules/bluetooth/bluez5-
util.c | 46
+++++--
src/modules/bluetooth/bluez5-
util.h | 10
+-
src/modules/bluetooth/module-bluetooth-policy.c
| 3
+-
src/modules/bluetooth/module-bluez5-
device.c | 102
++++++++++----
src/modules/bluetooth/module-bluez5-
discover.c | 6
+-
7 files changed, 274 insertions(+), 65
deletions(-)
Hello James,
thank you for continuing your work. Unfortunately your
patch set collides with running ofono. Currently, with
the default of "headest=auto", the native and the ofono
backends are active in parallel. This is possible
because ofono only supports HFP while PA only supports
HSP.
If PA starts supporting HFP headsets, this breaks above
assumption and ofono and PA both try to register the
corresponding HFP UUID.
To work around the problem, I suggest to disable native
HFP support if headset_backend == HEADSET_BACKEND_AUTO,
unless configured otherwise on the command line.
Actually, Pulseaudio already includes an ofono is running
check, so the enable should be set to no for backend
ofono or backend auto and ofono running, which would
enable it in the widest possible set of scenarios.
I can cook up a patch for that, hang on.
James
This does only work for the special case when PA acts in
the HSP headset role. In this case, no duplicate UUIDs are
registered and disabling the profile only needs to be done
because otherwise PA and ofono would both be listening for
incoming SCO connections.
It seems to me that pulse checks the dbus connection to ofono
before deciding on the backend (and therefore deciding what
to register), so as long as pulse finds ofono running, there
won't be any duplicate registrations.
The case here is different in that a duplicate UUID is
registered. This means, by the time PA recognizes that
ofono is running, ofono already tried to register the UUID
and failed. That's why you have to disable HFP even if
ofono is currently not running.
I don't think that can be true if ofono is already running,
can it?
Either ofono registered the HFP UUIDs way earlier or pulse
sees ofono isn't running and registers them.
I don't think requiring the ofono daemon to be running before
pulseaudio is insurmountable. On my openSUSE system, ofono
is started from systemd and pulse from user login, so this is
automatically satisfied. I think where pulse is used in
system mode, you just have to tell systemd to start ofono
first, which is certainly doable.
I don't think it is a good idea to rely on the order ofono and
pulse are started. Also. if pulse is running and has already
registered the UUID, what will happen if ofono starts? ofono
has no checks for pulse and will simply try to register the
UUID. I fear that this happens before pulse recognizes that
ofono is running.
This is an initialisation issue for how you want everything to
work. The current distro setup is ofono from systemd and pulse
from login, so I can't actually see what the problem is ...
unless there's some reason why what the distros are doing is
wrong?
If pulse is already running when you start ofono, that's caveat
emptor ... plus, as far as I can tell, it still all works because
ofono doesn't seem to get the uuid.
The point is to get a working default configuration the distros
can use. That means that we need a good way of distinguishing
the non-ofono case for backend = auto. Ofono not running seems
to be the definitive test unless there's another you can propose?
People may start/stop ofono/PA manually (or PA may crash
and be re-spawned). The main point is that currently there is
no restriction on the order you start things and your patch set
should not introduce it.
But maybe using the same approach like for the HSP headset
role of PA works if PA sees ofono before ofono tries to register
the UUID. I guess the best idea is to test what happens.
The current fly in the ointment is that for a dual HFP/HSP
device, we need to fall back to the HSP profile not simply
disable the HFP one, which requires extra jiggerypokery.
No, you don't need to fall back to HSP. The HFP connection will
be handled by ofono in that case.
Well, yes you do. There's no current easy way to use HFP_HF with
ofono, so you want the HSP profile to be exposed if it exists, so
there's an easy connection to the headset.
If ofono exposes HFP, you won't get your headset to connect to HSP,
even if the device supports both, unless you could switch off HFP
on the device side. Since PA 11.0 HFP_HF is working quite well with
ofono, though it is clumsy to set up. And if you run ofono you
might want headset support through ofono because ofono enables the
HFP control features which PA cannot support.
My suggestion was to disable HFP in "headset=auto" mode only if you
don't overwrite it on the command line. If you run ofono with
--noplugin=hfp_ag_bluez5 and enable HFP support in PA explicitly,
you will still have HFP headset support through PA if you want it,
only the default would be to go through ofono like it is now. I
think you should have the choice of three options:
1) No ofono at all, everything goes through the native backend
2) HFP_HF support through PA, HFP_AG support through ofono
3) HFP_HF and HFP_AG support through ofono
(Note about terminology: to me "HFP HF support through PA" means that
PA implements the HF role, but you seem to understand "HFP HF support
through PA" so that PA implements the AG role. In the following text
I use my definition.)
Sorry, was a bit unclear. I am referring to the role of the bluetooth
device above, not the role of PA.
Yes, that's my definition too. The patch series only establishes a
binding for HFP_HF ... HFP_AG is left exclusively for ofono.
I think we should use the native backend for the HFP AG role by
default. If the native HFP AG implementation causes a conflict with
ofono in its default configuration (which seems likely to be the
case), then "headset=auto" should not enable the native HFP AG
implementation, which then means that we should use "headset=native"
by default.
Using "headset=native" by default means that we lose HFP HF support
in the default configuration, but I don't think that's a big loss.
Setting the default to native works for me: it's basically what all
distros get today anyway because they don't install ofono by default.
That probably means we don't need the elaborate switching for HSP_AG
either, but it would become harmless, so could be removed later.
If we want to support the "HFP AG support through PA, HFP HF support
through ofono" case, then some new configuration option is needed,
and I think it would be clearest if that would be a separate patch
after this series.
This is not true, the patch supplies an option to enable/disable
the HFP implementation. Simple usage of that switch would
provide all the required functionality. The default for the option
would just be disabled in "auto" mode and enabled in "native"
mode. There is not a big code change needed, only a check
if the headset mode is "auto" or "native" and changing the
default accordingly.
The above is the essence of what I proposed to solve the
issue with headset=auto and I really don't understand why
this is causing such discussions. Leaving it as is definitely
breaks "headset=auto".
This is going to make added complexity (and added work) for users, so I
prefer the default backend approach. The proposed patch is below.
James
---
diff --git a/src/modules/bluetooth/module-bluez5-discover.c
b/src/modules/bluetooth/module-bluez5-discover.c
index 52d848f0..3a62f28c 100644
--- a/src/modules/bluetooth/module-bluez5-discover.c
+++ b/src/modules/bluetooth/module-bluez5-discover.c
@@ -93,7 +93,7 @@ static pa_hook_result_t
device_connection_changed_cb(pa_bluetooth_discovery *y,
}
#ifdef HAVE_BLUEZ_5_NATIVE_HEADSET
-const char *default_headset_backend = "auto";
+const char *default_headset_backend = "native";
#else
const char *default_headset_backend = "ofono";
#endif
_______________________________________________
I think you misunderstood Tanu's reply. Changing the default to "native"
is fine for
me, but that still does not solve the problem that "auto" does not work
any more
as expected. Tanu also says "... then "headset=auto" should not enable
the native
HFP AG implementation ..." which basically still means you have to
disable HFP
for "headset=auto". And when you disable HFP for "auto" mode, it is not
a big deal
to not disable it if this is specified on the command line. (Or as said
above just
change the default to disabled with "headset=auto". If the option is
specified, it
will overwrite the default.)
Or do you propose to remove the auto switch completely? I think that
would be a
bad thing because we have all the infrastructure to support it.
Regarding terminology, I meanwhile prefer to add the side to which the role
refers, so "PA in headset role" is the same as "bluetooth device in AG role"
and vice versa. I guess that's why it is so often confused and I forgot
to specify
what I meant above.
Regards
Georg
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
