[pulseaudio-discuss] [PATCH v4 0/3] use bluetooth HFP in pulseaudio when available
James Bottomley
James.Bottomley at HansenPartnership.com
Thu Sep 21 14:47:47 UTC 2017
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.)
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 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
More information about the pulseaudio-discuss
mailing list