Re: [pulseaudio-discuss] [PATCH v5 1/4] bluetooth: use consistent profile names
On Sun, 2017-09-24 at 15:07 +0200, Georg Chini wrote: > On 21.09.2017 21:26, James Bottomley wrote: > > > > The PA_BLUETOOTH_PROFILE names should mirror the PA_BLUETOOTH_UUID > > names using profile_function instead of randomly made up > > names. Fix > > this with the transformation: > > > > PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT -> > > PA_BLUETOOTH_PROFILE_HSP_HS > > PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY -> > > PA_BLUETOOTH_PROFILE_HFP_AG > > > > Signed-off-by: James Bottomley > om> > > > It looks to me like the conversion is not completely correct. In > fact, > the native backend (in your notation) currently implements > > PA_BLUETOOTH_PROFILE_HSP_HS > PA_BLUETOOTH_PROFILE_HSP_AG > > while the ofono backend implements > > PA_BLUETOOTH_PROFILE_HFP_HF > PA_BLUETOOTH_PROFILE_HFP_AG So your problem is that the current ofono backend actually uses PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT which gets translated to PA_BLUETOOTH_PROFILE_HSP_HS but you think that's a bug in the current code because it should be HFP instead? > Would it collide with your later patches to distinguish between the > different roles/profiles already in your first patch? As far as I can > see, the real separation of HSP/HFP is mainly done in the native > backend. Well, yes, because this patch is designed as an obvious constant rename which is easy to review. If I start adding logic changes, it defeats the purpose of the patch being a simple rename. Before the patch that follows this, there is no equivalent of PA_BLUETOOTH_PROFILE_HFP_HF, so I don't really want to introduce one here before I add the patch that gives it meaning. I agree this profile should already have existed in the ofono backend, but it doesn't. James ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v5 2/4] bluetooth: separate HSP and HFP
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 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v5 2/4] bluetooth: separate HSP and HFP
On Sun, 2017-09-24 at 15:35 +0200, Georg Chini wrote:
> On 21.09.2017 21:27, James Bottomley wrote:
> >
> > When all headsets supported both HSP and HFP, life was good and we
> > only needed to implement HSP in the native backend. Unfortunately
> > some headsets have started supporting HFP only. Unfortuantely, we
> > can't simply switch to HFP only because that might break older HSP
> > only headsets meaning we need to support both HSP and HFP
> > separately.
> >
> > This patch separates them from a joint profile to being two
> > separate ones. The older one retains the headset_head_unit name,
> > meaning any saved parameters will still select this (keeping us
> > backward compatible). It also introduces a new headset_handsfree.
> >
> > For headsets that support both HSP and HFP, the two profiles will
> > become separately visible and selectable. This will only matter
> > once we start adding features to HFP that HSP can't support (like
> > wideband audio).
>
> This paragraph is a bit misleading because you can't select
> the profiles separately. If HFP is available, it will be used.
OK, so how about
For headsets that support both HSP and HFP, HFP becomes the default
selectable profile unless enable_native_hfp_hf is set to no. This will
only matter once we start adding features to HFP that HSP can't support
(like wideband audio).
[...]
> > static bool device_supports_profile(pa_bluetooth_device *device,
> > pa_bluetooth_profile_t profile) {
> > +bool show_hfp, show_hsp, enable_native_hfp_hf;
> > +
> > +enable_native_hfp_hf =
> > pa_bluetooth_discovery_get_enable_native_hfp_hf(device->discovery);
> > +
> > +if (enable_native_hfp_hf) {
> > +show_hfp = pa_hashmap_get(device->uuids,
> > PA_BLUETOOTH_UUID_HFP_HF);
> > +show_hsp = !show_hfp;
> > +} else {
> > +show_hfp = false;
> > +show_hsp = true;
> > +}
>
> You have to consider the case that the ofono backend is used. In that
> case show_hfp=true; show_hsp=false;
> What about the AG device roles? If we start distinguishing between
> HFP and HSP, I think we should do it everywhere. If "headset=native",
> we only support HSP_AG devices, while with "auto" and "ofono" we
> support only HFP_AG devices.
The patch only affects HFP_HF and HSP_HS it doesn't affect the HFP_AG
and HSP_AG showings. Since the patches only really affect the headset
role, I don't think they should add any changes to the AG role.
Right at the moment for the headset role, the ofono backend only wants
one thing show here and it doesn't care what (because it gets the audio
stream from ofono via a dbus socket), however it does strike me that
what we now show is more technically accurate even in the ofono case
because if HFP_HF is supported it will be used.
[...]
> > @@ -2010,9 +2041,22 @@ static int add_card(struct userdata *u) {
> >
> > create_card_ports(u, data.ports);
> >
> > +enable_native_hfp_hf =
> > pa_bluetooth_discovery_get_enable_native_hfp_hf(u->discovery);
> > +
> > +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_native_hfp_hf && pa_streq(uuid,
> > PA_BLUETOOTH_UUID_HFP_HF)) {
> > +pa_log_info("device supports HFP but disabling profile
> > as requested");
> > +continue;
> > +}
> > +
> > +if (has_both && pa_streq(uuid, PA_BLUETOOTH_UUID_HSP_HS))
> > {
> > +pa_log_info("device support HSP and HFP, selecting HFP
> > only");
> > +continue;
> > +}
> > +
>
> You need to skip this (or maybe log another message) if the
> ofono backend is used and enable_native_hfp_hf is not set.
But isn't it accurate for ofono as well? If you use the ofono backend
and you have HFP_HF support it will be selected?
> >
> > if (uuid_to_profile(uuid, &profile) < 0)
> > continue;
> >
> > diff --git a/src/modules/bluetooth/module-bluez5-discover.c
> > b/src/modules/bluetooth/module-bluez5-discover.c
> > index c535ead4..bfb361ae 100644
> > --- a/src/modules/bluetooth/module-bluez5-discover.c
> > +++ b/src/modules/bluetooth/module-bluez5-discover.c
> > @@ -104,6 +104,7 @@ int pa__init(pa
Re: [pulseaudio-discuss] [PATCH v5 1/4] bluetooth: use consistent profile names
On Mon, 2017-09-25 at 15:49 +0200, Georg Chini wrote: > On 25.09.2017 15:31, James Bottomley wrote: > > > > On Sun, 2017-09-24 at 15:07 +0200, Georg Chini wrote: > > > > > > On 21.09.2017 21:26, James Bottomley wrote: > > > > > > > > The PA_BLUETOOTH_PROFILE names should mirror the > > > > PA_BLUETOOTH_UUID names using profile_function instead of > > > > randomly made up names. Fix this with the transformation: > > > > > > > > PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT -> > > > > PA_BLUETOOTH_PROFILE_HSP_HS > > > > PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY -> > > > > PA_BLUETOOTH_PROFILE_HFP_AG > > > > > > > > Signed-off-by: James Bottomley > > > > > > > > > > > It looks to me like the conversion is not completely correct. In > > > fact, the native backend (in your notation) currently implements > > > > > > PA_BLUETOOTH_PROFILE_HSP_HS > > > PA_BLUETOOTH_PROFILE_HSP_AG > > > > > > while the ofono backend implements > > > > > > PA_BLUETOOTH_PROFILE_HFP_HF > > > PA_BLUETOOTH_PROFILE_HFP_AG > > So your problem is that the current ofono backend actually > > uses PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT which gets translated > > to PA_BLUETOOTH_PROFILE_HSP_HS but you think that's a bug in the > > current code because it should be HFP instead? > > I would not call it a bug. It just would be nice when in the end > all the names would be correct. It is a bit strange to have > PA_BLUETOOTH_PROFILE_HSP_HS in the ofono backend when > it actually implements PA_BLUETOOTH_PROFILE_HFP_HF. > The same applies for the AG role and the native backend. > The native backend implements PA_BLUETOOTH_PROFILE_HSP_AG, > not HFP. I really think, if this has to be done, it should be done as a separate patch to keep this patch simple and obvious. Even better, why don't you decide what should happen to ofono and send the patch to the list. I can keep it in this series under your authorship if necessary, that way it would be applied when this series is? > > > Would it collide with your later patches to distinguish between > > > the different roles/profiles already in your first patch? As far > > > as I can see, the real separation of HSP/HFP is mainly done in > > > the native backend. > > Well, yes, because this patch is designed as an obvious constant > > rename which is easy to review. If I start adding logic changes, > > it defeats the purpose of the patch being a simple rename. > > I understand the purpose to keep it simple. Maybe you could just > split the two AG roles in that patch and add a remark in the commit > message that you will do the separation of the HS/HF role in the > next patch? This would still require some code changes but they > would be minimal. > > > Before the patch that follows this, there is no equivalent > > of PA_BLUETOOTH_PROFILE_HFP_HF, so I don't really want to introduce > > one here before I add the patch that gives it meaning. I agree > > this profile should already have existed in the ofono backend, but > > it doesn't. > > After introducing PA_BLUETOOTH_PROFILE_HFP_HF, it should then > be also used in the ofono backend. So right at the moment, I believe from the logic that ofono will continue to work as it does today even with these patches applied. My problem with making any changes to the ofono backend is that I've never been able to get it working, so I have no means of testing it. Since you have it all working, could you test out the changes you want and send them yourself? That way there'd be no cockups. Regards, James ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3] bluetooth: Wide Band Speech implementaion for backend ofono
On Mon, 2018-08-20 at 19:19 +0530, Sathish Narasimman wrote:
> From: Sathish Narasimman
>
> mSBC-encoded streams for HFP. The Wide Band Speech(WBS) encoding and
> decoding
> is implemeted with this patch. This patch was refered from original
> patch of Joao Paula Rechi Vita and was verified with the supported
> bluetooth controller.
Which headset did you test this with? When I try it with an LG 900 I
get a huge amount of chop which shreds the audio quality. For this
headset the packet size (MTU) still needs to be set at 48 on both the
send and receive side otherwise the audio doesn't work at all.
What seems to be happening is that the LG insists on running the eSCO
links at full speed but the encoded packets only take about half the
bandwidth, so on the headset receiving side, we get about half as many
mSBC encoded packets with the rest being zero padded.
The problem occurs because on the receive side you count mSBC packets,
so the reader/writer thread you try to send 1 mSBC encoded packet for
every 1 mSBC packet you receive.
But in this send loop:
> +/* Send MTU bytes of it, if there is more it will send later
> */
> +while (u->msbc_info.ebuffer_start + u->write_link_mtu <= u-
> >msbc_info.ebuffer_end) {
> +l = pa_write(u->stream_fd,
> + u->msbc_info.ebuffer + u-
> >msbc_info.ebuffer_start,
> + u->write_link_mtu,
> + &u->stream_write_type);
> +
> +wrote = true;
> +if (l <= 0) {
> +pa_log_debug("Error while writing: l %d, errno %d",
> l, errno);
> +break;
> +}
> +
> +u->msbc_info.ebuffer_start += l;
> +if (u->msbc_info.ebuffer_start >= u-
> >msbc_info.ebuffer_end)
> +u->msbc_info.ebuffer_start = u-
> >msbc_info.ebuffer_end = 0;
> +}
> +
> +pa_memblock_release(u->write_memchunk.memblock);
> +
> +if (wrote && l < 0) {
> +
> +if (errno == EINTR)
> +/* Retry right away if we got interrupted */
> +continue;
> +
> +else if (errno == EAGAIN)
> +/* Hmm, apparently the socket was not writable, give
> up for now */
> +break;
> +
> +pa_log_error("Failed to write data to SCO socket: %s",
> pa_cstrerror(errno));
> +ret = -1;
> +break;
> +}
> +
> +if ((size_t) l != (size_t)u->write_link_mtu) {
> +pa_log_error("Wrote memory block to socket only
> partially! %llu written, wanted to write %llu.",
> +(unsigned long long) l,
> +(unsigned long long) u->write_link_mtu);
> +ret = -1;
> +break;
> +}
> +
> +u->write_index += (uint64_t) u->write_memchunk.length;
> +
> +pa_memblock_unref(u->write_memchunk.memblock);
> +pa_memchunk_reset(&u->write_memchunk);
> +
> +ret = 1;
> +break;
> +}
Because the mtu is 48 and the mSBC encode size is 60, the transmission
stops after 48 bytes, we now wait for a new mSBC packet to be received,
so we miss the next eSCO transmission window and the headset fills in
with zeros causing the packet to be chopped and the next one to be
discarded as invalid.
I suspect the only way to get mSBC to work for this type of headset is
to count actual received packets, always to transmit full 60 byte mSBC
packets in adjacent frames and to pad with zeros if we're not ready.
James
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] Adding support for bluetooth headsets that only have HFP
I recently switched from a JBL Flip 2 which supports both HSP and HFP
to an Ultimate Ears Boom 2 which is HFP only. Needless to say this
currently means that the Ultimate Ears fails to work with pulseaudio
because we only support HSP via the native backend. The first thing to
note is that we can easily get HFP running in pulseaudio via a quick
hack to switch support from HSP to HFP (see patch below) because if you
don't negotiate protocols on HFP it looks pretty much like HSP for the
audio stream. Unfortunately, doing this would break half the headsets
out there which only support HSP, so the hack is for demo purposes
only.
I think the fix is to expose HFP and HSP separately as different
pulseaudio profiles. This would also allow negotiating useful HFP
features, like wideband audio, which HSP doesn't support. It would
also mean that people currently using HSP would continue, even if they
had HFP and that headsets which support both will show both in the PA
configuration possibilities. If people are OK with this approach, I
can produce a patch set.
James
---
diff --git a/src/modules/bluetooth/backend-native.c
b/src/modules/bluetooth/backend-native.c
index 86376c0..33feb98 100644
--- a/src/modules/bluetooth/backend-native.c
+++ b/src/modules/bluetooth/backend-native.c
@@ -438,7 +438,7 @@ static void profile_init(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
switch (profile) {
case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
object_name = HSP_AG_PROFILE;
-uuid = PA_BLUETOOTH_UUID_HSP_AG;
+uuid = PA_BLUETOOTH_UUID_HFP_AG;
break;
default:
pa_assert_not_reached();
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] bluetooth: separate HSP and HFP
When all headsets supported both HSP and HFP, life was good and we
only needed to implement HSP in the native backend. Unfortunately
some headsets have started supporting HFP only. Unfortuantely, we
can't simply switch to HFP only because that might break older HSP
only headsets meaning we need to support both HSP and HFP separately.
This patch separates them from a joint profile to being two separate
ones. The older one retains the headset_head_unit name, meaning any
saved parameters will still select this (keeping us backward
compatible). It also introduces a new headset_handsfree.
For headsets that support both HSP and HFP, the two profiles will
become separately visible and selectable. This will only matter once
we start adding features to HFP that HSP can't support (like wideband
audio).
Signed-off-by:
diff --git a/src/modules/bluetooth/backend-native.c
b/src/modules/bluetooth/backend-native.c
index cf88126..1c71572 100644
--- a/src/modules/bluetooth/backend-native.c
+++ b/src/modules/bluetooth/backend-native.c
@@ -59,6 +59,7 @@ struct transport_rfcomm {
#define BLUEZ_PROFILE_INTERFACE BLUEZ_SERVICE ".Profile1"
#define HSP_AG_PROFILE "/Profile/HSPAGProfile"
+#define HFP_AG_PROFILE "/Profile/HFPAGProfile"
#define PROFILE_INTROSPECT_XML \
DBUS_INTROSPECT_1_0_XML_DOCTYPE_DECL_NODE \
@@ -324,6 +325,7 @@ static DBusMessage *profile_new_connection(DBusConnection
*conn, DBusMessage *m,
DBusMessageIter arg_i;
char *pathfd;
struct transport_rfcomm *trfc;
+bool is_hfp;
if (!dbus_message_iter_init(m, &arg_i) ||
!pa_streq(dbus_message_get_signature(m), "oha{sv}")) {
pa_log_error("Invalid signature found in NewConnection");
@@ -331,7 +333,13 @@ static DBusMessage *profile_new_connection(DBusConnection
*conn, DBusMessage *m,
}
handler = dbus_message_get_path(m);
-pa_assert(pa_streq(handler, HSP_AG_PROFILE));
+
+if (pa_streq(handler, HFP_AG_PROFILE))
+ is_hfp = true;
+else if (pa_streq(handler, HSP_AG_PROFILE))
+ is_hfp = false;
+else
+ pa_assert_not_reached();
pa_assert(dbus_message_iter_get_arg_type(&arg_i) == DBUS_TYPE_OBJECT_PATH);
dbus_message_iter_get_basic(&arg_i, &path);
@@ -351,7 +359,11 @@ static DBusMessage *profile_new_connection(DBusConnection
*conn, DBusMessage *m,
sender = dbus_message_get_sender(m);
-p = PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT;
+if (is_hfp)
+ p = PA_BLUETOOTH_PROFILE_HEADSET_HFP;
+else
+ p = PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT;
+
pathfd = pa_sprintf_malloc ("%s/fd%d", path, fd);
t = pa_bluetooth_transport_new(d, sender, pathfd, p, NULL, 0);
pa_xfree(pathfd);
@@ -403,7 +415,8 @@ static DBusHandlerResult profile_handler(DBusConnection *c,
DBusMessage *m, void
pa_log_debug("dbus: path=%s, interface=%s, member=%s", path, interface,
member);
-if (!pa_streq(path, HSP_AG_PROFILE))
+if (!pa_streq(path, HSP_AG_PROFILE)
+ && !pa_streq(path, HFP_AG_PROFILE))
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
if (dbus_message_is_method_call(m, "org.freedesktop.DBus.Introspectable",
"Introspect")) {
@@ -442,6 +455,10 @@ static void profile_init(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
object_name = HSP_AG_PROFILE;
uuid = PA_BLUETOOTH_UUID_HSP_AG;
break;
+case PA_BLUETOOTH_PROFILE_HEADSET_HFP:
+ object_name = HFP_AG_PROFILE;
+uuid = PA_BLUETOOTH_UUID_HFP_AG;
+break;
default:
pa_assert_not_reached();
break;
@@ -458,6 +475,9 @@ static void profile_done(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection),
HSP_AG_PROFILE);
break;
+case PA_BLUETOOTH_PROFILE_HEADSET_HFP:
+
dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection),
HFP_AG_PROFILE);
+break;
default:
pa_assert_not_reached();
break;
@@ -484,6 +504,7 @@ pa_bluetooth_backend
*pa_bluetooth_native_backend_new(pa_core *c, pa_bluetooth_d
backend->discovery = y;
profile_init(backend, PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT);
+profile_init(backend, PA_BLUETOOTH_PROFILE_HEADSET_HFP);
return backend;
}
@@ -494,6 +515,7 @@ void pa_bluetooth_native_backend_free(pa_bluetooth_backend
*backend) {
pa_dbus_free_pending_list(&backend->pending);
profile_done(backend, PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT);
+profile_done(backend, PA_BLUETOOTH_PROFILE_HEADSET_HFP);
pa_dbus_connection_unref(backend->connection);
diff --git a/src/modules/bluetooth/bluez5-util.c
b/src/modules/bluetooth/bluez5-util.c
index 7d63f35..da7184e 100644
--- a/src/modules/bluetooth/bluez5-util.c
+
Re: [pulseaudio-discuss] Adding support for bluetooth headsets that only have HFP
On Fri, 2016-08-19 at 20:24 +0300, Tanu Kaskinen wrote: > On Thu, 2016-08-18 at 07:51 -0700, James Bottomley wrote: > > I recently switched from a JBL Flip 2 which supports both HSP and > > HFP > > to an Ultimate Ears Boom 2 which is HFP only. Needless to say this > > currently means that the Ultimate Ears fails to work with > > pulseaudio because we only support HSP via the native backend. The > > first thing to note is that we can easily get HFP running in > > pulseaudio via a quick hack to switch support from HSP to HFP (see > > patch below) because if you don't negotiate protocols on HFP it > > looks pretty much like HSP for the audio stream. Unfortunately, > > doing this would break half the headsets out there which only > > support HSP, so the hack is for demo purposes only. > > > > I think the fix is to expose HFP and HSP separately as different > > pulseaudio profiles. This would also allow negotiating useful HFP > > features, like wideband audio, which HSP doesn't support. It would > > also mean that people currently using HSP would continue, even if > > they had HFP and that headsets which support both will show both in > > the PA configuration possibilities. If people are OK with this > > approach, I can produce a patch set. > > I was under the impression that supporting HFP would have to be much > more complex than that. If that one-line hack really is sufficient > for getting audio moving, I think it makes sense to add HFP support > to the native backend. I see you already went ahead and produced a > more complete patch :) I'll have a look shortly. I still think something has to be done in the negotiation. The spec definitely implies there's a mandatory rfcomm negotiation before you establish audio links. I think I've found a device that's more unforgiving with this (the Boom seems fine to establish audio links with no negotiation), so I'll complete the patch series when I get this working. The patch is still more of a demo of "is it OK that the interface appears like this" than a "this works for all known devices apply it". James ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] bluetooth: separate HSP and HFP
On Fri, 2016-08-19 at 22:33 +0300, Tanu Kaskinen wrote:
> On Thu, 2016-08-18 at 11:14 -0700, James Bottomley wrote:
> > When all headsets supported both HSP and HFP, life was good and we
> > only needed to implement HSP in the native backend. Unfortunately
> > some headsets have started supporting HFP only. Unfortuantely, we
> > can't simply switch to HFP only because that might break older HSP
> > only headsets meaning we need to support both HSP and HFP
> > separately.
> >
> > This patch separates them from a joint profile to being two
> > separate ones. The older one retains the headset_head_unit name,
> > meaning any saved parameters will still select this (keeping us
> > backward compatible). It also introduces a new headset_handsfree.
> >
> > For headsets that support both HSP and HFP, the two profiles will
> > become separately visible and selectable. This will only matter
> > once we start adding features to HFP that HSP can't support (like
> > wideband audio).
> >
> > Signed-off-by:
>
> Thanks, this looks pretty neat. It's hard to tell if any changes are
> missing, but the changes that are there look good, although please
> use spaces for all indenting.
Heh, kernel coding style built into emacs. I assume there's some sort
of tab to space converter somewhere I can run this through.
> One thing that is missing, though, is updating module-bluetooth
> -policy.c. It references "headset_head_unit" in one place, and that
> code should take the new profile into account too.
in profile_available_hook_callback() will do.
> Did you test if volume control works in both directions? I don't know
> if HSP and HFP volume handling is identical. The observed volume
> should change if you set it with some volume control GUI, and the
> slider in the GUI should move when you set the volume from the
> headset's own buttons.
Yes, the HFP uses the same AT commands as the HSP for volume and
microphone control. However, the boom does have a weirdness in that
it's top volume is 14 which is directly contrary to the standard which
says it must be 15, so using the remote volume controls I can only get
up to 92% (I assume it's a manufacturer off by one reading the standard
error).
> > @@ -47,6 +47,7 @@ typedef enum profile {
> > PA_BLUETOOTH_PROFILE_A2DP_SINK,
> > PA_BLUETOOTH_PROFILE_A2DP_SOURCE,
> > PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT,
> > +PA_BLUETOOTH_PROFILE_HEADSET_HFP,
> > PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY,
>
> Since the enum effectively lists profile+role combinations, this
> naming scheme would make more sense to me:
>
> PA_BLUETOOTH_PROFILE_HSP_HS,
> PA_BLUETOOTH_PROFILE_HFP_HF,
> PA_BLUETOOTH_PROFILE_HFP_AG
>
> (and PA_BLUETOOTH_PROFILE_HSP_AG when the patch for that finally gets
> merged). That's what the UUID constants use too.
OK, will update.
> Similarly, I'd be tempted to use "hfp_hf" as the profile name, but
> maybe "headset_handsfree" is better, since it's more in line with the
> other profile names.
Yes, I felt we were a bit stuck with that one.
James
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] bluetooth: separate HSP and HFP
On Fri, 2016-08-19 at 22:33 +0300, Tanu Kaskinen wrote: > On Thu, 2016-08-18 at 11:14 -0700, James Bottomley wrote: > > When all headsets supported both HSP and HFP, life was good and we > > only needed to implement HSP in the native backend. Unfortunately > > some headsets have started supporting HFP only. Unfortuantely, we > > can't simply switch to HFP only because that might break older HSP > > only headsets meaning we need to support both HSP and HFP > > separately. > > > > This patch separates them from a joint profile to being two > > separate ones. The older one retains the headset_head_unit name, > > meaning any saved parameters will still select this (keeping us > > backward compatible). It also introduces a new headset_handsfree. > > > > For headsets that support both HSP and HFP, the two profiles will > > become separately visible and selectable. This will only matter > > once we start adding features to HFP that HSP can't support (like > > wideband audio). > > > > Signed-off-by: > > Thanks, this looks pretty neat. It's hard to tell if any changes are > missing, but the changes that are there look good, although please > use spaces for all indenting. One thing that is missing, though, is > updating module-bluetooth-policy.c. It references "headset_head_unit" > in one place, and that code should take the new profile into account > too. I actually ran into a problem with the approach. It's not really a pulseaudio problem per-se, it's a bluetooth/bluez one: bluez can't talk simultaneously to both HFP and HSP; once you connect to one "headset" profile at once and the connection to the other will fail. Right at the moment pulseaudio connects to each available profile so the audio switching can be fast. The problem is that the first connected profile for devices which support dual HFP/HSP will always be HFP (dictated by the ordering of the defaults array in bluez:src/profile.c) and the HSP connection will subsequently fail, so every dual HSP/HFP device would become HFP only with this patch (i.e. you won't be able to switch to HSP even if you want to). There are a couple of ways of fixing this: 1. Accept the limitation and disable HSP if the device supports both. The drawback would be any device that badly implements HFP but works currently with HSP (not sure if such devices exist). If it becomes a problem, HFP could be disabled with an option. 2. Don't register with bluez until the device is selected. This means all the negotiation and setup has to occur after the PA profile is selected which would lead to a noticeable slowdown in audio switching. Is there any preference (1. is certainly easier for me)? James ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] bluetooth: separate HSP and HFP
On Sat, 2016-08-20 at 21:03 +0300, Tanu Kaskinen wrote: > On Sat, 2016-08-20 at 10:28 -0700, James Bottomley wrote: > > I actually ran into a problem with the approach. It's not really a > > pulseaudio problem per-se, it's a bluetooth/bluez one: bluez can't > > talk simultaneously to both HFP and HSP; once you connect to one > > "headset" profile at once and the connection to the other will > > fail. Right at the moment pulseaudio connects to each available > > profile so the audio switching can be fast. The problem is that > > the first connected profile for devices which support dual HFP/HSP > > will always be HFP (dictated by the ordering of the defaults array > > in bluez:src/profile.c) and the HSP connection will subsequently > > fail, so every dual HSP/HFP device would become HFP only with this > > patch (i.e. you won't be able to switch to HSP even if you want > > to). > > > > There are a couple of ways of fixing this: > > > >1. Accept the limitation and disable HSP if the device supports > > both. > >The drawback would be any device that badly implements HFP > > but > > works currently with HSP (not sure if such devices exist). > > If it > > becomes a problem, HFP could be disabled with an option. > >2. Don't register with bluez until the device is selected. This > > means > > all the negotiation and setup has to occur after the PA > > profile is > > selected which would lead to a noticeable slowdown in audio > > switching. > > > > Is there any preference (1. is certainly easier for me)? > > To clarify, is the problem that bluez has some arbitrary limitation > that it's not possible to connect both HFP and HSP? AFAIK bluetooth > doesn't allow two simultaneous audio streams, but I guess that's not > the problem here? No, the problem is establishing the rfcomm link. I *think* it's actually an intrinsic limitation of the device. > Would it be possible to fix this in bluez? (I'm not asking you to > fix it, I'm just trying to understand the situation - my old > understanding was that bluez would support connecting both > profiles.) I get a connection refused error directly from the device when I try to establish a HSP rfcomm link after establishing a HFP one, so I don't think bluez can work around this (except by managing the switching, I suppose, which would be complex). > I'm not sure how bluez advertises the supported profiles to other > devices, but if bluez doesn't advertise e.g. HSP support unless > pulseaudio has first registered the profile, then I don't think > option 2 is viable. Discovery gives us all the supported uuids and we then decide which ones to register profiles for, so we know as soon as the device is plugged in all the uuids for the profiles it supports. > Also, pulseaudio doesn't manage when profiles are connected or > disconnected, and starting to do that (and potentially stomping on > other components' toes) doesn't sound like a good idea. Yes, that's more my worry. > An option for disabling HFP seems much more feasible. OK, I'll code up option 1. James ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] bluetooth: separate HSP and HFP
On Sat, 2016-08-20 at 21:34 +0300, Tanu Kaskinen wrote: > On Sat, 2016-08-20 at 11:18 -0700, James Bottomley wrote: > > On Sat, 2016-08-20 at 21:03 +0300, Tanu Kaskinen wrote: > > > To clarify, is the problem that bluez has some arbitrary > > > limitation that it's not possible to connect both HFP and HSP? > > > AFAIK bluetooth doesn't allow two simultaneous audio streams, but > > > I guess that's not the problem here? > > > > No, the problem is establishing the rfcomm link. I *think* it's > > actually an intrinsic limitation of the device. > > Mmh... I should spend some time with the bluetooth specs to get a > better understanding of this stuff. Rfcomm gets mentioned often, but > I don't really have any idea what it is beyond "some kind of > communication channel". That's really all it is. It behaves like a serial link and speaks a weird extension of the modem AT protocol. Every bluetooth profile has one and spec requires you establish this link first and then bring up the rest of the supported stuff for the profile. However, Marcel would know better. I'm not really a bluetooth expert, I just get to play one every time my desktop won't do what I want with my devices ... > > > Would it be possible to fix this in bluez? (I'm not asking you > > > to fix it, I'm just trying to understand the situation - my old > > > understanding was that bluez would support connecting both > > > profiles.) > > > > I get a connection refused error directly from the device when I > > try to establish a HSP rfcomm link after establishing a HFP one, so > > I don't think bluez can work around this (except by managing the > > switching, I suppose, which would be complex). > > In my world, profiles can be "connected" or "disconnected". Is > "connected without rfcomm link" a valid thing? Not according to any profile specs I've read (although I haven't read them all). > > > I'm not sure how bluez advertises the supported profiles to other > > > devices, but if bluez doesn't advertise e.g. HSP support unless > > > pulseaudio has first registered the profile, then I don't think > > > option 2 is viable. > > > > Discovery gives us all the supported uuids and we then decide which > > ones to register profiles for, so we know as soon as the device is > > plugged in all the uuids for the profiles it supports. > > I meant advertising the local capabilities to remote devices. A > laptop with bluez and pulseaudio installed should advertise audio > support to other bluetooth devices, but I'd guess that won't happen > if pulseaudio doesn't register the audio profiles first. So the way I'd do it is to register one PA transport for each profile we see a uuid for but not register the connection over dbus to the bluez.org endpoint until the transport is activated. It's the registration of the bluez endpoint that triggers bluez to connect to the bluetooth profile. This really is contrary to the way it works today because we expect an idle transport to be able to connect instantly. James ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] bluetooth: separate HSP and HFP
On Sat, 2016-08-20 at 21:34 +0300, Tanu Kaskinen wrote:
> On Sat, 2016-08-20 at 11:18 -0700, James Bottomley wrote:
On Sat, 2016-08-20 at 21:03 +0300, Tanu Kaskinen wrote:
> > > Also, pulseaudio doesn't manage when profiles are connected or
> > > disconnected, and starting to do that (and potentially stomping
> > > on other components' toes) doesn't sound like a good idea.
> >
> > Yes, that's more my worry.
> >
> > > An option for disabling HFP seems much more feasible.
> >
> > OK, I'll code up option 1.
>
> Ok, sounds good.
This is basically what it looks like: it's a global disable HFP
registration flag. I can't do anything more fine grained because if
the HFP profile ever registers, bluez will use it to connect every dual
HFP/HSP device.
James
---
diff --git a/src/modules/bluetooth/backend-native.c
b/src/modules/bluetooth/backend-native.c
index c311b5f..de6be0c 100644
--- a/src/modules/bluetooth/backend-native.c
+++ b/src/modules/bluetooth/backend-native.c
@@ -673,7 +673,8 @@ pa_bluetooth_backend
*pa_bluetooth_native_backend_new(pa_core *c, pa_bluetooth_d
backend->discovery = y;
profile_init(backend, PA_BLUETOOTH_PROFILE_HSP_HS);
-profile_init(backend, PA_BLUETOOTH_PROFILE_HFP_HF);
+if (!disable_profile_hfp)
+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 8149297..b38730a 100644
--- a/src/modules/bluetooth/bluez5-util.c
+++ b/src/modules/bluetooth/bluez5-util.c
@@ -76,6 +76,8 @@
" " \
""
+bool disable_profile_hfp = false;
+
struct pa_bluetooth_discovery {
PA_REFCNT_DECLARE;
diff --git a/src/modules/bluetooth/bluez5-util.h
b/src/modules/bluetooth/bluez5-util.h
index 6cd4f28..027728e 100644
--- a/src/modules/bluetooth/bluez5-util.h
+++ b/src/modules/bluetooth/bluez5-util.h
@@ -126,6 +126,8 @@ struct pa_bluetooth_adapter {
bool valid;
};
+extern bool disable_profile_hfp; /* globally disable HFP */
+
#ifdef HAVE_BLUEZ_5_OFONO_HEADSET
pa_bluetooth_backend *pa_bluetooth_ofono_backend_new(pa_core *c,
pa_bluetooth_discovery *y);
void pa_bluetooth_ofono_backend_free(pa_bluetooth_backend *b);
diff --git a/src/modules/bluetooth/module-bluetooth-discover.c
b/src/modules/bluetooth/module-bluetooth-discover.c
index fe8aec9..b5a0693 100644
--- a/src/modules/bluetooth/module-bluetooth-discover.c
+++ b/src/modules/bluetooth/module-bluetooth-discover.c
@@ -32,7 +32,8 @@ PA_MODULE_DESCRIPTION("Detect available Bluetooth daemon and
load the correspond
PA_MODULE_VERSION(PACKAGE_VERSION);
PA_MODULE_LOAD_ONCE(true);
PA_MODULE_USAGE(
-"headset=ofono|native|auto (bluez 5 only)"
+"headset=ofono|native|auto (bluez 5 only) "
+"disable_profile_hfp= (bluez 5 only)"
);
struct userdata {
diff --git a/src/modules/bluetooth/module-bluez5-device.c
b/src/modules/bluetooth/module-bluez5-device.c
index e00210a..995dac0 100644
--- a/src/modules/bluetooth/module-bluez5-device.c
+++ b/src/modules/bluetooth/module-bluez5-device.c
@@ -54,7 +54,10 @@ PA_MODULE_AUTHOR("João Paulo Rechi Vita");
PA_MODULE_DESCRIPTION("BlueZ 5 Bluetooth audio sink and source");
PA_MODULE_VERSION(PACKAGE_VERSION);
PA_MODULE_LOAD_ONCE(false);
-PA_MODULE_USAGE("path=");
+PA_MODULE_USAGE(
+"path= "
+"disable_profile_hfp="
+);
#define MAX_PLAYBACK_CATCH_UP_USEC (100 * PA_USEC_PER_MSEC)
#define FIXED_LATENCY_PLAYBACK_A2DP (25 * PA_USEC_PER_MSEC)
@@ -68,6 +71,7 @@ PA_MODULE_USAGE("path=");
static const char* const valid_modargs[] = {
"path",
+"disable_profile_hfp",
NULL
};
@@ -1965,6 +1969,7 @@ static int add_card(struct userdata *u) {
pa_bluetooth_profile_t *p;
const char *uuid;
void *state;
+bool has_both;
pa_assert(u);
pa_assert(u->device);
@@ -1995,9 +2000,20 @@ static int add_card(struct userdata *u) {
create_card_ports(u, data.ports);
+has_both = !disable_profile_hfp && 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 (disable_profile_hfp && pa_streq(uuid, PA_BLUETOOTH_UUID_HFP_HF)) {
+pa_log_info("device supports HFP but disabling profile as
requested");
+ continue;
+ }
+
+ if (has_both && pa_streq(uuid, PA_BLUETOOTH_UUID_HSP_HS)) {
+ pa_log_info("device support HSP and HFP, selecting HFP only");
+ continue;
+ }
+
if (uuid_to_profile(uuid, &profile) < 0)
continue
[pulseaudio-discuss] [PATCH v2 0/3] use bluetooth HFP in pulseaudio when available
This is round 2 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). 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| 176 +++--- src/modules/bluetooth/backend-ofono.c | 2 +- src/modules/bluetooth/bluez5-util.c | 22 ++- src/modules/bluetooth/bluez5-util.h | 9 +- src/modules/bluetooth/module-bluetooth-discover.c | 3 +- src/modules/bluetooth/module-bluetooth-policy.c | 3 +- src/modules/bluetooth/module-bluez5-device.c | 103 + src/modules/bluetooth/module-bluez5-discover.c| 13 +- src/pulsecore/core-util.h | 7 + 9 files changed, 280 insertions(+), 58 deletions(-) ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH v2 1/3] bluetooth: use consistent profile names
The PA_BLUETOOTH_PROFILE names should mirror the PA_BLUETOOTH_UUID
names using profile_function instead of randomly made up names. Fix
this with the transformation:
PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT -> PA_BLUETOOTH_PROFILE_HSP_HS
PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY -> PA_BLUETOOTH_PROFILE_HFP_AG
Signed-off-by: James Bottomley
---
src/modules/bluetooth/backend-native.c | 10 +++---
src/modules/bluetooth/backend-ofono.c| 2 +-
src/modules/bluetooth/bluez5-util.c | 8 ++---
src/modules/bluetooth/bluez5-util.h | 4 +--
src/modules/bluetooth/module-bluez5-device.c | 46 ++--
5 files changed, 35 insertions(+), 35 deletions(-)
diff --git a/src/modules/bluetooth/backend-native.c
b/src/modules/bluetooth/backend-native.c
index cf88126..005363b 100644
--- a/src/modules/bluetooth/backend-native.c
+++ b/src/modules/bluetooth/backend-native.c
@@ -351,7 +351,7 @@ static DBusMessage *profile_new_connection(DBusConnection
*conn, DBusMessage *m,
sender = dbus_message_get_sender(m);
-p = PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT;
+p = PA_BLUETOOTH_PROFILE_HSP_HS;
pathfd = pa_sprintf_malloc ("%s/fd%d", path, fd);
t = pa_bluetooth_transport_new(d, sender, pathfd, p, NULL, 0);
pa_xfree(pathfd);
@@ -438,7 +438,7 @@ static void profile_init(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
pa_assert(b);
switch (profile) {
-case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+case PA_BLUETOOTH_PROFILE_HSP_HS:
object_name = HSP_AG_PROFILE;
uuid = PA_BLUETOOTH_UUID_HSP_AG;
break;
@@ -455,7 +455,7 @@ static void profile_done(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
pa_assert(b);
switch (profile) {
-case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+case PA_BLUETOOTH_PROFILE_HSP_HS:
dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection),
HSP_AG_PROFILE);
break;
default:
@@ -483,7 +483,7 @@ pa_bluetooth_backend
*pa_bluetooth_native_backend_new(pa_core *c, pa_bluetooth_d
backend->discovery = y;
-profile_init(backend, PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT);
+profile_init(backend, PA_BLUETOOTH_PROFILE_HSP_HS);
return backend;
}
@@ -493,7 +493,7 @@ void pa_bluetooth_native_backend_free(pa_bluetooth_backend
*backend) {
pa_dbus_free_pending_list(&backend->pending);
-profile_done(backend, PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT);
+profile_done(backend, PA_BLUETOOTH_PROFILE_HSP_HS);
pa_dbus_connection_unref(backend->connection);
diff --git a/src/modules/bluetooth/backend-ofono.c
b/src/modules/bluetooth/backend-ofono.c
index 755df9e..cab5b72 100644
--- a/src/modules/bluetooth/backend-ofono.c
+++ b/src/modules/bluetooth/backend-ofono.c
@@ -253,7 +253,7 @@ static void hf_audio_agent_card_found(pa_bluetooth_backend
*backend, const char
goto fail;
}
-card->transport = pa_bluetooth_transport_new(d, backend->ofono_bus_id,
path, PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY, NULL, 0);
+card->transport = pa_bluetooth_transport_new(d, backend->ofono_bus_id,
path, PA_BLUETOOTH_PROFILE_HFP_AG, NULL, 0);
card->transport->acquire = hf_audio_agent_transport_acquire;
card->transport->release = hf_audio_agent_transport_release;
card->transport->userdata = card;
diff --git a/src/modules/bluetooth/bluez5-util.c
b/src/modules/bluetooth/bluez5-util.c
index 7d63f35..959d447 100644
--- a/src/modules/bluetooth/bluez5-util.c
+++ b/src/modules/bluetooth/bluez5-util.c
@@ -174,10 +174,10 @@ static bool device_supports_profile(pa_bluetooth_device
*device, pa_bluetooth_pr
return !!pa_hashmap_get(device->uuids,
PA_BLUETOOTH_UUID_A2DP_SINK);
case PA_BLUETOOTH_PROFILE_A2DP_SOURCE:
return !!pa_hashmap_get(device->uuids,
PA_BLUETOOTH_UUID_A2DP_SOURCE);
-case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+case PA_BLUETOOTH_PROFILE_HSP_HS:
return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_HSP_HS)
|| !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_HFP_HF);
-case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
+case PA_BLUETOOTH_PROFILE_HFP_AG:
return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_HSP_AG)
|| !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_HFP_AG);
case PA_BLUETOOTH_PROFILE_OFF:
@@ -1262,9 +1262,9 @@ const char
*pa_bluetooth_profile_to_string(pa_bluetooth_profile_t profile) {
return "a2dp_sink";
case PA_BLUETOOTH_PROFILE_A2DP_SOURCE:
return "a2dp_source";
-case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+case PA_BLUETOOTH_PROFILE_HSP_HS:
return "headset_head_unit";
-c
[pulseaudio-discuss] [PATCH v2 2/3] bluetooth: separate HSP and HFP
When all headsets supported both HSP and HFP, life was good and we
only needed to implement HSP in the native backend. Unfortunately
some headsets have started supporting HFP only. Unfortuantely, we
can't simply switch to HFP only because that might break older HSP
only headsets meaning we need to support both HSP and HFP separately.
This patch separates them from a joint profile to being two separate
ones. The older one retains the headset_head_unit name, meaning any
saved parameters will still select this (keeping us backward
compatible). It also introduces a new headset_handsfree.
For headsets that support both HSP and HFP, the two profiles will
become separately visible and selectable. This will only matter once
we start adding features to HFP that HSP can't support (like wideband
audio).
Signed-off-by:
---
v2:
- fold in review feedback
- add global disable option for not registering HFP
---
src/modules/bluetooth/backend-native.c| 29 -
src/modules/bluetooth/bluez5-util.c | 9 ++-
src/modules/bluetooth/bluez5-util.h | 3 +
src/modules/bluetooth/module-bluetooth-discover.c | 3 +-
src/modules/bluetooth/module-bluetooth-policy.c | 3 +-
src/modules/bluetooth/module-bluez5-device.c | 73 +++
src/modules/bluetooth/module-bluez5-discover.c| 13 +++-
7 files changed, 113 insertions(+), 20 deletions(-)
diff --git a/src/modules/bluetooth/backend-native.c
b/src/modules/bluetooth/backend-native.c
index 005363b..f7453a4 100644
--- a/src/modules/bluetooth/backend-native.c
+++ b/src/modules/bluetooth/backend-native.c
@@ -59,6 +59,7 @@ struct transport_rfcomm {
#define BLUEZ_PROFILE_INTERFACE BLUEZ_SERVICE ".Profile1"
#define HSP_AG_PROFILE "/Profile/HSPAGProfile"
+#define HFP_AG_PROFILE "/Profile/HFPAGProfile"
#define PROFILE_INTROSPECT_XML \
DBUS_INTROSPECT_1_0_XML_DOCTYPE_DECL_NODE \
@@ -324,6 +325,7 @@ static DBusMessage *profile_new_connection(DBusConnection
*conn, DBusMessage *m,
DBusMessageIter arg_i;
char *pathfd;
struct transport_rfcomm *trfc;
+bool is_hfp;
if (!dbus_message_iter_init(m, &arg_i) ||
!pa_streq(dbus_message_get_signature(m), "oha{sv}")) {
pa_log_error("Invalid signature found in NewConnection");
@@ -331,7 +333,13 @@ static DBusMessage *profile_new_connection(DBusConnection
*conn, DBusMessage *m,
}
handler = dbus_message_get_path(m);
-pa_assert(pa_streq(handler, HSP_AG_PROFILE));
+
+if (pa_streq(handler, HFP_AG_PROFILE))
+is_hfp = true;
+else if (pa_streq(handler, HSP_AG_PROFILE))
+is_hfp = false;
+else
+pa_assert_not_reached();
pa_assert(dbus_message_iter_get_arg_type(&arg_i) == DBUS_TYPE_OBJECT_PATH);
dbus_message_iter_get_basic(&arg_i, &path);
@@ -351,7 +359,11 @@ static DBusMessage *profile_new_connection(DBusConnection
*conn, DBusMessage *m,
sender = dbus_message_get_sender(m);
-p = PA_BLUETOOTH_PROFILE_HSP_HS;
+if (is_hfp)
+p = PA_BLUETOOTH_PROFILE_HFP_HF;
+else
+p = PA_BLUETOOTH_PROFILE_HSP_HS;
+
pathfd = pa_sprintf_malloc ("%s/fd%d", path, fd);
t = pa_bluetooth_transport_new(d, sender, pathfd, p, NULL, 0);
pa_xfree(pathfd);
@@ -403,7 +415,8 @@ static DBusHandlerResult profile_handler(DBusConnection *c,
DBusMessage *m, void
pa_log_debug("dbus: path=%s, interface=%s, member=%s", path, interface,
member);
-if (!pa_streq(path, HSP_AG_PROFILE))
+if (!pa_streq(path, HSP_AG_PROFILE)
+&& !pa_streq(path, HFP_AG_PROFILE))
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
if (dbus_message_is_method_call(m, "org.freedesktop.DBus.Introspectable",
"Introspect")) {
@@ -442,6 +455,10 @@ static void profile_init(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
object_name = HSP_AG_PROFILE;
uuid = PA_BLUETOOTH_UUID_HSP_AG;
break;
+case PA_BLUETOOTH_PROFILE_HFP_HF:
+object_name = HFP_AG_PROFILE;
+uuid = PA_BLUETOOTH_UUID_HFP_AG;
+break;
default:
pa_assert_not_reached();
break;
@@ -458,6 +475,9 @@ static void profile_done(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
case PA_BLUETOOTH_PROFILE_HSP_HS:
dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection),
HSP_AG_PROFILE);
break;
+case PA_BLUETOOTH_PROFILE_HFP_HF:
+
dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection),
HFP_AG_PROFILE);
+break;
default:
pa_assert_not_reached();
break;
@@ -484,6 +504,8 @@ pa_bluetooth_backend
*pa_bluetooth_native_backend_new(pa_core *c, pa_bluetooth_d
backend->discovery = y;
profile_init(backend, PA_BLUETOOTH_PROFILE_HSP_HS);
+if (!disable_profile_hfp)
+pr
[pulseaudio-discuss] [PATCH v2 3/3] bluetooth: add correct HFP rfcomm negotiation
HFP 1.6 requires a stateful negotiation of AT commands. The prior
version got away with initialising HFP simply by replying 'OK' to
every negotiation attempt. This one actually tries to parse the state
and make sure the negotiation occurs correctly
Signed-off-by: James Bottomley
---
src/modules/bluetooth/backend-native.c | 139 ++---
src/modules/bluetooth/bluez5-util.c| 5 +-
src/modules/bluetooth/bluez5-util.h| 2 +-
src/pulsecore/core-util.h | 7 ++
4 files changed, 141 insertions(+), 12 deletions(-)
diff --git a/src/modules/bluetooth/backend-native.c
b/src/modules/bluetooth/backend-native.c
index f7453a4..c7a4aa1 100644
--- a/src/modules/bluetooth/backend-native.c
+++ b/src/modules/bluetooth/backend-native.c
@@ -50,6 +50,45 @@ struct transport_rfcomm {
pa_mainloop_api *mainloop;
};
+struct hfp_config {
+uint32_t capabilities;
+int state;
+};
+
+/*
+ * the separate hansfree headset (HF) and Audio Gateway (AG) features
+ */
+enum hfp_hf_features {
+HFP_HF_EC_NR = 0,
+HFP_HF_CALL_WAITING = 1,
+HFP_HF_CLI = 2,
+HFP_HF_VR = 3,
+HFP_HF_RVOL = 4,
+HFP_HF_ESTATUS = 5,
+HFP_HF_ECALL = 6,
+HFP_HF_CODECS = 7,
+};
+
+enum hfp_ag_features {
+HFP_AG_THREE_WAY = 0,
+HFP_AG_EC_NR = 1,
+HFP_AG_VR = 2,
+HFP_AG_RING = 3,
+HFP_AG_NUM_TAG = 4,
+HFP_AG_REJECT = 5,
+HFP_AG_ESTATUS = 6,
+HFP_AG_ECALL = 7,
+HFP_AG_EERR = 8,
+HFP_AG_CODECS = 9,
+};
+
+/* gateway features we support */
+static uint32_t hfp_features =
+/* LG HBS900 reboots if it doesn't see this */
+(1 << HFP_AG_THREE_WAY) |
+/* HFP 1.6 requires this */
+ (1 << HFP_AG_ESTATUS );
+
#define BLUEZ_SERVICE "org.bluez"
#define BLUEZ_MEDIA_TRANSPORT_INTERFACE BLUEZ_SERVICE ".MediaTransport1"
@@ -101,6 +140,27 @@ static pa_dbus_pending*
send_and_add_to_pending(pa_bluetooth_backend *backend, D
return p;
}
+static void rfcomm_write(int fd, const char *str)
+{
+size_t len;
+char buf[512];
+
+pa_log_debug("RFCOMM >> %s", str);
+sprintf(buf, "\r\n%s\r\n", str);
+len = write(fd, buf, strlen(buf));
+
+if (len != strlen(buf))
+pa_log_error("RFCOMM write error: %s", pa_cstrerror(errno));
+}
+
+static void hfp_send_features(int fd)
+{
+char buf[512];
+
+sprintf(buf, "+BRSF: %d", hfp_features);
+rfcomm_write(fd, buf);
+}
+
static int bluez5_sco_acquire_cb(pa_bluetooth_transport *t, bool optional,
size_t *imtu, size_t *omtu) {
pa_bluetooth_device *d = t->device;
struct sockaddr_sco addr;
@@ -216,6 +276,58 @@ static void register_profile(pa_bluetooth_backend *b,
const char *profile, const
send_and_add_to_pending(b, m, register_profile_reply, pa_xstrdup(profile));
}
+static void transport_put(pa_bluetooth_transport *t)
+{
+pa_bluetooth_transport_put(t);
+
+pa_log_debug("Transport %s available for profile %s", t->path,
pa_bluetooth_profile_to_string(t->profile));
+}
+
+static int hfp_rfcomm_handle(int fd, pa_bluetooth_transport *t, const char
*buf)
+{
+struct hfp_config *c = t->config;
+int val;
+
+/* stateful negotiation */
+if (c->state == 0 && sscanf(buf, "AT+BRSF=%d", &val) == 1) {
+ c->capabilities = val;
+ pa_log_info("HFP capabilities returns 0x%x", val);
+ hfp_send_features(fd);
+ c->state = 1;
+ return 0;
+} else if (c->state == 1 && pa_strcont("AT+CIND=?", buf)) {
+ /* we declare minimal indicators */
+ rfcomm_write(fd, "+CIND: (\"call\",(0,1))");
+ c->state = 2;
+ return 0;
+} else if (c->state == 2 && pa_strcont("AT+CIND?", buf)) {
+ rfcomm_write(fd, "+CIND: 0");
+ c->state = 3;
+ return 0;
+} else if (c->state == 3 && pa_strcont("AT+CMER=", buf)) {
+ rfcomm_write(fd, "OK");
+ c->state = 4;
+ transport_put(t);
+ return 1;
+}
+
+
+if (c->state != 4)
+ goto error;
+
+/* misc things to handle in fully connected state */
+
+if (pa_strcont("AT+BTRH?", buf)) {
+ return 0;
+} else if (pa_strcont("AT+CHLD=?", buf)) {
+ return 0;
+}
+
+ error:
+rfcomm_write(fd, "ERROR");
+return 1;
+}
+
static void rfcomm_io_callback(pa_mainloop_api *io, pa_io_event *e, int fd,
pa_io_event_flags_t events, void *userdata) {
pa_bluetooth_transport *t = userdata;
@@ -246,16 +358,18 @@ static void rfcomm_io_callback(pa_mainloop_api *io,
pa_io_event *e, int fd, pa_i
} else if (sscanf(buf, "AT+VGM=%d", &gain) == 1) {
t->microphone_gain = gain;
pa_hook_fire(pa_bluetooth_discovery_hook(t->device->discovery,
PA_BLUE
Re: [pulseaudio-discuss] [PATCH v2 2/3] bluetooth: separate HSP and HFP
On Sun, 2016-08-21 at 21:03 +0300, Tanu Kaskinen wrote:
> On Sun, 2016-08-21 at 15:19 +0300, Tanu Kaskinen wrote:
> > On Sat, 2016-08-20 at 15:03 -0700, James Bottomley wrote:
> > > static const char* const valid_modargs[] = {
> > > "path",
> > > +"disable_profile_hfp",
> >
> > We try to avoid negative option names on the basis that double
> > negatives are not nice ("disable_foo=no"), so I'd prefer
> > "enable_profile_hfp".
>
> Hmm, maybe that's not a good name, if the option only affects HFP HF
> support, and not HFP AG. Would "enable_profile_hfp_hf" work? It's
> pretty awful from user-friendliness perspective, but I have trouble
> coming up with better names.
As long as I can put a comment saying it was your suggestion ...
However, yes, I can do it. I don't really see it as a huge problem.
descriptive options aren't bad just because they're long (unless
they're 100s of characters long).
James
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 3/3] bluetooth: add correct HFP rfcomm negotiation
On Sun, 2016-08-21 at 23:01 +0300, Tanu Kaskinen wrote:
> On Sat, 2016-08-20 at 15:05 -0700, James Bottomley wrote:
> > +static int hfp_rfcomm_handle(int fd, pa_bluetooth_transport *t,
> > const char *buf)
> > +{
> > +struct hfp_config *c = t->config;
> > +int val;
> > +
> > +/* stateful negotiation */
> > +if (c->state == 0 && sscanf(buf, "AT+BRSF=%d", &val) == 1) {
> > + c->capabilities = val;
> > + pa_log_info("HFP capabilities returns 0x%x", val);
> > + hfp_send_features(fd);
> > + c->state = 1;
> > + return 0;
> > +} else if (c->state == 1 && pa_strcont("AT+CIND=?", buf)) {
> > + /* we declare minimal indicators */
> > + rfcomm_write(fd, "+CIND: (\"call\",(0,1))");
> > + c->state = 2;
> > + return 0;
> > +} else if (c->state == 2 && pa_strcont("AT+CIND?", buf)) {
> > + rfcomm_write(fd, "+CIND: 0");
> > + c->state = 3;
> > + return 0;
> > +} else if (c->state == 3 && pa_strcont("AT+CMER=", buf)) {
> > + rfcomm_write(fd, "OK");
> > + c->state = 4;
> > + transport_put(t);
> > + return 1;
> > +}
> > +
> > +
> > +if (c->state != 4)
> > + goto error;
> > +
> > +/* misc things to handle in fully connected state */
> > +
> > +if (pa_strcont("AT+BTRH?", buf)) {
> > + return 0;
> > +} else if (pa_strcont("AT+CHLD=?", buf)) {
> > + return 0;
> > +}
>
> The commands are queries, so shouldn't they receive some kind of a
> reply (in addition to just "OK")?
the ones that need it do: AT+BRSF= expects a +BRSF reply and the
AT+CIND ones expect +CIND: replies, but the rest are informational and
can simply be acknowledged with an OK.
> Now that I've read some of the HFP spec, I believe there are other
> commands (possibly many of them) that we may encounter too after the
> initial setup, for example AT+CMER. How did these two commands end up
> being handled specially? Did your headset not work without this code?
We handle AT+CMER. The v1.6 version of the standard contains a diagram
which shows the minimum dialog you need. It's basically BRSF, two
CINDS and a CMER. Once the CMER is sent and replied to, you can
establish audio. The other end may optionally send a AT+CHLD=? but we
can just reply OK to that. I actually theorise that just replying OK
to this entire negotiation sequence is fine because it indicates a
minimal but functional audio gateway, so in theory, the negotiation is
an optional best practice because the original headset code will just
reply OK on its own.
> It seems likely that other headsets will have other requirements,
> which is why I don't feel comfortable switching the majority of
> headsets from HSP to this partial HFP implementation by default.
Not if they're spec compliant and they have to follow the standards
otherwise they wouldn't talk to phones. Fortunately bluez was used in
early androids and it still has a headset model (in
blues/android/handsfree*) so I used that to see what android does. The
trick for us is initialising the bare minimum so we don't get complex
call stuff from the headset.
> One possibility for avoiding manual configuration in HFP-only use
> cases would be to add some logic to automatically register the HFP
> profile when we notice that a HFP-only headset is being used.
It would still obscure HSP, though, if you're switching headsets
without reloading the modules.
> Tabs are still appearing here.
Sorry, I think I forgot to run the patches through a tab checker.
> > @@ -246,16 +358,18 @@ static void
> > rfcomm_io_callback(pa_mainloop_api *io, pa_io_event *e, int fd,
> > pa_i
> > } else if (sscanf(buf, "AT+VGM=%d", &gain) == 1) {
> >t->microphone_gain = gain;
> >pa_hook_fire(pa_bluetooth_discovery_hook(t->device
> > ->discovery, PA_BLUETOOTH_HOOK_TRANSPORT_MICROPHONE_GAIN_CHANGED),
> > t);
> > -}
> > -
> > -pa_log_debug("RFCOMM >> OK");
> > -
> > -len = write(fd, "\r\nOK\r\n", 6);
> > +} else if (t->config) {
>
> The assumption here is that non-NULL config means that we're using
> HFP rather than HSP, right? I think that should be clarified with a
> comment, or in some other way.
I'll add a comment.
> > + switch (hfp_rfcomm_handle(fd, t, buf)) {
> > + case -1:
> > + goto fail;
>
>
Re: [pulseaudio-discuss] [PATCH v2 3/3] bluetooth: add correct HFP rfcomm negotiation
On Sun, 2016-08-21 at 16:16 -0400, James Bottomley wrote: > The v1.6 version of the standard contains a diagram > which shows the minimum dialog you need. For this standard: https://www.bluetooth.org/docman/handlers/downloaddoc.ashx?doc_id=238193 it's figure 4.1 (service level connection establishment) James ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 2/3] bluetooth: separate HSP and HFP
On Sun, 2016-08-21 at 15:19 +0300, Tanu Kaskinen wrote:
> One thing that needs fixing is the profile waiting logic - we wait
> until all supported profiles are connected (or until a timeout
> expires) before loading module-bluez5-device. Since we will now
> connect only HFP or HSP, it doesn't make sense to wait for both of
> them getting connected. The waiting logic is implemented in
> pa_bluetooth_transport_set_state().
This actually seems to be broken today. I unwound all my patches and I
still see the debug message
Aug 21 19:23:28 jarvis pulseaudio[12479]: [pulseaudio] bluez5-util.c:
Timeout expired, and device /org/bluez/hci0/dev_B8_AD_3E_8E_DE_EF still
has disconnected profiles:
meaning the timer expired even though all profiles were connected. I
think the bug is that pa_bluetooth_transport_set_state() starts the
timer when we go from 0->1 disconnected profiles, but after that, the
test (old_any_connected !=
pa_bluetooth_device_any_transport_connected(t->device) is always true
and so the timer never gets stopped if we're waiting for more than one
transport connection.
I think the fix is this.
James
---
diff --git a/src/modules/bluetooth/bluez5-util.c
b/src/modules/bluetooth/bluez5-util.c
index 7d63f35..e8a0b3d 100644
--- a/src/modules/bluetooth/bluez5-util.c
+++ b/src/modules/bluetooth/bluez5-util.c
@@ -259,6 +259,7 @@ static void
device_start_waiting_for_profiles(pa_bluetooth_device *device) {
void pa_bluetooth_transport_set_state(pa_bluetooth_transport *t,
pa_bluetooth_transport_state_t state) {
bool old_any_connected;
+unsigned n_disconnected_profiles;
pa_assert(t);
@@ -274,8 +275,9 @@ void
pa_bluetooth_transport_set_state(pa_bluetooth_transport *t, pa_bluetooth_tr
pa_hook_fire(&t->device->discovery->hooks[PA_BLUETOOTH_HOOK_TRANSPORT_STATE_CHANGED],
t);
+n_disconnected_profiles = device_count_disconnected_profiles(t->device);
+
if (old_any_connected !=
pa_bluetooth_device_any_transport_connected(t->device)) {
-unsigned n_disconnected_profiles;
/* If there are profiles that are expected to get connected soon (based
* on the UUID list), we wait for a bit before announcing the new
@@ -285,8 +287,6 @@ void
pa_bluetooth_transport_set_state(pa_bluetooth_transport *t, pa_bluetooth_tr
* which would prevent module-card-restore from restoring the initial
* profile properly. */
-n_disconnected_profiles =
device_count_disconnected_profiles(t->device);
-
if (n_disconnected_profiles == 0)
device_stop_waiting_for_profiles(t->device);
@@ -294,6 +294,9 @@ void
pa_bluetooth_transport_set_state(pa_bluetooth_transport *t, pa_bluetooth_tr
device_start_waiting_for_profiles(t->device);
else
pa_hook_fire(&t->device->discovery->hooks[PA_BLUETOOTH_HOOK_DEVICE_CONNECTION_CHANGED],
t->device);
+} else if (n_disconnected_profiles == 0) {
+device_stop_waiting_for_profiles(t->device);
+
pa_hook_fire(&t->device->discovery->hooks[PA_BLUETOOTH_HOOK_DEVICE_CONNECTION_CHANGED],
t->device);
}
}
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 3/3] bluetooth: add correct HFP rfcomm negotiation
On Mon, 2016-08-22 at 15:12 +0300, Tanu Kaskinen wrote:
> On Sun, 2016-08-21 at 16:16 -0400, James Bottomley wrote:
> > On Sun, 2016-08-21 at 23:01 +0300, Tanu Kaskinen wrote:
> > >
> > > On Sat, 2016-08-20 at 15:05 -0700, James Bottomley wrote:
> > > >
> > > > +static int hfp_rfcomm_handle(int fd, pa_bluetooth_transport
> > > > *t,
> > > > const char *buf)
> > > > +{
> > > > +struct hfp_config *c = t->config;
> > > > +int val;
> > > > +
> > > > +/* stateful negotiation */
> > > > +if (c->state == 0 && sscanf(buf, "AT+BRSF=%d", &val) == 1)
> > > > {
> > > > + c->capabilities = val;
> > > > + pa_log_info("HFP capabilities returns 0x%x", val);
> > > > + hfp_send_features(fd);
> > > > + c->state = 1;
> > > > + return 0;
> > > > +} else if (c->state == 1 && pa_strcont("AT+CIND=?", buf))
> > > > {
> > > > + /* we declare minimal indicators */
> > > > + rfcomm_write(fd, "+CIND: (\"call\",(0,1))");
> > > > + c->state = 2;
> > > > + return 0;
> > > > +} else if (c->state == 2 && pa_strcont("AT+CIND?", buf)) {
> > > > + rfcomm_write(fd, "+CIND: 0");
> > > > + c->state = 3;
> > > > + return 0;
> > > > +} else if (c->state == 3 && pa_strcont("AT+CMER=", buf)) {
> > > > + rfcomm_write(fd, "OK");
> > > > + c->state = 4;
> > > > + transport_put(t);
> > > > + return 1;
> > > > +}
> > > > +
> > > > +
> > > > +if (c->state != 4)
> > > > + goto error;
> > > > +
> > > > +/* misc things to handle in fully connected state */
> > > > +
> > > > +if (pa_strcont("AT+BTRH?", buf)) {
> > > > + return 0;
> > > > +} else if (pa_strcont("AT+CHLD=?", buf)) {
> > > > + return 0;
> > > > +}
> > >
> > > The commands are queries, so shouldn't they receive some kind of
> > > a reply (in addition to just "OK")?
> >
> > the ones that need it do: AT+BRSF= expects a +BRSF reply and the
> > AT+CIND ones expect +CIND: replies, but the rest are informational
> > and can simply be acknowledged with an OK.
>
> I was referring specifically to AT+BTRH?
BTRH is the headset asking about on-hold statement. A simple OK means
nothing is on-hold (that's 4.29.1)
> and AT+CHLD=?.
This one is a bit weird: in theory the response should be call hold
states, but we didn't show any call held indicators, so none (i.e. just
respond OK) is the correct response. I actually experimented with
showing some and the headset crashed.
> Is it really correct to reply "OK" without providing the answer to
> the query first?
A plain OK is the correct response to a lot of these per spec.
> Responding "ERROR" like we do for all other unsupported commands
> would seem more logical. (AT+CHLD=? is part of the initial setup
> sequence, though, so I think we should support it by responding
> "+CHLD: ...".)
Not unless we support held call indicators. I could add them, but I
was trying to make the initial state as simple as possible.
> > > Now that I've read some of the HFP spec, I believe there are
> > > other commands (possibly many of them) that we may encounter too
> > > after the initial setup, for example AT+CMER. How did these two
> > > commands end up being handled specially? Did your headset not
> > > work without this code?
> >
> > We handle AT+CMER. The v1.6 version of the standard contains a
> > diagram which shows the minimum dialog you need. It's basically
> > BRSF, two CINDS and a CMER. Once the CMER is sent and replied to,
> > you can establish audio. The other end may optionally send a
> > AT+CHLD=? but we can just reply OK to that. I actually theorise
> > that just replying OK to this entire negotiation sequence is fine
> > because it indicates a minimal but functional audio gateway, so in
> > theory, the negotiation is an optional best practice because the
> > original headset code will just reply OK on its own.
>
> We don't handle AT+CMER, except as part of the initial setup
&g
Re: [pulseaudio-discuss] issue with audio over headset_head_unit
On Thu, 2016-08-25 at 16:35 +0530, Arun Raghavan wrote: > On Thu, 25 Aug 2016, at 03:10 PM, Anand Nekkunti wrote: > > Hi > > I am working audio over Bluetooth . I am able to pair and > > connect > > with > > all speakers(a2dp and HSD ). > > but audio coming only speakers which supports a2dp protocol . I am > > not > > able to understand why speakers which has HSD protocol not > > working(no > > error > > in pulseaudio log) . Do I need configure anything for this or is > > there > > any > > way forcefully set protocol as a2dp > > > > Board :ARM imx6 > > pulseaudio : 6.0 > > Bluez :5 > > > > Working Speaker : > > > > index: 2 > > name: > > driver: > > flags: HARDWARE DECIBEL_VOLUME LATENCY > > state: IDLE > > suspend cause: > > priority: 9030 > > volume: front-left: 65536 / 100% / 0.00 dB, front-right: 65536 / > > 100% / > > 0.00 dB > >balance 0.00 > > base volume: 65536 / 100% / 0.00 dB > > volume steps: 65537 > > muted: no > > current latency: 28.67 ms > > max request: 3 KiB > > max rewind: 0 KiB > > monitor source: 4 > > sample spec: s16le 2ch 44100Hz > > channel map: front-left,front-right > > Stereo > > used by: 0 > > linked by: 0 > > fixed latency: 42.41 ms > > card: 2 > > module: 21 > > properties: > > bluetooth.protocol = "a2dp_sink" > > device.description = "Mi Bluetooth Speaker" > > device.string = "00:9E:C8:11:69:42" > > device.api = "bluez" > > device.class = "sound" > > device.bus = "bluetooth" > > device.form_factor = "portable" > > bluez.path = "/org/bluez/hci0/dev_00_9E_C8_11_69_42" > > bluez.class = "0x24041c" > > bluez.alias = "Mi Bluetooth Speaker " > > device.icon_name = "multimedia-player-bluetooth" > > ports: > > portable-output: Portable (priority 0, latency offset 0 usec, > > available: > > yes) > > properties: > > active port: > > > > > > > > Not Working Speaker: > >index: 2 > > name: > > driver: > > flags: HARDWARE HW_VOLUME_CTRL LATENCY > > state: RUNNING > > suspend cause: > > priority: 9030 > > volume: mono: 43691 / 67% > >balance 0.00 > > base volume: 65536 / 100% > > volume steps: 16 > > muted: no > > current latency: 31.00 ms > > max request: 0 KiB > > max rewind: 0 KiB > > monitor source: 4 > > sample spec: s16le 1ch 8000Hz > > channel map: mono > > Mono > > used by: 1 > > linked by: 1 > > fixed latency: 128.00 ms > > card: 2 > > module: 21 > > properties: > > bluetooth.protocol = "headset_head_unit" > > device.intended_roles = "phone" > > device.description = "Bose AE2w 01.01.00" > > device.string = "00:0C:8A:67:14:4E" > > device.api = "bluez" > > device.class = "sound" > > device.bus = "bluetooth" > > device.form_factor = "headset" > > bluez.path = "/org/bluez/hci0/dev_00_0C_8A_67_14_4E" > > bluez.class = "0x240404" > > bluez.alias = "Bose AE2w 01.01.00" > > device.icon_name = "audio-headset-bluetooth" > > ports: > > headset-output: Headset (priority 0, latency offset 0 usec, > > available: > > unknown) > > properties: > > active port: > > Are you using the native backend (HSP) or oFono (HFP) here? Might > help to look at system logs, or get verbose output from PulseAudio > and BlueZ. There seems to be a misunderstanding here (I just went around the block with Marcel on it). The fact is that the backend-ofono can only talk to a phone, where it emulates HFP, it can't talk to an actual headset. Meaning that if you have a headset trying to talk to your computer, you are ipso facto using the native backend. The confusion seems to be because at one time there were patches to backend-ofono to talk to headsets, but they depended on non-upstream bluez parts and they were never merged. James ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 3/3] bluetooth: add correct HFP rfcomm negotiation
On Wed, 2016-08-24 at 16:17 +0300, Tanu Kaskinen wrote: > On Tue, 2016-08-23 at 08:37 -0400, James Bottomley wrote: > > On Mon, 2016-08-22 at 15:12 +0300, Tanu Kaskinen wrote: > > > and AT+CHLD=?. > > > > This one is a bit weird: in theory the response should be call hold > > states, but we didn't show any call held indicators, so none (i.e. > > just respond OK) is the correct response. I actually experimented > > with showing some and the headset crashed. > > Ok. I couldn't find a statement from the HFP spec about how "we don't > support any of the +CHLD values" should be reported, but judging from > the 3GPP 27.007 spec, it looks like the response to AT+CHLD=? is > optional. Actually, if you've found spec support from only responding OK, I'll rework the code to do that rather than making up a pointless indicator. The slight problem is what happens to the mandatory AT+CHLD? negotiation because I can see some headsets not sending that if we don't support any indicators, but I think I can cope with that in the sequence. > AT+CIND=? is pretty similar in that we'd rather not pretend to > support any indicators, but the 3GPP spec doesn't mark the reply as > optional in that case, and the list of indicators has to have at > least one entry. However, "if MT is not currently reachable, +CME > ERROR: is returned" - maybe returning +CME ERROR for AT+CIND=? > would make more sense? I'd really rather make up an indicator than return error, because I bet error in the initialisation sequence isn't usual for headsets. > > > > > Now that I've read some of the HFP spec, I believe there are > > > > > other commands (possibly many of them) that we may encounter > > > > > too after the initial setup, for example AT+CMER. How did > > > > > these two commands end up being handled specially? Did your > > > > > headset not work without this code? > > > > > > > > We handle AT+CMER. The v1.6 version of the standard contains a > > > > diagram which shows the minimum dialog you need. It's > > > > basically BRSF, two CINDS and a CMER. Once the CMER is sent > > > > and replied to, you can establish audio. The other end may > > > > optionally send a AT+CHLD=? but we can just reply OK to that. > > > > I actually theorise that just replying OK to this entire > > > > negotiation sequence is fine because it indicates a minimal > > > > but functional audio gateway, so in theory, the negotiation is > > > > an optional best practice because the original headset code > > > > will just reply OK on its own. > > > > > > We don't handle AT+CMER, except as part of the initial setup > > > sequence. Further AT+CMER commands get "ERROR" as reply. > > > > We should only ever get one. Actually 3,0,0,1 to enable event > > reporting. > > Why should we only ever get one? Why should the headset never send > 3,0,0,0 to disable event reporting? (If you're going to change the > code to respond OK to everything, this question becomes moot, but > still I'm somewhat interested in how we can make assumptions about > what commands headsets will or will not use.) The latest code will just respond OK to this ... it doesn't matter because we never send event indicators, so whether the headset wants them on or off is irrelevant to us. > > > What makes AT+BTRH? special in that we should reply "OK" to it > > > and "ERROR" to all other commands? > > > > Actually, I think we should just reply OK to all commands (a bit > > like HFP). There's no harm and we don't alter any state. > > That sounds good, if it removes the need for special handling for a > headset-specific set of commands. Yes, see below ... I'll post the complete set as a v3, but this is what I currently have. > > > > > One possibility for avoiding manual configuration in HFP-only > > > > > use cases would be to add some logic to automatically > > > > > register the HFP profile when we notice that a HFP-only > > > > > headset is being used. > > > > > > > > It would still obscure HSP, though, if you're switching > > > > headsets without reloading the modules. > > > > > > True. However, that would be a problem only if > > > > > > - the user actively uses multiple headsets > > > - one of the headsets supports both HSP and HFP, and one supports > > > only HFP >
[pulseaudio-discuss] [PATCH v4 1/3] bluetooth: use consistent profile names
The PA_BLUETOOTH_PROFILE names should mirror the PA_BLUETOOTH_UUID
names using profile_function instead of randomly made up names. Fix
this with the transformation:
PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT -> PA_BLUETOOTH_PROFILE_HSP_HS
PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY -> PA_BLUETOOTH_PROFILE_HFP_AG
Signed-off-by: James Bottomley
---
v4: update for PA 11.0
---
src/modules/bluetooth/backend-native.c | 28 +++
src/modules/bluetooth/backend-ofono.c| 4 +--
src/modules/bluetooth/bluez5-util.c | 10 +++---
src/modules/bluetooth/bluez5-util.h | 4 +--
src/modules/bluetooth/module-bluez5-device.c | 54 ++--
5 files changed, 50 insertions(+), 50 deletions(-)
diff --git a/src/modules/bluetooth/backend-native.c
b/src/modules/bluetooth/backend-native.c
index 0f0104d8..f2009bfd 100644
--- a/src/modules/bluetooth/backend-native.c
+++ b/src/modules/bluetooth/backend-native.c
@@ -449,7 +449,7 @@ static void set_speaker_gain(pa_bluetooth_transport *t,
uint16_t gain) {
/* 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_HEADSET_HEAD_UNIT) {
+if (t->profile == PA_BLUETOOTH_PROFILE_HSP_HS) {
len = sprintf(buf, "\r\n+VGS=%d\r\n", gain);
pa_log_debug("RFCOMM >> +VGS=%d", gain);
} else {
@@ -476,7 +476,7 @@ static void set_microphone_gain(pa_bluetooth_transport *t,
uint16_t gain) {
/* 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_HEADSET_HEAD_UNIT) {
+if (t->profile == PA_BLUETOOTH_PROFILE_HSP_HS) {
len = sprintf(buf, "\r\n+VGM=%d\r\n", gain);
pa_log_debug("RFCOMM >> +VGM=%d", gain);
} else {
@@ -509,9 +509,9 @@ static DBusMessage *profile_new_connection(DBusConnection
*conn, DBusMessage *m,
handler = dbus_message_get_path(m);
if (pa_streq(handler, HSP_AG_PROFILE)) {
-p = PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT;
+p = PA_BLUETOOTH_PROFILE_HSP_HS;
} else if (pa_streq(handler, HSP_HS_PROFILE)) {
-p = PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY;
+p = PA_BLUETOOTH_PROFILE_HFP_AG;
} else {
pa_log_error("Invalid handler");
goto fail;
@@ -626,11 +626,11 @@ static void profile_init(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
pa_assert(b);
switch (profile) {
-case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+case PA_BLUETOOTH_PROFILE_HSP_HS:
object_name = HSP_AG_PROFILE;
uuid = PA_BLUETOOTH_UUID_HSP_AG;
break;
-case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
+case PA_BLUETOOTH_PROFILE_HFP_AG:
object_name = HSP_HS_PROFILE;
uuid = PA_BLUETOOTH_UUID_HSP_HS;
break;
@@ -647,10 +647,10 @@ static void profile_done(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
pa_assert(b);
switch (profile) {
-case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+case PA_BLUETOOTH_PROFILE_HSP_HS:
dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection),
HSP_AG_PROFILE);
break;
-case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
+case PA_BLUETOOTH_PROFILE_HFP_AG:
dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection),
HSP_HS_PROFILE);
break;
default:
@@ -665,9 +665,9 @@ void
pa_bluetooth_native_backend_enable_hs_role(pa_bluetooth_backend *native_bac
return;
if (enable_hs_role)
- profile_init(native_backend,
PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY);
+ profile_init(native_backend, PA_BLUETOOTH_PROFILE_HFP_AG);
else
- profile_done(native_backend,
PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY);
+ profile_done(native_backend, PA_BLUETOOTH_PROFILE_HFP_AG);
native_backend->enable_hs_role = enable_hs_role;
}
@@ -693,8 +693,8 @@ pa_bluetooth_backend
*pa_bluetooth_native_backend_new(pa_core *c, pa_bluetooth_d
backend->enable_hs_role = enable_hs_role;
if (enable_hs_role)
- profile_init(backend, PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY);
-profile_init(backend, PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT);
+ profile_init(backend, PA_BLUETOOTH_PROFILE_HFP_AG);
+profile_init(backend, PA_BLUETOOTH_PROFILE_HSP_HS);
return backend;
}
@@ -705,8 +705,8 @@ void pa_bluetooth_native_backend_free(pa_bluetooth_backend
*backend) {
pa_dbus_free_pendi
[pulseaudio-discuss] [PATCH v4 0/3] use bluetooth HFP in pulseaudio when available
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(-) -- 2.12.3 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH v4 2/3] bluetooth: separate HSP and HFP
When all headsets supported both HSP and HFP, life was good and we
only needed to implement HSP in the native backend. Unfortunately
some headsets have started supporting HFP only. Unfortuantely, we
can't simply switch to HFP only because that might break older HSP
only headsets meaning we need to support both HSP and HFP separately.
This patch separates them from a joint profile to being two separate
ones. The older one retains the headset_head_unit name, meaning any
saved parameters will still select this (keeping us backward
compatible). It also introduces a new headset_handsfree.
For headsets that support both HSP and HFP, the two profiles will
become separately visible and selectable. This will only matter once
we start adding features to HFP that HSP can't support (like wideband
audio).
Signed-off-by:
---
v4: Update for PA 11.0
v2:
- fold in review feedback
- add global disable option for not registering HFP
v3:
- change parameter to enable_profile_hfp
- update device_supports_profile to be aware of hfp/hsp exclusivity
- change parameter to enable_profile_hfp_hf
---
src/modules/bluetooth/backend-native.c | 16 ++-
src/modules/bluetooth/bluez5-util.c | 31 +++--
src/modules/bluetooth/bluez5-util.h | 4 +-
src/modules/bluetooth/module-bluetooth-policy.c | 3 +-
src/modules/bluetooth/module-bluez5-device.c| 60 +
src/modules/bluetooth/module-bluez5-discover.c | 6 ++-
6 files changed, 105 insertions(+), 15 deletions(-)
diff --git a/src/modules/bluetooth/backend-native.c
b/src/modules/bluetooth/backend-native.c
index f2009bfd..b8a8f706 100644
--- a/src/modules/bluetooth/backend-native.c
+++ b/src/modules/bluetooth/backend-native.c
@@ -62,6 +62,7 @@ struct transport_data {
#define BLUEZ_PROFILE_INTERFACE BLUEZ_SERVICE ".Profile1"
#define HSP_AG_PROFILE "/Profile/HSPAGProfile"
+#define HFP_AG_PROFILE "/Profile/HFPAGProfile"
#define HSP_HS_PROFILE "/Profile/HSPHSProfile"
/* RFCOMM channel for HSP headset role
@@ -512,6 +513,8 @@ static DBusMessage *profile_new_connection(DBusConnection
*conn, DBusMessage *m,
p = PA_BLUETOOTH_PROFILE_HSP_HS;
} else if (pa_streq(handler, HSP_HS_PROFILE)) {
p = PA_BLUETOOTH_PROFILE_HFP_AG;
+} else if (pa_streq(handler, HFP_AG_PROFILE)) {
+ p = PA_BLUETOOTH_PROFILE_HFP_HF;
} else {
pa_log_error("Invalid handler");
goto fail;
@@ -589,7 +592,8 @@ static DBusHandlerResult profile_handler(DBusConnection *c,
DBusMessage *m, void
pa_log_debug("dbus: path=%s, interface=%s, member=%s", path, interface,
member);
-if (!pa_streq(path, HSP_AG_PROFILE) && !pa_streq(path, HSP_HS_PROFILE))
+if (!pa_streq(path, HSP_AG_PROFILE) && !pa_streq(path, HSP_HS_PROFILE)
+&& !pa_streq(path, HFP_AG_PROFILE))
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
if (dbus_message_is_method_call(m, "org.freedesktop.DBus.Introspectable",
"Introspect")) {
@@ -633,6 +637,10 @@ static void profile_init(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
case PA_BLUETOOTH_PROFILE_HFP_AG:
object_name = HSP_HS_PROFILE;
uuid = PA_BLUETOOTH_UUID_HSP_HS;
+ break;
+case PA_BLUETOOTH_PROFILE_HFP_HF:
+object_name = HFP_AG_PROFILE;
+uuid = PA_BLUETOOTH_UUID_HFP_AG;
break;
default:
pa_assert_not_reached();
@@ -652,6 +660,9 @@ static void profile_done(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
break;
case PA_BLUETOOTH_PROFILE_HFP_AG:
dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection),
HSP_HS_PROFILE);
+ break;
+case PA_BLUETOOTH_PROFILE_HFP_HF:
+
dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection),
HFP_AG_PROFILE);
break;
default:
pa_assert_not_reached();
@@ -695,6 +706,8 @@ 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))
+profile_init(backend, PA_BLUETOOTH_PROFILE_HFP_HF);
return backend;
}
@@ -707,6 +720,7 @@ void pa_bluetooth_native_backend_free(pa_bluetooth_backend
*backend) {
if (backend->enable_hs_role)
profile_done(backend, PA_BLUETOOTH_PROFILE_HFP_AG);
profile_done(backend, PA_BLUETOOTH_PROFILE_HSP_HS);
+profile_done(backend, PA_BLUETOOTH_PROFILE_HFP_HF);
pa_dbus_connection_unref(backend->connection);
diff --git a/src/modules/bluetooth/bluez5-util.c
b/src/modules/bluetooth/bluez5-util.c
index 4470f2ef..17771d29 100644
--- a/src/modules/bluetooth/bluez5-util.c
+++ b/src/modules/bluetooth/bluez5-util.c
@@ -92,6 +92,7 @@ struct pa_bluetooth_discovery {
int heads
[pulseaudio-discuss] [PATCH v4 3/3] bluetooth: add correct HFP rfcomm negotiation
HFP 1.6 requires a stateful negotiation of AT commands. The prior
version got away with initialising HFP simply by replying 'OK' to
every negotiation attempt. This one actually tries to parse the state
and make sure the negotiation occurs correctly
Signed-off-by: James Bottomley
---
v4:
- Update for PA 11.0
- Finally sort out CIND negotiation for complex headsets
v3:
- remove internal debugging
- added comment for t->config being not null for hfp
- removed unused returns from hfp_rfcomm_handle()
- remove rfcomm comment
- use pa_startswith
- simplify negotiation
---
src/modules/bluetooth/backend-native.c | 124 +++--
src/modules/bluetooth/bluez5-util.c| 5 +-
src/modules/bluetooth/bluez5-util.h| 2 +-
3 files changed, 125 insertions(+), 6 deletions(-)
diff --git a/src/modules/bluetooth/backend-native.c
b/src/modules/bluetooth/backend-native.c
index b8a8f706..e7f48ed6 100644
--- a/src/modules/bluetooth/backend-native.c
+++ b/src/modules/bluetooth/backend-native.c
@@ -53,6 +53,43 @@ struct transport_data {
pa_mainloop_api *mainloop;
};
+struct hfp_config {
+uint32_t capabilities;
+int state;
+};
+
+/*
+ * the separate hansfree headset (HF) and Audio Gateway (AG) features
+ */
+enum hfp_hf_features {
+HFP_HF_EC_NR = 0,
+HFP_HF_CALL_WAITING = 1,
+HFP_HF_CLI = 2,
+HFP_HF_VR = 3,
+HFP_HF_RVOL = 4,
+HFP_HF_ESTATUS = 5,
+HFP_HF_ECALL = 6,
+HFP_HF_CODECS = 7,
+};
+
+enum hfp_ag_features {
+HFP_AG_THREE_WAY = 0,
+HFP_AG_EC_NR = 1,
+HFP_AG_VR = 2,
+HFP_AG_RING = 3,
+HFP_AG_NUM_TAG = 4,
+HFP_AG_REJECT = 5,
+HFP_AG_ESTATUS = 6,
+HFP_AG_ECALL = 7,
+HFP_AG_EERR = 8,
+HFP_AG_CODECS = 9,
+};
+
+/* gateway features we support, which is as little as we can get away with */
+static uint32_t hfp_features =
+/* HFP 1.6 requires this */
+(1 << HFP_AG_ESTATUS );
+
#define BLUEZ_SERVICE "org.bluez"
#define BLUEZ_MEDIA_TRANSPORT_INTERFACE BLUEZ_SERVICE ".MediaTransport1"
@@ -109,6 +146,27 @@ static pa_dbus_pending*
send_and_add_to_pending(pa_bluetooth_backend *backend, D
return p;
}
+static void rfcomm_write(int fd, const char *str)
+{
+size_t len;
+char buf[512];
+
+pa_log_debug("RFCOMM >> %s", str);
+sprintf(buf, "\r\n%s\r\n", str);
+len = write(fd, buf, strlen(buf));
+
+if (len != strlen(buf))
+pa_log_error("RFCOMM write error: %s", pa_cstrerror(errno));
+}
+
+static void hfp_send_features(int fd)
+{
+char buf[512];
+
+sprintf(buf, "+BRSF: %d", hfp_features);
+rfcomm_write(fd, buf);
+}
+
static int sco_do_connect(pa_bluetooth_transport *t) {
pa_bluetooth_device *d = t->device;
struct sockaddr_sco addr;
@@ -352,6 +410,61 @@ static void register_profile(pa_bluetooth_backend *b,
const char *profile, const
send_and_add_to_pending(b, m, register_profile_reply, pa_xstrdup(profile));
}
+static void transport_put(pa_bluetooth_transport *t)
+{
+pa_bluetooth_transport_put(t);
+
+pa_log_debug("Transport %s available for profile %s", t->path,
pa_bluetooth_profile_to_string(t->profile));
+}
+
+static bool hfp_rfcomm_handle(int fd, pa_bluetooth_transport *t, const char
*buf)
+{
+struct hfp_config *c = t->config;
+int val;
+
+/* stateful negotiation */
+if (c->state == 0 && sscanf(buf, "AT+BRSF=%d", &val) == 1) {
+ c->capabilities = val;
+ pa_log_info("HFP capabilities returns 0x%x", val);
+ hfp_send_features(fd);
+ c->state = 1;
+ return true;
+} else if (c->state == 1 && pa_startswith(buf, "AT+CIND=?")) {
+ /* we declare minimal no indicators */
+ rfcomm_write(fd, "+CIND: "
+/* many indicators can be supported, only call and
+ * callheld are mandatory, so that's all we repy */
+"(\"call\",(0-1)),"
+"(\"callheld\",(0-2))");
+c->state = 2;
+return true;
+} else if (c->state == 2 && pa_startswith(buf, "AT+CIND?")) {
+ rfcomm_write(fd, "+CIND: 0,0");
+c->state = 3;
+return true;
+} else if ((c->state == 2 || c->state == 3) && pa_startswith(buf,
"AT+CMER=")) {
+rfcomm_write(fd, "\r\nOK\r\n");
+c->state = 4;
+transport_put(t);
+return false;
+}
+
+/* if we get here, negotiation should be complete */
+if (c->state != 4) {
+pa_log_error("HFP negotiation failed in state %d with inbound %s\n",
+ c->state, buf);
+rfcomm_write(fd, "ERROR");
+return false;
+}
+
+/*
+ * once we&#
Re: [pulseaudio-discuss] [PATCH v4 0/3] use bluetooth HFP in pulseaudio when available
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 > Then, native HFP would normally only be available with > "headset=native", but those who want to support a mobile through > ofono and a HFP headset directly through PA can still configure it by > explicitly enabling HFP headsets in PA and disabling them on the > ofono side. > > Regards > Georg > ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4 0/3] use bluetooth HFP in pulseaudio when available
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. 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. James ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v4 0/3] use bluetooth HFP in pulseaudio when available
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 i
Re: [pulseaudio-discuss] [PATCH v4 0/3] use bluetooth HFP in pulseaudio when available
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 == HE
Re: [pulseaudio-discuss] [PATCH v4 0/3] use bluetooth HFP in pulseaudio when available
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, enab
[pulseaudio-discuss] [PATCH v5 0/4] use bluetooth HFP in pulseaudio native backend when available
This is round 5 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). This incarnation changes the name of the enabling option and only defaults to true if the native backend is selected (which becomes the default). James Bottomley (4): bluetooth: use consistent profile names bluetooth: separate HSP and HFP bluetooth: add correct HFP rfcomm negotiation bluetooth: make native the default backend src/modules/bluetooth/backend-native.c | 169 +--- 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 | 11 +- 7 files changed, 279 insertions(+), 66 deletions(-) -- 2.12.3 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH v5 1/4] bluetooth: use consistent profile names
The PA_BLUETOOTH_PROFILE names should mirror the PA_BLUETOOTH_UUID
names using profile_function instead of randomly made up names. Fix
this with the transformation:
PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT -> PA_BLUETOOTH_PROFILE_HSP_HS
PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY -> PA_BLUETOOTH_PROFILE_HFP_AG
Signed-off-by: James Bottomley
---
v4: update for PA 11.0
---
src/modules/bluetooth/backend-native.c | 28 +++
src/modules/bluetooth/backend-ofono.c| 4 +--
src/modules/bluetooth/bluez5-util.c | 10 +++---
src/modules/bluetooth/bluez5-util.h | 4 +--
src/modules/bluetooth/module-bluez5-device.c | 54 ++--
5 files changed, 50 insertions(+), 50 deletions(-)
diff --git a/src/modules/bluetooth/backend-native.c
b/src/modules/bluetooth/backend-native.c
index 0f0104d8..f2009bfd 100644
--- a/src/modules/bluetooth/backend-native.c
+++ b/src/modules/bluetooth/backend-native.c
@@ -449,7 +449,7 @@ static void set_speaker_gain(pa_bluetooth_transport *t,
uint16_t gain) {
/* 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_HEADSET_HEAD_UNIT) {
+if (t->profile == PA_BLUETOOTH_PROFILE_HSP_HS) {
len = sprintf(buf, "\r\n+VGS=%d\r\n", gain);
pa_log_debug("RFCOMM >> +VGS=%d", gain);
} else {
@@ -476,7 +476,7 @@ static void set_microphone_gain(pa_bluetooth_transport *t,
uint16_t gain) {
/* 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_HEADSET_HEAD_UNIT) {
+if (t->profile == PA_BLUETOOTH_PROFILE_HSP_HS) {
len = sprintf(buf, "\r\n+VGM=%d\r\n", gain);
pa_log_debug("RFCOMM >> +VGM=%d", gain);
} else {
@@ -509,9 +509,9 @@ static DBusMessage *profile_new_connection(DBusConnection
*conn, DBusMessage *m,
handler = dbus_message_get_path(m);
if (pa_streq(handler, HSP_AG_PROFILE)) {
-p = PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT;
+p = PA_BLUETOOTH_PROFILE_HSP_HS;
} else if (pa_streq(handler, HSP_HS_PROFILE)) {
-p = PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY;
+p = PA_BLUETOOTH_PROFILE_HFP_AG;
} else {
pa_log_error("Invalid handler");
goto fail;
@@ -626,11 +626,11 @@ static void profile_init(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
pa_assert(b);
switch (profile) {
-case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+case PA_BLUETOOTH_PROFILE_HSP_HS:
object_name = HSP_AG_PROFILE;
uuid = PA_BLUETOOTH_UUID_HSP_AG;
break;
-case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
+case PA_BLUETOOTH_PROFILE_HFP_AG:
object_name = HSP_HS_PROFILE;
uuid = PA_BLUETOOTH_UUID_HSP_HS;
break;
@@ -647,10 +647,10 @@ static void profile_done(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
pa_assert(b);
switch (profile) {
-case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+case PA_BLUETOOTH_PROFILE_HSP_HS:
dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection),
HSP_AG_PROFILE);
break;
-case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
+case PA_BLUETOOTH_PROFILE_HFP_AG:
dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection),
HSP_HS_PROFILE);
break;
default:
@@ -665,9 +665,9 @@ void
pa_bluetooth_native_backend_enable_hs_role(pa_bluetooth_backend *native_bac
return;
if (enable_hs_role)
- profile_init(native_backend,
PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY);
+ profile_init(native_backend, PA_BLUETOOTH_PROFILE_HFP_AG);
else
- profile_done(native_backend,
PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY);
+ profile_done(native_backend, PA_BLUETOOTH_PROFILE_HFP_AG);
native_backend->enable_hs_role = enable_hs_role;
}
@@ -693,8 +693,8 @@ pa_bluetooth_backend
*pa_bluetooth_native_backend_new(pa_core *c, pa_bluetooth_d
backend->enable_hs_role = enable_hs_role;
if (enable_hs_role)
- profile_init(backend, PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY);
-profile_init(backend, PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT);
+ profile_init(backend, PA_BLUETOOTH_PROFILE_HFP_AG);
+profile_init(backend, PA_BLUETOOTH_PROFILE_HSP_HS);
return backend;
}
@@ -705,8 +705,8 @@ void pa_bluetooth_native_backend_free(pa_bluetooth_backend
*backend) {
pa_dbus_free_pendi
[pulseaudio-discuss] [PATCH v5 2/4] bluetooth: separate HSP and HFP
When all headsets supported both HSP and HFP, life was good and we
only needed to implement HSP in the native backend. Unfortunately
some headsets have started supporting HFP only. Unfortuantely, we
can't simply switch to HFP only because that might break older HSP
only headsets meaning we need to support both HSP and HFP separately.
This patch separates them from a joint profile to being two separate
ones. The older one retains the headset_head_unit name, meaning any
saved parameters will still select this (keeping us backward
compatible). It also introduces a new headset_handsfree.
For headsets that support both HSP and HFP, the two profiles will
become separately visible and selectable. This will only matter once
we start adding features to HFP that HSP can't support (like wideband
audio).
Signed-off-by:
---
v5:
- rename option to enable_native_hfp_hf
- don't call profile_done for HFP_HF unless it was initialised
v3:
- Update for PA 11.0
v2:
- fold in review feedback
- add global disable option for not registering HFP
v3:
- change parameter to enable_profile_hfp
- update device_supports_profile to be aware of hfp/hsp exclusivity
- change parameter to enable_profile_hfp_hf
---
src/modules/bluetooth/backend-native.c | 17 ++-
src/modules/bluetooth/bluez5-util.c | 31 +++--
src/modules/bluetooth/bluez5-util.h | 4 +-
src/modules/bluetooth/module-bluetooth-policy.c | 3 +-
src/modules/bluetooth/module-bluez5-device.c| 60 +
src/modules/bluetooth/module-bluez5-discover.c | 6 ++-
6 files changed, 106 insertions(+), 15 deletions(-)
diff --git a/src/modules/bluetooth/backend-native.c
b/src/modules/bluetooth/backend-native.c
index f2009bfd..9ec9244b 100644
--- a/src/modules/bluetooth/backend-native.c
+++ b/src/modules/bluetooth/backend-native.c
@@ -62,6 +62,7 @@ struct transport_data {
#define BLUEZ_PROFILE_INTERFACE BLUEZ_SERVICE ".Profile1"
#define HSP_AG_PROFILE "/Profile/HSPAGProfile"
+#define HFP_AG_PROFILE "/Profile/HFPAGProfile"
#define HSP_HS_PROFILE "/Profile/HSPHSProfile"
/* RFCOMM channel for HSP headset role
@@ -512,6 +513,8 @@ static DBusMessage *profile_new_connection(DBusConnection
*conn, DBusMessage *m,
p = PA_BLUETOOTH_PROFILE_HSP_HS;
} else if (pa_streq(handler, HSP_HS_PROFILE)) {
p = PA_BLUETOOTH_PROFILE_HFP_AG;
+} else if (pa_streq(handler, HFP_AG_PROFILE)) {
+p = PA_BLUETOOTH_PROFILE_HFP_HF;
} else {
pa_log_error("Invalid handler");
goto fail;
@@ -589,7 +592,8 @@ static DBusHandlerResult profile_handler(DBusConnection *c,
DBusMessage *m, void
pa_log_debug("dbus: path=%s, interface=%s, member=%s", path, interface,
member);
-if (!pa_streq(path, HSP_AG_PROFILE) && !pa_streq(path, HSP_HS_PROFILE))
+if (!pa_streq(path, HSP_AG_PROFILE) && !pa_streq(path, HSP_HS_PROFILE)
+&& !pa_streq(path, HFP_AG_PROFILE))
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
if (dbus_message_is_method_call(m, "org.freedesktop.DBus.Introspectable",
"Introspect")) {
@@ -634,6 +638,10 @@ static void profile_init(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
object_name = HSP_HS_PROFILE;
uuid = PA_BLUETOOTH_UUID_HSP_HS;
break;
+case PA_BLUETOOTH_PROFILE_HFP_HF:
+object_name = HFP_AG_PROFILE;
+uuid = PA_BLUETOOTH_UUID_HFP_AG;
+break;
default:
pa_assert_not_reached();
break;
@@ -653,6 +661,9 @@ static void profile_done(pa_bluetooth_backend *b,
pa_bluetooth_profile_t profile
case PA_BLUETOOTH_PROFILE_HFP_AG:
dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection),
HSP_HS_PROFILE);
break;
+case PA_BLUETOOTH_PROFILE_HFP_HF:
+
dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection),
HFP_AG_PROFILE);
+break;
default:
pa_assert_not_reached();
break;
@@ -695,6 +706,8 @@ 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_native_hfp_hf(y))
+profile_init(backend, PA_BLUETOOTH_PROFILE_HFP_HF);
return backend;
}
@@ -707,6 +720,8 @@ void pa_bluetooth_native_backend_free(pa_bluetooth_backend
*backend) {
if (backend->enable_hs_role)
profile_done(backend, PA_BLUETOOTH_PROFILE_HFP_AG);
profile_done(backend, PA_BLUETOOTH_PROFILE_HSP_HS);
+if (pa_bluetooth_discovery_get_enable_native_hfp_hf(backend->discovery))
+ profile_done(backend, PA_BLUETOOTH_PROFILE_HFP_HF);
pa_dbus_connection_unref(backend->connection);
diff --git a/src/modules/bluetooth/bluez5-util.c
b/src/modules/bluetooth/bluez5-util.c
index 4470
[pulseaudio-discuss] [PATCH v5 3/4] bluetooth: add correct HFP rfcomm negotiation
HFP 1.6 requires a stateful negotiation of AT commands. The prior
version got away with initialising HFP simply by replying 'OK' to
every negotiation attempt. This one actually tries to parse the state
and make sure the negotiation occurs correctly
Signed-off-by: James Bottomley
---
v4:
- Update for PA 11.0
- Finally sort out CIND negotiaton for complex headsets
v3:
- remove internal debugging
- added comment for t->config being not null for hfp
- removed unused returns from hfp_rfcomm_handle()
- remove rfcomm comment
- use pa_startswith
- simplify negotiation
---
src/modules/bluetooth/backend-native.c | 124 +++--
src/modules/bluetooth/bluez5-util.c| 5 +-
src/modules/bluetooth/bluez5-util.h| 2 +-
3 files changed, 125 insertions(+), 6 deletions(-)
diff --git a/src/modules/bluetooth/backend-native.c
b/src/modules/bluetooth/backend-native.c
index 9ec9244b..99efa066 100644
--- a/src/modules/bluetooth/backend-native.c
+++ b/src/modules/bluetooth/backend-native.c
@@ -53,6 +53,43 @@ struct transport_data {
pa_mainloop_api *mainloop;
};
+struct hfp_config {
+uint32_t capabilities;
+int state;
+};
+
+/*
+ * the separate hansfree headset (HF) and Audio Gateway (AG) features
+ */
+enum hfp_hf_features {
+HFP_HF_EC_NR = 0,
+HFP_HF_CALL_WAITING = 1,
+HFP_HF_CLI = 2,
+HFP_HF_VR = 3,
+HFP_HF_RVOL = 4,
+HFP_HF_ESTATUS = 5,
+HFP_HF_ECALL = 6,
+HFP_HF_CODECS = 7,
+};
+
+enum hfp_ag_features {
+HFP_AG_THREE_WAY = 0,
+HFP_AG_EC_NR = 1,
+HFP_AG_VR = 2,
+HFP_AG_RING = 3,
+HFP_AG_NUM_TAG = 4,
+HFP_AG_REJECT = 5,
+HFP_AG_ESTATUS = 6,
+HFP_AG_ECALL = 7,
+HFP_AG_EERR = 8,
+HFP_AG_CODECS = 9,
+};
+
+/* gateway features we support, which is as little as we can get away with */
+static uint32_t hfp_features =
+/* HFP 1.6 requires this */
+(1 << HFP_AG_ESTATUS );
+
#define BLUEZ_SERVICE "org.bluez"
#define BLUEZ_MEDIA_TRANSPORT_INTERFACE BLUEZ_SERVICE ".MediaTransport1"
@@ -109,6 +146,27 @@ static pa_dbus_pending*
send_and_add_to_pending(pa_bluetooth_backend *backend, D
return p;
}
+static void rfcomm_write(int fd, const char *str)
+{
+size_t len;
+char buf[512];
+
+pa_log_debug("RFCOMM >> %s", str);
+sprintf(buf, "\r\n%s\r\n", str);
+len = write(fd, buf, strlen(buf));
+
+if (len != strlen(buf))
+pa_log_error("RFCOMM write error: %s", pa_cstrerror(errno));
+}
+
+static void hfp_send_features(int fd)
+{
+char buf[512];
+
+sprintf(buf, "+BRSF: %d", hfp_features);
+rfcomm_write(fd, buf);
+}
+
static int sco_do_connect(pa_bluetooth_transport *t) {
pa_bluetooth_device *d = t->device;
struct sockaddr_sco addr;
@@ -352,6 +410,61 @@ static void register_profile(pa_bluetooth_backend *b,
const char *profile, const
send_and_add_to_pending(b, m, register_profile_reply, pa_xstrdup(profile));
}
+static void transport_put(pa_bluetooth_transport *t)
+{
+pa_bluetooth_transport_put(t);
+
+pa_log_debug("Transport %s available for profile %s", t->path,
pa_bluetooth_profile_to_string(t->profile));
+}
+
+static bool hfp_rfcomm_handle(int fd, pa_bluetooth_transport *t, const char
*buf)
+{
+struct hfp_config *c = t->config;
+int val;
+
+/* stateful negotiation */
+if (c->state == 0 && sscanf(buf, "AT+BRSF=%d", &val) == 1) {
+ c->capabilities = val;
+ pa_log_info("HFP capabilities returns 0x%x", val);
+ hfp_send_features(fd);
+ c->state = 1;
+ return true;
+} else if (c->state == 1 && pa_startswith(buf, "AT+CIND=?")) {
+ /* we declare minimal no indicators */
+rfcomm_write(fd, "+CIND: "
+ /* many indicators can be supported, only call and
+ * callheld are mandatory, so that's all we repy */
+ "(\"call\",(0-1)),"
+ "(\"callheld\",(0-2))");
+c->state = 2;
+return true;
+} else if (c->state == 2 && pa_startswith(buf, "AT+CIND?")) {
+rfcomm_write(fd, "+CIND: 0,0");
+c->state = 3;
+return true;
+} else if ((c->state == 2 || c->state == 3) && pa_startswith(buf,
"AT+CMER=")) {
+rfcomm_write(fd, "\r\nOK\r\n");
+c->state = 4;
+transport_put(t);
+return false;
+}
+
+/* if we get here, negotiation should be complete */
+if (c->state != 4) {
+pa_log_error("HFP negotiation failed in state %d with inbound %s\n",
+ c->state, buf);
+rfcomm_write(fd, "ERROR");
+return false;
+}
+
+/*
+ * once we&#
[pulseaudio-discuss] [PATCH v5 4/4] bluetooth: make native the default backend
Change default backend from 'auto' to 'native' so that in the usual
install pulseaudio uses the native backend with HFP_HF handling.
set default to false unless the backend is the native one, in which
case the default becomes true.
Additionally set default value of enable_native_hfp_hf to false unless
the backend is the native one, in which case the default becomes
true. so that we only bind the HFP_HF end point in the native case
(leaving it free for ofono in the ofono backend or auto case)
Signed-off-by: James Bottomley
---
src/modules/bluetooth/module-bluez5-discover.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/modules/bluetooth/module-bluez5-discover.c
b/src/modules/bluetooth/module-bluez5-discover.c
index bfb361ae..d2a0420d 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_native_hfp_hf = true;
+bool enable_native_hfp_hf;
pa_assert(m);
@@ -125,6 +125,9 @@ int pa__init(pa_module *m) {
goto fail;
}
+/* default value if no module parameter */
+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");
--
2.12.3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v5 3/4] bluetooth: add correct HFP rfcomm negotiation
On Fri, 2017-09-22 at 21:09 +0300, Luiz Augusto von Dentz wrote:
> Hi James,
>
> On Thu, Sep 21, 2017 at 10:28 PM, James Bottomley
> wrote:
[...]
> > +static bool hfp_rfcomm_handle(int fd, pa_bluetooth_transport *t,
> > const char *buf)
> > +{
> > +struct hfp_config *c = t->config;
> > +int val;
> > +
>
> Was this code tested against PTS or you just got it working with some
> specific headsets? Im guessing this is for SLC handling but Im afraid
> this was never really qualified properly so perhaps some statement
> here would be nice.
>
> If you have no idea what Im talking about you please have a look at:
>
> https://www.bluetooth.org/docman/handlers/DownloadDoc.ashx?doc_id=411
> 65
The site seems to be login protected, but if this is some type of
simulator, it would be useful to run it by that if you can get me
access.
> There may be commands we would need to implement even if the
> indicators are not supported to pass qualification. If this is just a
> 'works for me' solution then we better put a disclaimer about not
> using this code on Bluetooth certified products.
I can ask Marcel what the usual certification route of Linux code is,
if it's important. I wasn't aware we'd put any pulseaudio code through
bluetooth qualifications before (unless the vendors just did it
quietly).
[...]
> > +/*
> > + * once we're fully connected, just reply OK to everything
> > + * it will just be the headset sending the occasional status
> > + * update, but we process only the ones we care about
> > + */
>
> Im not really sure responding "OK" to everything will do well for
> interoperability, some vendors uses HFP with the its own set of
> vendor commands which may not always accept OK as a response or it
> may trigger some special mode.
I modelled that on the android code
https://android.googlesource.com/platform/packages/apps/Bluetooth/+/master/src/com/android/bluetooth/hfp/HeadsetStateMachine.java
It responds OK to any vendor specific command it doesn't recognise.
James
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
