2014-12-03 10:42:23

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH v3 0/3] android/handsfree: Move to use bt_sco lib

This set has couple of fixes plus patch moving sco handling to
bt_sco lib

v2:
* Improve disconnect_sco()
* remove patch adding default to switch/case
* Patch for handling idle state can be removed because of improvement of
disconnect_sco()

v3:
* added patch for disconnect SCO to bt_sco lib. this is needed for following patch

Lukasz Rymanowski (3):
android/sco: Fix for disconnect SCO
android/handsfree: Use bt_sco lib in HFP AG code
android/handsfree: Simplify codec selection

android/handsfree.c | 185 +++++++++++++---------------------------------------
android/sco.c | 12 +---
2 files changed, 47 insertions(+), 150 deletions(-)

--
1.8.4



2014-12-03 13:24:21

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] android/handsfree: Move to use bt_sco lib

Hi Ɓukasz,

On Wednesday 03 of December 2014 11:42:23 Lukasz Rymanowski wrote:
> This set has couple of fixes plus patch moving sco handling to
> bt_sco lib
>
> v2:
> * Improve disconnect_sco()
> * remove patch adding default to switch/case
> * Patch for handling idle state can be removed because of improvement of
> disconnect_sco()
>
> v3:
> * added patch for disconnect SCO to bt_sco lib. this is needed for following patch
>
> Lukasz Rymanowski (3):
> android/sco: Fix for disconnect SCO
> android/handsfree: Use bt_sco lib in HFP AG code
> android/handsfree: Simplify codec selection
>
> android/handsfree.c | 185 +++++++++++++---------------------------------------
> android/sco.c | 12 +---
> 2 files changed, 47 insertions(+), 150 deletions(-)
>

All patches applied, thanks.

--
Best regards,
Szymon Janc

2014-12-03 10:42:25

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH v3 2/3] android/handsfree: Use bt_sco lib in HFP AG code

---
android/handsfree.c | 180 +++++++++++++---------------------------------------
1 file changed, 44 insertions(+), 136 deletions(-)

diff --git a/android/handsfree.c b/android/handsfree.c
index 45826d9..5c19b6e 100644
--- a/android/handsfree.c
+++ b/android/handsfree.c
@@ -48,6 +48,7 @@
#include "src/log.h"
#include "utils.h"
#include "sco-msg.h"
+#include "sco.h"

#define HSP_AG_CHANNEL 12
#define HFP_AG_CHANNEL 13
@@ -137,9 +138,6 @@ struct hf_device {
bool hsp;

struct hfp_gw *gw;
-
- GIOChannel *sco;
- guint sco_watch;
guint delay_sco;
};

@@ -173,7 +171,7 @@ static GIOChannel *hfp_server = NULL;
static uint32_t hsp_record_id = 0;
static GIOChannel *hsp_server = NULL;

-static GIOChannel *sco_server = NULL;
+static struct bt_sco *sco = NULL;

static unsigned int max_hfp_clients = 0;

@@ -254,16 +252,11 @@ static void device_destroy(struct hf_device *dev)
{
hfp_gw_unref(dev->gw);

- if (dev->sco_watch)
- g_source_remove(dev->sco_watch);
-
if (dev->delay_sco)
g_source_remove(dev->delay_sco);

- if (dev->sco) {
- g_io_channel_shutdown(dev->sco, TRUE, NULL);
- g_io_channel_unref(dev->sco);
- }
+ if (dev->audio_state == HAL_EV_HANDSFREE_AUDIO_STATE_CONNECTED)
+ bt_sco_disconnect(sco);

g_source_remove(dev->ring);
g_free(dev->clip);
@@ -936,22 +929,19 @@ static void at_cmd_btrh(struct hfp_context *context,
hfp_gw_send_result(dev->gw, HFP_RESULT_ERROR);
}

-static gboolean sco_watch_cb(GIOChannel *chan, GIOCondition cond,
- gpointer user_data)
+static void disconnect_sco_cb(const bdaddr_t *addr)
{
- struct hf_device *dev = user_data;
-
- g_io_channel_shutdown(dev->sco, TRUE, NULL);
- g_io_channel_unref(dev->sco);
- dev->sco = NULL;
+ struct hf_device *dev;

DBG("");

- dev->sco_watch = 0;
+ dev = find_device(addr);
+ if (!dev) {
+ error("handsfree: Could not find device");
+ return;
+ }

set_audio_state(dev, HAL_EV_HANDSFREE_AUDIO_STATE_DISCONNECTED);
-
- return FALSE;
}

static void select_codec(struct hf_device *dev, uint8_t codec_type)
@@ -987,12 +977,18 @@ static bool codec_negotiation_supported(struct hf_device *dev)
(hfp_ag_features & HFP_AG_FEAT_CODEC);
}

-static void connect_sco_cb(GIOChannel *chan, GError *err, gpointer user_data)
+static void connect_sco_cb(enum sco_status status, const bdaddr_t *addr)
{
- struct hf_device *dev = user_data;
+ struct hf_device *dev;

- if (err) {
- error("handsfree: audio connect failed (%s)", err->message);
+ dev = find_device(addr);
+ if (!dev) {
+ error("handsfree: Connect sco failed, no device?");
+ return;
+ }
+
+ if (status != SCO_STATUS_OK) {
+ error("handsfree: audio connect failed");

set_audio_state(dev, HAL_EV_HANDSFREE_AUDIO_STATE_DISCONNECTED);

@@ -1008,24 +1004,13 @@ static void connect_sco_cb(GIOChannel *chan, GError *err, gpointer user_data)
return;
}

- g_io_channel_set_close_on_unref(chan, TRUE);
-
- dev->sco = g_io_channel_ref(chan);
- dev->sco_watch = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP | G_IO_NVAL,
- sco_watch_cb, dev);
-
set_audio_state(dev, HAL_EV_HANDSFREE_AUDIO_STATE_CONNECTED);
}

static bool connect_sco(struct hf_device *dev)
{
- GIOChannel *io;
- GError *gerr = NULL;
uint16_t voice_settings;

- if (dev->sco)
- return false;
-
if (!codec_negotiation_supported(dev))
voice_settings = 0;
else if (dev->negotiated_codec != CODEC_ID_CVSD)
@@ -1033,19 +1018,8 @@ static bool connect_sco(struct hf_device *dev)
else
voice_settings = BT_VOICE_CVSD_16BIT;

- io = bt_io_connect(connect_sco_cb, dev, NULL, &gerr,
- BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
- BT_IO_OPT_DEST_BDADDR, &dev->bdaddr,
- BT_IO_OPT_VOICE, voice_settings,
- BT_IO_OPT_INVALID);
-
- if (!io) {
- error("handsfree: unable to connect audio: %s", gerr->message);
- g_error_free(gerr);
+ if (!bt_sco_connect(sco, &dev->bdaddr, voice_settings))
return false;
- }
-
- g_io_channel_unref(io);

set_audio_state(dev, HAL_EV_HANDSFREE_AUDIO_STATE_CONNECTING);

@@ -1843,27 +1817,13 @@ failed:

static bool disconnect_sco(struct hf_device *dev)
{
- if (!dev->sco) {
- if (dev->delay_sco) {
- g_source_remove(dev->delay_sco);
- dev->delay_sco = 0;
- }
-
+ if (dev->audio_state == HAL_EV_HANDSFREE_AUDIO_STATE_DISCONNECTED ||
+ dev->audio_state == HAL_EV_HANDSFREE_AUDIO_STATE_DISCONNECTING)
return false;
- }

+ bt_sco_disconnect(sco);
set_audio_state(dev, HAL_EV_HANDSFREE_AUDIO_STATE_DISCONNECTING);

- if (dev->sco_watch) {
- g_source_remove(dev->sco_watch);
- dev->sco_watch = 0;
- }
-
- g_io_channel_shutdown(dev->sco, TRUE, NULL);
- g_io_channel_unref(dev->sco);
- dev->sco = NULL;
-
- set_audio_state(dev, HAL_EV_HANDSFREE_AUDIO_STATE_DISCONNECTED);
return true;
}

@@ -1927,11 +1887,6 @@ static void handle_disconnect_audio(const void *buf, uint16_t len)
goto done;
}

- if (dev->audio_state != HAL_EV_HANDSFREE_AUDIO_STATE_CONNECTED) {
- status = HAL_STATUS_FAILED;
- goto done;
- }
-
status = disconnect_sco(dev) ? HAL_STATUS_SUCCESS : HAL_STATUS_FAILED;

done:
@@ -2681,42 +2636,26 @@ static sdp_record_t *headset_ag_record(void)
return record;
}

-static void confirm_sco_cb(GIOChannel *chan, gpointer user_data)
+static bool confirm_sco_cb(const bdaddr_t *addr, uint16_t *voice_settings)
{
char address[18];
- bdaddr_t bdaddr;
- GError *err = NULL;
struct hf_device *dev;

- bt_io_get(chan, &err,
- BT_IO_OPT_DEST, address,
- BT_IO_OPT_DEST_BDADDR, &bdaddr,
- BT_IO_OPT_INVALID);
- if (err) {
- error("handsfree: audio confirm failed (%s)", err->message);
- g_error_free(err);
- goto drop;
- }
+ ba2str(addr, address);

DBG("incoming SCO connection from %s", address);

- dev = find_device(&bdaddr);
- if (!dev || dev->state != HAL_EV_HANDSFREE_CONN_STATE_SLC_CONNECTED ||
- dev->sco) {
+ dev = find_device(addr);
+ if (!dev || dev->state != HAL_EV_HANDSFREE_CONN_STATE_SLC_CONNECTED) {
error("handsfree: audio connection from %s rejected", address);
- goto drop;
+ return false;
}

- if (!bt_io_accept(chan, connect_sco_cb, dev, NULL, NULL)) {
- error("handsfree: failed to accept audio connection");
- goto drop;
- }
+ /* If HF initiate SCO there must be no WBS used */
+ *voice_settings = 0;

set_audio_state(dev, HAL_EV_HANDSFREE_AUDIO_STATE_CONNECTING);
- return;
-
-drop:
- g_io_channel_shutdown(chan, TRUE, NULL);
+ return true;
}

static bool enable_hsp_ag(void)
@@ -2912,40 +2851,12 @@ static void cleanup_hfp_ag(void)
}
}

-static bool enable_sco_server(void)
-{
- GError *err = NULL;
-
- sco_server = bt_io_listen(NULL, confirm_sco_cb, NULL, NULL, &err,
- BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
- BT_IO_OPT_INVALID);
- if (!sco_server) {
- error("handsfree: Failed to listen on SCO: %s", err->message);
- g_error_free(err);
- cleanup_hsp_ag();
- cleanup_hfp_ag();
- return false;
- }
-
- return true;
-}
-
-static void disable_sco_server(void)
-{
- if (sco_server) {
- g_io_channel_shutdown(sco_server, TRUE, NULL);
- g_io_channel_unref(sco_server);
- sco_server = NULL;
- }
-}
-
static void bt_sco_get_fd(const void *buf, uint16_t len)
{
const struct sco_cmd_get_fd *cmd = buf;
struct sco_rsp_get_fd rsp;
struct hf_device *dev;
bdaddr_t bdaddr;
- GError *err;
int fd;

DBG("");
@@ -2953,19 +2864,9 @@ static void bt_sco_get_fd(const void *buf, uint16_t len)
android2bdaddr(cmd->bdaddr, &bdaddr);

dev = find_device(&bdaddr);
- if (!dev || !dev->sco)
+ if (!dev || !bt_sco_get_fd_and_mtu(sco, &fd, &rsp.mtu))
goto failed;

- err = NULL;
- if (!bt_io_get(dev->sco, &err, BT_IO_OPT_MTU, &rsp.mtu,
- BT_IO_OPT_INVALID)) {
- error("Unable to get MTU: %s\n", err->message);
- g_clear_error(&err);
- goto failed;
- }
-
- fd = g_io_channel_unix_get_fd(dev->sco);
-
DBG("fd %d mtu %u", fd, rsp.mtu);

ipc_send_rsp_full(sco_ipc, SCO_SERVICE_ID, SCO_OP_GET_FD,
@@ -3022,9 +2923,14 @@ bool bt_handsfree_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode,
if (!enable_hsp_ag())
goto failed_queue;

- if (!enable_sco_server())
+ sco = bt_sco_new(addr);
+ if (!sco)
goto failed_hsp;

+ bt_sco_set_confirm_cb(sco, confirm_sco_cb);
+ bt_sco_set_connect_cb(sco, connect_sco_cb);
+ bt_sco_set_disconnect_cb(sco, disconnect_sco_cb);
+
if (mode == HAL_MODE_HANDSFREE_HSP_ONLY)
goto done;

@@ -3036,7 +2942,8 @@ bool bt_handsfree_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode,
if (enable_hfp_ag())
goto done;

- disable_sco_server();
+ bt_sco_unref(sco);
+ sco = NULL;
hfp_ag_features = 0;
failed_hsp:
cleanup_hsp_ag();
@@ -3068,7 +2975,8 @@ void bt_handsfree_unregister(void)

cleanup_hfp_ag();
cleanup_hsp_ag();
- disable_sco_server();
+ bt_sco_unref(sco);
+ sco = NULL;

hfp_ag_features = 0;

--
1.8.4


2014-12-03 10:42:26

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH v3 3/3] android/handsfree: Simplify codec selection

Voice settings 0 is legacy, lets use BT_VOICE_CVSD_16BIT instead.
---
android/handsfree.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/android/handsfree.c b/android/handsfree.c
index 5c19b6e..aeb933f 100644
--- a/android/handsfree.c
+++ b/android/handsfree.c
@@ -1011,9 +1011,8 @@ static bool connect_sco(struct hf_device *dev)
{
uint16_t voice_settings;

- if (!codec_negotiation_supported(dev))
- voice_settings = 0;
- else if (dev->negotiated_codec != CODEC_ID_CVSD)
+ if (codec_negotiation_supported(dev) &&
+ dev->negotiated_codec != CODEC_ID_CVSD)
voice_settings = BT_VOICE_TRANSPARENT;
else
voice_settings = BT_VOICE_CVSD_16BIT;
--
1.8.4


2014-12-03 10:42:24

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH v3 1/3] android/sco: Fix for disconnect SCO

We should just shutdown io and wait for disconnect callback in
disconnect function, otherwise we might get some races e.g. when doing
connect/disconnect SCO. This patch fix that.
---
android/sco.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/android/sco.c b/android/sco.c
index 7dd7013..e8ac685 100644
--- a/android/sco.c
+++ b/android/sco.c
@@ -310,18 +310,8 @@ void bt_sco_disconnect(struct bt_sco *sco)
if (!sco)
return;

- if (sco->watch) {
- g_source_remove(sco->watch);
- sco->watch = 0;
- }
-
- if (sco->io) {
+ if (sco->io)
g_io_channel_shutdown(sco->io, TRUE, NULL);
- g_io_channel_unref(sco->io);
- sco->io = NULL;
- }
-
- clear_remote_address(sco);
}

bool bt_sco_get_fd_and_mtu(struct bt_sco *sco, int *fd, uint16_t *mtu)
--
1.8.4