Linux-Bluetooth Archive on lore.kernel.org
 help / color / Atom feed
* [Bluez PATCH v1] device - If HFP is supported, ignore HSP
@ 2020-06-30 22:49 Yu Liu
  2020-07-01  0:30 ` Luiz Augusto von Dentz
  2020-07-03 10:15 ` Pali Rohár
  0 siblings, 2 replies; 3+ messages in thread
From: Yu Liu @ 2020-06-30 22:49 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Hsin-Yu Chao, Sonny Sasaka

From: Hsin-Yu Chao <hychao@google.com>

For a BT headset that supports both HSP and HFP, BlueZ creates
service instances for these two profiles and connects them.
It's uncertain that which of HSP and HFP eventually get connected
and being used for SCO audio. And we start observing some problem
because of this uncertainty:

- For headset that supports WBS, we need HFP connect for codec
negotiation. If HSP connects but not HFP, WBS cannot be used.
- For WH-1000XM3, if BlueZ ever initiated HFP connection but failed,
headset won't have working SCO audio even HSP is connected.

Fix this at when device probes services, if HFP is in the uuid list,
don't bother create one for HSP.

Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
---

Changes in v1:
- Initial change

 src/device.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/device.c b/src/device.c
index 7b0eb256e..4036bfd81 100644
--- a/src/device.c
+++ b/src/device.c
@@ -4370,6 +4370,11 @@ static struct btd_service *probe_service(struct btd_device *device,
 	if (!device_match_profile(device, profile, uuids))
 		return NULL;
 
+	/* If device supports HFP, don't bother create service for HSP. */
+	if (g_slist_find_custom(uuids, HFP_HS_UUID, bt_uuid_strcmp) &&
+		bt_uuid_strcmp(profile->remote_uuid, HSP_HS_UUID) == 0)
+		return NULL;
+
 	l = find_service_with_profile(device->services, profile);
 	if (l)
 		return l->data;
-- 
2.27.0.212.ge8ba1cc988-goog


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Bluez PATCH v1] device - If HFP is supported, ignore HSP
  2020-06-30 22:49 [Bluez PATCH v1] device - If HFP is supported, ignore HSP Yu Liu
@ 2020-07-01  0:30 ` Luiz Augusto von Dentz
  2020-07-03 10:15 ` Pali Rohár
  1 sibling, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2020-07-01  0:30 UTC (permalink / raw)
  To: Yu Liu; +Cc: linux-bluetooth, Hsin-Yu Chao, Sonny Sasaka

Hi Yu Liu,

On Tue, Jun 30, 2020 at 3:55 PM Yu Liu <yudiliu@google.com> wrote:
>
> From: Hsin-Yu Chao <hychao@google.com>
>
> For a BT headset that supports both HSP and HFP, BlueZ creates
> service instances for these two profiles and connects them.
> It's uncertain that which of HSP and HFP eventually get connected
> and being used for SCO audio. And we start observing some problem
> because of this uncertainty:
>
> - For headset that supports WBS, we need HFP connect for codec
> negotiation. If HSP connects but not HFP, WBS cannot be used.

We should probably have a way to detect when one or the other is
connected and then don't attempt to connect again since they would
likely conflict when it comes to connecting SCO, that said is your
system setting AutoConnect for both HSP and HFP?

> - For WH-1000XM3, if BlueZ ever initiated HFP connection but failed,
> headset won't have working SCO audio even HSP is connected.

Hmm I guess this will need to treated separately, not sure if we can
exclude HSP to be connected once HFP fails, but then again I think the
system should not have them both set to be Auto Connected, HSP most
likely should only be set as as a fallback if HFP is not supported.

> Fix this at when device probes services, if HFP is in the uuid list,
> don't bother create one for HSP.
>
> Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
> ---
>
> Changes in v1:
> - Initial change
>
>  src/device.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/src/device.c b/src/device.c
> index 7b0eb256e..4036bfd81 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -4370,6 +4370,11 @@ static struct btd_service *probe_service(struct btd_device *device,
>         if (!device_match_profile(device, profile, uuids))
>                 return NULL;
>
> +       /* If device supports HFP, don't bother create service for HSP. */
> +       if (g_slist_find_custom(uuids, HFP_HS_UUID, bt_uuid_strcmp) &&
> +               bt_uuid_strcmp(profile->remote_uuid, HSP_HS_UUID) == 0)
> +               return NULL;
> +
>         l = find_service_with_profile(device->services, profile);
>         if (l)
>                 return l->data;
> --
> 2.27.0.212.ge8ba1cc988-goog
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Bluez PATCH v1] device - If HFP is supported, ignore HSP
  2020-06-30 22:49 [Bluez PATCH v1] device - If HFP is supported, ignore HSP Yu Liu
  2020-07-01  0:30 ` Luiz Augusto von Dentz
@ 2020-07-03 10:15 ` Pali Rohár
  1 sibling, 0 replies; 3+ messages in thread
From: Pali Rohár @ 2020-07-03 10:15 UTC (permalink / raw)
  To: Yu Liu; +Cc: linux-bluetooth, Hsin-Yu Chao, Sonny Sasaka

On Tuesday 30 June 2020 15:49:34 Yu Liu wrote:
> From: Hsin-Yu Chao <hychao@google.com>
> 
> For a BT headset that supports both HSP and HFP, BlueZ creates
> service instances for these two profiles and connects them.
> It's uncertain that which of HSP and HFP eventually get connected
> and being used for SCO audio. And we start observing some problem
> because of this uncertainty:
> 
> - For headset that supports WBS, we need HFP connect for codec
> negotiation. If HSP connects but not HFP, WBS cannot be used.
> - For WH-1000XM3, if BlueZ ever initiated HFP connection but failed,
> headset won't have working SCO audio even HSP is connected.
> 
> Fix this at when device probes services, if HFP is in the uuid list,
> don't bother create one for HSP.
> 
> Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
> ---
> 
> Changes in v1:
> - Initial change
> 
>  src/device.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/device.c b/src/device.c
> index 7b0eb256e..4036bfd81 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -4370,6 +4370,11 @@ static struct btd_service *probe_service(struct btd_device *device,
>  	if (!device_match_profile(device, profile, uuids))
>  		return NULL;
>  
> +	/* If device supports HFP, don't bother create service for HSP. */
> +	if (g_slist_find_custom(uuids, HFP_HS_UUID, bt_uuid_strcmp) &&
> +		bt_uuid_strcmp(profile->remote_uuid, HSP_HS_UUID) == 0)
> +		return NULL;
> +

This patch would completely break devices with buggy HFP profile
support.

Also this patch disallow usage of HSP when both HSP and HFP are
supported.

I'm against this change as it disallow to use HSP and HFP simultaneously
with hsphpfd daemon. With hsphfpd daemon it is working with non-buggy
headsets.

Also in pulseaudio we have more bug reports that HFP is does not work
(even when advertised), but HSP is working fine.

If you have a broken headset as you described in commit message, then
please add some quirk for it. But do not disallow usage of HSP / HFP for
all devices including non-broken ones, just because you have one broken
device.

>  	l = find_service_with_profile(device->services, profile);
>  	if (l)
>  		return l->data;
> -- 
> 2.27.0.212.ge8ba1cc988-goog
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-30 22:49 [Bluez PATCH v1] device - If HFP is supported, ignore HSP Yu Liu
2020-07-01  0:30 ` Luiz Augusto von Dentz
2020-07-03 10:15 ` Pali Rohár

Linux-Bluetooth Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-bluetooth/0 linux-bluetooth/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-bluetooth linux-bluetooth/ https://lore.kernel.org/linux-bluetooth \
		linux-bluetooth@vger.kernel.org
	public-inbox-index linux-bluetooth

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-bluetooth


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git