[pulseaudio-discuss] [PATCH v4 0/3] use bluetooth HFP in pulseaudio when available
James Bottomley
James.Bottomley at HansenPartnership.com
Thu Sep 21 17:08:35 UTC 2017
On Thu, 2017-09-21 at 19:15 +0300, Tanu Kaskinen wrote:
> On Thu, 2017-09-21 at 17:47 +0200, Georg Chini wrote:
> >
> > On 21.09.2017 16:47, James Bottomley wrote:
> > >
> > > On Thu, 2017-09-21 at 17:28 +0300, Tanu Kaskinen wrote:
> > > >
> > > > 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.
>
> Yeah, sorry, I wasn't aware of that option, as I hadn't read the
> patch. You two just seemed to be choosing between enabling or
> disabling the native HFP AG implementation by default, and I just
> wanted to say that I want it to be enabled by default, but without
> breaking "headset=auto".
>
> >
> > The default for the option would just be disabled in "auto" mode
> > and enabled in "native" mode.
>
> The option seems to be named "enable_profile_hfp_hf", which suggests
> that disabling it will disable the ofono implementation of HFP AG as
> well, and that's not what we want. Maybe rename the option to
> "enable_native_hfp_hf"?
I updated the current patch to rename the option and condition it on
the backend setting (see below. Patch would need integrating into the
series, since the option rename needs to go back into patch 2 but it
gives you the idea)
> > 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".
>
> Ok, sounds fine, with the added note that we should set
> "headset=native" by default.
OK, something like this
James
---
diff --git a/src/modules/bluetooth/backend-native.c b/src/modules/bluetooth/backend-native.c
index 16b2aa6c..c3a79d75 100644
--- a/src/modules/bluetooth/backend-native.c
+++ b/src/modules/bluetooth/backend-native.c
@@ -822,7 +822,7 @@ pa_bluetooth_backend *pa_bluetooth_native_backend_new(pa_core *c, pa_bluetooth_d
if (enable_hs_role)
profile_init(backend, PA_BLUETOOTH_PROFILE_HFP_AG);
profile_init(backend, PA_BLUETOOTH_PROFILE_HSP_HS);
- if (pa_bluetooth_discovery_get_enable_profile_hfp_hf(y))
+ if (pa_bluetooth_discovery_get_enable_native_hfp_hf(y))
profile_init(backend, PA_BLUETOOTH_PROFILE_HFP_HF);
return backend;
diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
index 6c77113e..8be8a11d 100644
--- a/src/modules/bluetooth/bluez5-util.c
+++ b/src/modules/bluetooth/bluez5-util.c
@@ -92,7 +92,7 @@ struct pa_bluetooth_discovery {
int headset_backend;
pa_bluetooth_backend *ofono_backend, *native_backend;
PA_LLIST_HEAD(pa_dbus_pending, pending);
- bool enable_profile_hfp_hf;
+ bool enable_native_hfp_hf;
};
static pa_dbus_pending* send_and_add_to_pending(pa_bluetooth_discovery *y, DBusMessage *m,
@@ -173,11 +173,11 @@ static const char *transport_state_to_string(pa_bluetooth_transport_state_t stat
}
static bool device_supports_profile(pa_bluetooth_device *device, pa_bluetooth_profile_t profile) {
- bool show_hfp, show_hsp, enable_profile_hfp_hf;
+ bool show_hfp, show_hsp, enable_native_hfp_hf;
- enable_profile_hfp_hf = pa_bluetooth_discovery_get_enable_profile_hfp_hf(device->discovery);
+ enable_native_hfp_hf = pa_bluetooth_discovery_get_enable_native_hfp_hf(device->discovery);
- if (enable_profile_hfp_hf) {
+ if (enable_native_hfp_hf) {
show_hfp = pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_HFP_HF);
show_hsp = !show_hfp;
} else {
@@ -553,12 +553,12 @@ pa_bluetooth_device* pa_bluetooth_discovery_get_device_by_path(pa_bluetooth_disc
return NULL;
}
-bool pa_bluetooth_discovery_get_enable_profile_hfp_hf(pa_bluetooth_discovery *y)
+bool pa_bluetooth_discovery_get_enable_native_hfp_hf(pa_bluetooth_discovery *y)
{
pa_assert(y);
pa_assert(PA_REFCNT_VALUE(y) > 0);
- return y->enable_profile_hfp_hf;
+ return y->enable_native_hfp_hf;
}
pa_bluetooth_device* pa_bluetooth_discovery_get_device_by_address(pa_bluetooth_discovery *y, const char *remote, const char *local) {
@@ -1754,7 +1754,7 @@ static void endpoint_done(pa_bluetooth_discovery *y, pa_bluetooth_profile_t prof
}
}
-pa_bluetooth_discovery* pa_bluetooth_discovery_get(pa_core *c, int headset_backend, bool enable_profile_hfp_hf) {
+pa_bluetooth_discovery* pa_bluetooth_discovery_get(pa_core *c, int headset_backend, bool enable_native_hfp_hf) {
pa_bluetooth_discovery *y;
DBusError err;
DBusConnection *conn;
@@ -1764,7 +1764,7 @@ pa_bluetooth_discovery* pa_bluetooth_discovery_get(pa_core *c, int headset_backe
PA_REFCNT_INIT(y);
y->core = c;
y->headset_backend = headset_backend;
- y->enable_profile_hfp_hf = enable_profile_hfp_hf;
+ y->enable_native_hfp_hf = enable_native_hfp_hf;
y->adapters = pa_hashmap_new_full(pa_idxset_string_hash_func, pa_idxset_string_compare_func, NULL,
(pa_free_cb_t) adapter_free);
y->devices = pa_hashmap_new_full(pa_idxset_string_hash_func, pa_idxset_string_compare_func, NULL,
diff --git a/src/modules/bluetooth/bluez5-util.h b/src/modules/bluetooth/bluez5-util.h
index 78c4f2fa..23f9a798 100644
--- a/src/modules/bluetooth/bluez5-util.h
+++ b/src/modules/bluetooth/bluez5-util.h
@@ -166,5 +166,5 @@ pa_bluetooth_discovery* pa_bluetooth_discovery_get(pa_core *core, int headset_ba
pa_bluetooth_discovery* pa_bluetooth_discovery_ref(pa_bluetooth_discovery *y);
void pa_bluetooth_discovery_unref(pa_bluetooth_discovery *y);
void pa_bluetooth_discovery_set_ofono_running(pa_bluetooth_discovery *y, bool is_running);
-bool pa_bluetooth_discovery_get_enable_profile_hfp_hf(pa_bluetooth_discovery *y);
+bool pa_bluetooth_discovery_get_enable_native_hfp_hf(pa_bluetooth_discovery *y);
#endif
diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
index 3d362f4b..d37ce9ce 100644
--- a/src/modules/bluetooth/module-bluez5-device.c
+++ b/src/modules/bluetooth/module-bluez5-device.c
@@ -2010,7 +2010,7 @@ static int add_card(struct userdata *u) {
pa_bluetooth_profile_t *p;
const char *uuid;
void *state;
- bool enable_profile_hfp_hf, has_both;
+ bool enable_native_hfp_hf, has_both;
pa_assert(u);
pa_assert(u->device);
@@ -2041,13 +2041,13 @@ static int add_card(struct userdata *u) {
create_card_ports(u, data.ports);
- enable_profile_hfp_hf = pa_bluetooth_discovery_get_enable_profile_hfp_hf(u->discovery);
+ enable_native_hfp_hf = pa_bluetooth_discovery_get_enable_native_hfp_hf(u->discovery);
- has_both = enable_profile_hfp_hf && pa_hashmap_get(d->uuids, PA_BLUETOOTH_UUID_HFP_HF) && pa_hashmap_get(d->uuids, PA_BLUETOOTH_UUID_HSP_HS);
+ has_both = enable_native_hfp_hf && pa_hashmap_get(d->uuids, PA_BLUETOOTH_UUID_HFP_HF) && pa_hashmap_get(d->uuids, PA_BLUETOOTH_UUID_HSP_HS);
PA_HASHMAP_FOREACH(uuid, d->uuids, state) {
pa_bluetooth_profile_t profile;
- if (!enable_profile_hfp_hf && pa_streq(uuid, PA_BLUETOOTH_UUID_HFP_HF)) {
+ if (!enable_native_hfp_hf && pa_streq(uuid, PA_BLUETOOTH_UUID_HFP_HF)) {
pa_log_info("device supports HFP but disabling profile as requested");
continue;
}
diff --git a/src/modules/bluetooth/module-bluez5-discover.c b/src/modules/bluetooth/module-bluez5-discover.c
index 52d848f0..b6e2803e 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
@@ -104,7 +104,7 @@ int pa__init(pa_module *m) {
const char *headset_str;
int headset_backend;
bool autodetect_mtu;
- bool enable_profile_hfp_hf = true;
+ bool enable_native_hfp_hf;
pa_assert(m);
@@ -125,12 +125,14 @@ int pa__init(pa_module *m) {
goto fail;
}
+ enable_native_hfp_hf = (headset_backend == HEADSET_BACKEND_NATIVE);
+
autodetect_mtu = false;
if (pa_modargs_get_value_boolean(ma, "autodetect_mtu", &autodetect_mtu) < 0) {
pa_log("Invalid boolean value for autodetect_mtu parameter");
}
- if (pa_modargs_get_value_boolean(ma, "enable_profile_hfp_hf", &enable_profile_hfp_hf) < 0) {
- pa_log("enable_profile_hfp_hf must be true or false");
+ if (pa_modargs_get_value_boolean(ma, "enable_native_hfp_hf", &enable_native_hfp_hf) < 0) {
+ pa_log("enable_native_hfp_hf must be true or false");
goto fail;
}
@@ -140,7 +142,7 @@ int pa__init(pa_module *m) {
u->autodetect_mtu = autodetect_mtu;
u->loaded_device_paths = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
- if (!(u->discovery = pa_bluetooth_discovery_get(u->core, headset_backend, enable_profile_hfp_hf)))
+ if (!(u->discovery = pa_bluetooth_discovery_get(u->core, headset_backend, enable_native_hfp_hf)))
goto fail;
u->device_connection_changed_slot =
More information about the pulseaudio-discuss
mailing list