arkq / bluez-alsa

Bluetooth Audio ALSA Backend
MIT License
864 stars 189 forks source link

HFP fixes #619

Closed borine closed 1 year ago

borine commented 1 year ago

A collection of changes for HFP, some minor bug fixes, some enhancements. The two oFono related commits need some carefult consideration.

"Improve HFP-HF to support gateways that use oFono" is a work-around for the way that oFono interprets the bluetooth HFP codec connection specification. oFono takes it to mean that the SCO connection must be completed before the codec connection is considered complete. It seems neither Android nor IOS have this interpretation, and nor does BueALSA. This commit attempts to allow for BlueALSA to work with both interpretations.

"Enable oFono AG support for mSBC" is a work-around for the way oFono does codec connection when in AG mode. Its worth noting here that oFono has a bug that causes it to enter a busy loop (100%CPU) when a bluetooth device is disconnected. So to test this it is best to patch the oFono source first. Here is my fix:

diff --git a/plugins/hfp_ag_bluez5.c b/plugins/hfp_ag_bluez5.c
index 15acf413..e981a9fb 100644
--- a/plugins/hfp_ag_bluez5.c
+++ b/plugins/hfp_ag_bluez5.c
@@ -249,7 +249,7 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,

    fd_dup = dup(fd);
    io = g_io_channel_unix_new(fd_dup);
-   g_io_add_watch_full(io, G_PRIORITY_DEFAULT, G_IO_HUP, io_hup_cb,
+   g_io_add_watch_full(io, G_PRIORITY_DEFAULT, G_IO_HUP|G_IO_ERR|G_IO_NVAL, io_hup_cb,
                        g_strdup(device), g_free);
    g_io_channel_unref(io);
codecov[bot] commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: -0.09 :warning:

Comparison is base (62dfa34) 69.97% compared to head (d21ed50) 69.88%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #619 +/- ## ========================================== - Coverage 69.97% 69.88% -0.09% ========================================== Files 48 48 Lines 8910 8910 ========================================== - Hits 6235 6227 -8 - Misses 2675 2683 +8 ``` [see 3 files with indirect coverage changes](https://app.codecov.io/gh/Arkq/bluez-alsa/pull/619/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Arkadiusz+Bokowy)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

arkq commented 1 year ago

~@borine I've got a question regarding "Fix SCO volume control". Are you sure that current master's got it wrong? I've just checked -p hsp-hs with an Android phone, and it seems to work correctly. I can see speaker volume changes in alsamixer, and changes made in alsamixer are reflected on phone's volume control (there is no mic control, though, as this feature is not very common).~

EDIT: OK, I've got it.... Indeed it's hard to comprehend what's a speaker and what is a mic in the context of headset :D

EDIT2: I've refactored a little bit your fix for volume handling. It's in the commit https://github.com/arkq/bluez-alsa/commit/fc82f23d2d2e3184a2da5313889f8405d78dcbf3. It will align better with the A2DP setup, and will prevent mistakes in the future. It will be committed under you name, since the whole credit for discovering this issue and the patch, goes to you. I hope you don't mind :)

borine commented 1 year ago

I've rebased against master, and added a new option for bluealsa-rfcomm to print the properties.

In particular, the list of supported features is useful to know in a script, and now that can be got without needing to open a rfcomm terminal and exchange AT messages.

arkq commented 1 year ago

I've just pushed a little refactor of your "Include mSBC in HF supported codecs" commit. I thought that maybe it will be possible to guess a little bit better wheter mSBC codec is available or not :D I've tested it with various scenarios and despite one or two corner cases, it shows mSBC codec only when it is indeed supported.