[PATCH v2 0/9] Add support for HandsfreeGateway to Media

[PATCH v2 0/9] Add support for HandsfreeGateway to Media

[Frédéric Dalleau]

This patch will enable HandsfreeGateway interface to work with PulseAudio.
This is similar to what is done for Headset interface.

Also there is some minor fixes in the pulse audio part required.
I've updated the patch on the PA list to v3

Best regards,
Frédéric

Frédéric Dalleau (9):
  Fix double free in error case in endpoint_reply
  Add state change callback to HandsfreeGateway
  Use state change callback from media interface
  Update g_strcmp0 to strcasecmp
  Fix RFCOMM disconnect on suspend request
  Set state to "connecting" when AG connects.
  Fix invalid state at SCO disconnect in gateway
  Introduce gateway locking mechanism
  Implement media_transport for HFP_HS_UUID

 audio/gateway.c   |  107 ++++++++++++++++++++++++++++++++++++++++++-
 audio/gateway.h   |   15 ++++++
 audio/media.c     |   61 ++++++++++++++++++++++++-
 audio/transport.c |  130 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 309 insertions(+), 4 deletions(-)

[PATCH v2 1/9] Fix double free in error case in endpoint_reply

[Frédéric Dalleau]

If SetConfiguration call fails, the headset or gateway for which
SetConfiguration is called is disconnected. This will free any
pending request, but this does not prevent the request to
terminate (endpoint_reply) and try to free itself once again.
Note that a copy of the freed pointer is tested which has not
been updated.
---
 audio/media.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/audio/media.c b/audio/media.c
index 42d8637..2076d04 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -264,7 +264,8 @@ done:
 	if (request->cb)
 		request->cb(endpoint, ret, size, request->user_data);
 
-	endpoint_request_free(request);
+	if (endpoint->request)
+		endpoint_request_free(endpoint->request);
 	endpoint->request = NULL;
 }
 
-- 
1.7.1

Re: [PATCH v2 1/9] Fix double free in error case in endpoint_reply

[Luiz Augusto von Dentz]

Hi Frédéric,

2011/8/22 Frédéric Dalleau <frederic.dalleau@linux.intel.com>:
> If SetConfiguration call fails, the headset or gateway for which
> SetConfiguration is called is disconnected. This will free any
> pending request, but this does not prevent the request to
> terminate (endpoint_reply) and try to free itself once again.
> Note that a copy of the freed pointer is tested which has not
> been updated.
> ---
>  audio/media.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/audio/media.c b/audio/media.c
> index 42d8637..2076d04 100644
> --- a/audio/media.c
> +++ b/audio/media.c
> @@ -264,7 +264,8 @@ done:
>        if (request->cb)
>                request->cb(endpoint, ret, size, request->user_data);
>
> -       endpoint_request_free(request);
> +       if (endpoint->request)
> +               endpoint_request_free(endpoint->request);
>        endpoint->request = NULL;
>  }
>
> --
> 1.7.1

Nice catch, ack.

-- 
Luiz Augusto von Dentz

[PATCH v2 2/9] Add state change callback to HandsfreeGateway

[Frédéric Dalleau]

Media interface needs to monitor state changes of HandsfreeGateway.
---
 audio/gateway.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 audio/gateway.h |    7 ++++++
 2 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/audio/gateway.c b/audio/gateway.c
index e9485d0..805b8cb 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -72,6 +72,14 @@ struct gateway {
 	int version;
 };
 
+struct gateway_state_callback {
+	gateway_state_cb cb;
+	void *user_data;
+	unsigned int id;
+};
+
+static GSList *gateway_callbacks = NULL;
+
 int gateway_close(struct audio_device *device);
 
 static const char *state2str(gateway_state_t state)
@@ -104,16 +112,37 @@ static void change_state(struct audio_device *dev, gateway_state_t new_state)
 {
 	struct gateway *gw = dev->gateway;
 	const char *val;
+	GSList *l;
+	gateway_state_t old_state;
 
 	if (gw->state == new_state)
 		return;
 
 	val = state2str(new_state);
+	old_state = gw->state;
 	gw->state = new_state;
 
 	emit_property_changed(dev->conn, dev->path,
 			AUDIO_GATEWAY_INTERFACE, "State",
 			DBUS_TYPE_STRING, &val);
+
+	for (l = gateway_callbacks; l != NULL; l = l->next) {
+		struct gateway_state_callback *cb = l->data;
+		cb->cb(dev, old_state, new_state, cb->user_data);
+	}
+}
+
+void gateway_set_state(struct audio_device *dev, gateway_state_t new_state)
+{
+	switch (new_state) {
+	case GATEWAY_STATE_DISCONNECTED:
+		gateway_close(dev);
+		break;
+	case GATEWAY_STATE_CONNECTING:
+	case GATEWAY_STATE_CONNECTED:
+	case GATEWAY_STATE_PLAYING:
+		break;
+	}
 }
 
 static void agent_disconnect(struct audio_device *dev, struct hf_agent *agent)
@@ -810,3 +839,35 @@ void gateway_suspend_stream(struct audio_device *dev)
 	gw->sco_start_cb_data = NULL;
 	change_state(dev, GATEWAY_STATE_CONNECTED);
 }
+
+unsigned int gateway_add_state_cb(gateway_state_cb cb, void *user_data)
+{
+	struct gateway_state_callback *state_cb;
+	static unsigned int id = 0;
+
+	state_cb = g_new(struct gateway_state_callback, 1);
+	state_cb->cb = cb;
+	state_cb->user_data = user_data;
+	state_cb->id = ++id;
+
+	gateway_callbacks = g_slist_append(gateway_callbacks, state_cb);
+
+	return state_cb->id;
+}
+
+gboolean gateway_remove_state_cb(unsigned int id)
+{
+	GSList *l;
+
+	for (l = gateway_callbacks; l != NULL; l = l->next) {
+		struct gateway_state_callback *cb = l->data;
+		if (cb && cb->id == id) {
+			gateway_callbacks = g_slist_remove(gateway_callbacks,
+									cb);
+			g_free(cb);
+			return TRUE;
+		}
+	}
+
+	return FALSE;
+}
diff --git a/audio/gateway.h b/audio/gateway.h
index a45ef82..32b5d6d 100644
--- a/audio/gateway.h
+++ b/audio/gateway.h
@@ -33,9 +33,14 @@ typedef enum {
 	GATEWAY_STATE_PLAYING,
 } gateway_state_t;
 
+typedef void (*gateway_state_cb) (struct audio_device *dev,
+					gateway_state_t old_state,
+					gateway_state_t new_state,
+					void *user_data);
 typedef void (*gateway_stream_cb_t) (struct audio_device *dev, GError *err,
 		void *user_data);
 
+void gateway_set_state(struct audio_device *dev, gateway_state_t new_state);
 void gateway_unregister(struct audio_device *dev);
 struct gateway *gateway_init(struct audio_device *device);
 gboolean gateway_is_connected(struct audio_device *dev);
@@ -49,3 +54,5 @@ int gateway_config_stream(struct audio_device *dev, gateway_stream_cb_t cb,
 gboolean gateway_cancel_stream(struct audio_device *dev, unsigned int id);
 int gateway_get_sco_fd(struct audio_device *dev);
 void gateway_suspend_stream(struct audio_device *dev);
+unsigned int gateway_add_state_cb(gateway_state_cb cb, void *user_data);
+gboolean gateway_remove_state_cb(unsigned int id);
-- 
1.7.1

Re: [PATCH v2 2/9] Add state change callback to HandsfreeGateway

[Luiz Augusto von Dentz]

Hi Frédéric,

2011/8/22 Frédéric Dalleau <frederic.dalleau@linux.intel.com>:
> Media interface needs to monitor state changes of HandsfreeGateway.
> ---
>  audio/gateway.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  audio/gateway.h |    7 ++++++
>  2 files changed, 68 insertions(+), 0 deletions(-)
>
> diff --git a/audio/gateway.c b/audio/gateway.c
> index e9485d0..805b8cb 100644
> --- a/audio/gateway.c
> +++ b/audio/gateway.c
> @@ -72,6 +72,14 @@ struct gateway {
>        int version;
>  };
>
> +struct gateway_state_callback {
> +       gateway_state_cb cb;
> +       void *user_data;
> +       unsigned int id;
> +};
> +
> +static GSList *gateway_callbacks = NULL;
> +
>  int gateway_close(struct audio_device *device);
>
>  static const char *state2str(gateway_state_t state)
> @@ -104,16 +112,37 @@ static void change_state(struct audio_device *dev, gateway_state_t new_state)
>  {
>        struct gateway *gw = dev->gateway;
>        const char *val;
> +       GSList *l;
> +       gateway_state_t old_state;
>
>        if (gw->state == new_state)
>                return;
>
>        val = state2str(new_state);
> +       old_state = gw->state;
>        gw->state = new_state;
>
>        emit_property_changed(dev->conn, dev->path,
>                        AUDIO_GATEWAY_INTERFACE, "State",
>                        DBUS_TYPE_STRING, &val);
> +
> +       for (l = gateway_callbacks; l != NULL; l = l->next) {
> +               struct gateway_state_callback *cb = l->data;
> +               cb->cb(dev, old_state, new_state, cb->user_data);
> +       }
> +}
> +
> +void gateway_set_state(struct audio_device *dev, gateway_state_t new_state)
> +{
> +       switch (new_state) {
> +       case GATEWAY_STATE_DISCONNECTED:
> +               gateway_close(dev);
> +               break;
> +       case GATEWAY_STATE_CONNECTING:
> +       case GATEWAY_STATE_CONNECTED:
> +       case GATEWAY_STATE_PLAYING:
> +               break;
> +       }
>  }
>
>  static void agent_disconnect(struct audio_device *dev, struct hf_agent *agent)
> @@ -810,3 +839,35 @@ void gateway_suspend_stream(struct audio_device *dev)
>        gw->sco_start_cb_data = NULL;
>        change_state(dev, GATEWAY_STATE_CONNECTED);
>  }
> +
> +unsigned int gateway_add_state_cb(gateway_state_cb cb, void *user_data)
> +{
> +       struct gateway_state_callback *state_cb;
> +       static unsigned int id = 0;
> +
> +       state_cb = g_new(struct gateway_state_callback, 1);
> +       state_cb->cb = cb;
> +       state_cb->user_data = user_data;
> +       state_cb->id = ++id;
> +
> +       gateway_callbacks = g_slist_append(gateway_callbacks, state_cb);
> +
> +       return state_cb->id;
> +}
> +
> +gboolean gateway_remove_state_cb(unsigned int id)
> +{
> +       GSList *l;
> +
> +       for (l = gateway_callbacks; l != NULL; l = l->next) {
> +               struct gateway_state_callback *cb = l->data;
> +               if (cb && cb->id == id) {
> +                       gateway_callbacks = g_slist_remove(gateway_callbacks,
> +                                                                       cb);
> +                       g_free(cb);
> +                       return TRUE;
> +               }
> +       }
> +
> +       return FALSE;
> +}
> diff --git a/audio/gateway.h b/audio/gateway.h
> index a45ef82..32b5d6d 100644
> --- a/audio/gateway.h
> +++ b/audio/gateway.h
> @@ -33,9 +33,14 @@ typedef enum {
>        GATEWAY_STATE_PLAYING,
>  } gateway_state_t;
>
> +typedef void (*gateway_state_cb) (struct audio_device *dev,
> +                                       gateway_state_t old_state,
> +                                       gateway_state_t new_state,
> +                                       void *user_data);
>  typedef void (*gateway_stream_cb_t) (struct audio_device *dev, GError *err,
>                void *user_data);
>
> +void gateway_set_state(struct audio_device *dev, gateway_state_t new_state);
>  void gateway_unregister(struct audio_device *dev);
>  struct gateway *gateway_init(struct audio_device *device);
>  gboolean gateway_is_connected(struct audio_device *dev);
> @@ -49,3 +54,5 @@ int gateway_config_stream(struct audio_device *dev, gateway_stream_cb_t cb,
>  gboolean gateway_cancel_stream(struct audio_device *dev, unsigned int id);
>  int gateway_get_sco_fd(struct audio_device *dev);
>  void gateway_suspend_stream(struct audio_device *dev);
> +unsigned int gateway_add_state_cb(gateway_state_cb cb, void *user_data);
> +gboolean gateway_remove_state_cb(unsigned int id);
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Ack

-- 
Luiz Augusto von Dentz

[PATCH v2 3/9] Use state change callback from media interface

[Frédéric Dalleau]

When a HFPHS endpoint is created, media will start to watch for
HandsfreeGateway state changes. media call SetConfiguration upon
connection and ClearConfiguration upon disconnection.
---
 audio/media.c |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 56 insertions(+), 0 deletions(-)

diff --git a/audio/media.c b/audio/media.c
index 2076d04..ae2b471 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -43,6 +43,7 @@
 #include "transport.h"
 #include "a2dp.h"
 #include "headset.h"
+#include "gateway.h"
 #include "manager.h"
 
 #ifndef DBUS_TYPE_UNIX_FD
@@ -78,6 +79,7 @@ struct media_endpoint {
 	uint8_t			*capabilities;	/* Endpoint property capabilities */
 	size_t			size;		/* Endpoint capabilities size */
 	guint			hs_watch;
+	guint			ag_watch;
 	guint			watch;
 	struct endpoint_request *request;
 	struct media_transport	*transport;
@@ -118,6 +120,9 @@ static void media_endpoint_destroy(struct media_endpoint *endpoint)
 	if (endpoint->hs_watch)
 		headset_remove_state_cb(endpoint->hs_watch);
 
+	if (endpoint->ag_watch)
+		gateway_remove_state_cb(endpoint->ag_watch);
+
 	if (endpoint->request)
 		media_endpoint_cancel(endpoint);
 
@@ -553,6 +558,46 @@ static void a2dp_destroy_endpoint(void *user_data)
 	release_endpoint(endpoint);
 }
 
+static void gateway_setconf_cb(struct media_endpoint *endpoint, void *ret,
+						int size, void *user_data)
+{
+	struct audio_device *dev = user_data;
+
+	if (ret != NULL)
+		return;
+
+	gateway_set_state(dev, GATEWAY_STATE_DISCONNECTED);
+}
+
+static void gateway_state_changed(struct audio_device *dev,
+					gateway_state_t old_state,
+					gateway_state_t new_state,
+					void *user_data)
+{
+	struct media_endpoint *endpoint = user_data;
+
+	DBG("");
+
+	switch (new_state) {
+	case GATEWAY_STATE_DISCONNECTED:
+		if (endpoint->transport &&
+			media_transport_get_dev(endpoint->transport) == dev) {
+
+			DBG("Clear endpoint %p", endpoint);
+			clear_configuration(endpoint);
+		}
+		break;
+	case GATEWAY_STATE_CONNECTING:
+		set_configuration(endpoint, dev, NULL, 0,
+					gateway_setconf_cb, dev, NULL);
+		break;
+	case GATEWAY_STATE_CONNECTED:
+		break;
+	case GATEWAY_STATE_PLAYING:
+		break;
+	}
+}
+
 static struct media_endpoint *media_endpoint_create(struct media_adapter *adapter,
 						const char *sender,
 						const char *path,
@@ -604,6 +649,17 @@ static struct media_endpoint *media_endpoint_create(struct media_adapter *adapte
 		if (dev)
 			set_configuration(endpoint, dev, NULL, 0,
 						headset_setconf_cb, dev, NULL);
+	} else if (strcasecmp(uuid, HFP_HS_UUID) == 0 ||
+					strcasecmp(uuid, HSP_HS_UUID) == 0) {
+		struct audio_device *dev;
+
+		endpoint->ag_watch = gateway_add_state_cb(gateway_state_changed,
+								endpoint);
+		dev = manager_find_device(NULL, &adapter->src, BDADDR_ANY,
+						AUDIO_GATEWAY_INTERFACE, TRUE);
+		if (dev)
+			set_configuration(endpoint, dev, NULL, 0,
+						gateway_setconf_cb, dev, NULL);
 	} else {
 		if (err)
 			*err = -EINVAL;
-- 
1.7.1

Re: [PATCH v2 3/9] Use state change callback from media interface

[Luiz Augusto von Dentz]

Hi Frédéric,

2011/8/22 Frédéric Dalleau <frederic.dalleau@linux.intel.com>:
> When a HFPHS endpoint is created, media will start to watch for
> HandsfreeGateway state changes. media call SetConfiguration upon
> connection and ClearConfiguration upon disconnection.
> ---
>  audio/media.c |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 56 insertions(+), 0 deletions(-)
>
> diff --git a/audio/media.c b/audio/media.c
> index 2076d04..ae2b471 100644
> --- a/audio/media.c
> +++ b/audio/media.c
> @@ -43,6 +43,7 @@
>  #include "transport.h"
>  #include "a2dp.h"
>  #include "headset.h"
> +#include "gateway.h"
>  #include "manager.h"
>
>  #ifndef DBUS_TYPE_UNIX_FD
> @@ -78,6 +79,7 @@ struct media_endpoint {
>        uint8_t                 *capabilities;  /* Endpoint property capabilities */
>        size_t                  size;           /* Endpoint capabilities size */
>        guint                   hs_watch;
> +       guint                   ag_watch;
>        guint                   watch;
>        struct endpoint_request *request;
>        struct media_transport  *transport;
> @@ -118,6 +120,9 @@ static void media_endpoint_destroy(struct media_endpoint *endpoint)
>        if (endpoint->hs_watch)
>                headset_remove_state_cb(endpoint->hs_watch);
>
> +       if (endpoint->ag_watch)
> +               gateway_remove_state_cb(endpoint->ag_watch);
> +
>        if (endpoint->request)
>                media_endpoint_cancel(endpoint);
>
> @@ -553,6 +558,46 @@ static void a2dp_destroy_endpoint(void *user_data)
>        release_endpoint(endpoint);
>  }
>
> +static void gateway_setconf_cb(struct media_endpoint *endpoint, void *ret,
> +                                               int size, void *user_data)
> +{
> +       struct audio_device *dev = user_data;
> +
> +       if (ret != NULL)
> +               return;
> +
> +       gateway_set_state(dev, GATEWAY_STATE_DISCONNECTED);
> +}
> +
> +static void gateway_state_changed(struct audio_device *dev,
> +                                       gateway_state_t old_state,
> +                                       gateway_state_t new_state,
> +                                       void *user_data)
> +{
> +       struct media_endpoint *endpoint = user_data;
> +
> +       DBG("");
> +
> +       switch (new_state) {
> +       case GATEWAY_STATE_DISCONNECTED:
> +               if (endpoint->transport &&
> +                       media_transport_get_dev(endpoint->transport) == dev) {
> +
> +                       DBG("Clear endpoint %p", endpoint);
> +                       clear_configuration(endpoint);
> +               }
> +               break;
> +       case GATEWAY_STATE_CONNECTING:
> +               set_configuration(endpoint, dev, NULL, 0,
> +                                       gateway_setconf_cb, dev, NULL);
> +               break;
> +       case GATEWAY_STATE_CONNECTED:
> +               break;
> +       case GATEWAY_STATE_PLAYING:
> +               break;
> +       }
> +}
> +
>  static struct media_endpoint *media_endpoint_create(struct media_adapter *adapter,
>                                                const char *sender,
>                                                const char *path,
> @@ -604,6 +649,17 @@ static struct media_endpoint *media_endpoint_create(struct media_adapter *adapte
>                if (dev)
>                        set_configuration(endpoint, dev, NULL, 0,
>                                                headset_setconf_cb, dev, NULL);
> +       } else if (strcasecmp(uuid, HFP_HS_UUID) == 0 ||
> +                                       strcasecmp(uuid, HSP_HS_UUID) == 0) {
> +               struct audio_device *dev;
> +
> +               endpoint->ag_watch = gateway_add_state_cb(gateway_state_changed,
> +                                                               endpoint);
> +               dev = manager_find_device(NULL, &adapter->src, BDADDR_ANY,
> +                                               AUDIO_GATEWAY_INTERFACE, TRUE);
> +               if (dev)
> +                       set_configuration(endpoint, dev, NULL, 0,
> +                                               gateway_setconf_cb, dev, NULL);
>        } else {
>                if (err)
>                        *err = -EINVAL;
> --
> 1.7.1
>

Ack

-- 
Luiz Augusto von Dentz

[PATCH v2 4/9] Update g_strcmp0 to strcasecmp

[Frédéric Dalleau]

---
 audio/media.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/audio/media.c b/audio/media.c
index ae2b471..5ab3eab 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -639,7 +639,7 @@ static struct media_endpoint *media_endpoint_create(struct media_adapter *adapte
 		if (endpoint->sep == NULL)
 			goto failed;
 	} else if (strcasecmp(uuid, HFP_AG_UUID) == 0 ||
-					g_strcmp0(uuid, HSP_AG_UUID) == 0) {
+					strcasecmp(uuid, HSP_AG_UUID) == 0) {
 		struct audio_device *dev;
 
 		endpoint->hs_watch = headset_add_state_cb(headset_state_changed,
-- 
1.7.1

Re: [PATCH v2 4/9] Update g_strcmp0 to strcasecmp

[Luiz Augusto von Dentz]

Hi Frédéric,

2011/8/22 Frédéric Dalleau <frederic.dalleau@linux.intel.com>:
> ---
>  audio/media.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/audio/media.c b/audio/media.c
> index ae2b471..5ab3eab 100644
> --- a/audio/media.c
> +++ b/audio/media.c
> @@ -639,7 +639,7 @@ static struct media_endpoint *media_endpoint_create(struct media_adapter *adapte
>                if (endpoint->sep == NULL)
>                        goto failed;
>        } else if (strcasecmp(uuid, HFP_AG_UUID) == 0 ||
> -                                       g_strcmp0(uuid, HSP_AG_UUID) == 0) {
> +                                       strcasecmp(uuid, HSP_AG_UUID) == 0) {
>                struct audio_device *dev;
>
>                endpoint->hs_watch = headset_add_state_cb(headset_state_changed,
> --
> 1.7.1
>
> --

Ack

-- 
Luiz Augusto von Dentz

[PATCH v2 5/9] Fix RFCOMM disconnect on suspend request

[Frédéric Dalleau]

When pulseaudio release an audio stream, gateway.c disconnects
RFCOMM instead of disconnecting only SCO.
---
 audio/gateway.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/audio/gateway.c b/audio/gateway.c
index 805b8cb..425ad16 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -811,7 +811,7 @@ int gateway_config_stream(struct audio_device *dev, gateway_stream_cb_t sco_cb,
 
 gboolean gateway_cancel_stream(struct audio_device *dev, unsigned int id)
 {
-	gateway_close(dev);
+	gateway_suspend_stream(dev);
 	return TRUE;
 }
 
-- 
1.7.1

Re: [PATCH v2 5/9] Fix RFCOMM disconnect on suspend request

[Luiz Augusto von Dentz]

Hi Frédéric,

2011/8/22 Frédéric Dalleau <frederic.dalleau@linux.intel.com>:
> When pulseaudio release an audio stream, gateway.c disconnects
> RFCOMM instead of disconnecting only SCO.
> ---
>  audio/gateway.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/audio/gateway.c b/audio/gateway.c
> index 805b8cb..425ad16 100644
> --- a/audio/gateway.c
> +++ b/audio/gateway.c
> @@ -811,7 +811,7 @@ int gateway_config_stream(struct audio_device *dev, gateway_stream_cb_t sco_cb,
>
>  gboolean gateway_cancel_stream(struct audio_device *dev, unsigned int id)
>  {
> -       gateway_close(dev);
> +       gateway_suspend_stream(dev);
>        return TRUE;
>  }
>
> --
> 1.7.1
>
> --

Ack

-- 
Luiz Augusto von Dentz

[PATCH v2 6/9] Set state to "connecting" when AG connects.

[Frédéric Dalleau]

This state is used by media.c to trigger pulseaudio configuration.
---
 audio/gateway.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/audio/gateway.c b/audio/gateway.c
index 425ad16..c3c4019 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -284,6 +284,8 @@ static void rfcomm_connect_cb(GIOChannel *chan, GError *err,
 		goto fail;
 	}
 
+	change_state(dev, GATEWAY_STATE_CONNECTING);
+
 	sk = g_io_channel_unix_get_fd(chan);
 
 	gw->rfcomm = g_io_channel_ref(chan);
-- 
1.7.1

Re: [PATCH v2 6/9] Set state to "connecting" when AG connects.

[Luiz Augusto von Dentz]

Hi Frédéric,

2011/8/22 Frédéric Dalleau <frederic.dalleau@linux.intel.com>:
> This state is used by media.c to trigger pulseaudio configuration.
> ---
>  audio/gateway.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/audio/gateway.c b/audio/gateway.c
> index 425ad16..c3c4019 100644
> --- a/audio/gateway.c
> +++ b/audio/gateway.c
> @@ -284,6 +284,8 @@ static void rfcomm_connect_cb(GIOChannel *chan, GError *err,
>                goto fail;
>        }
>
> +       change_state(dev, GATEWAY_STATE_CONNECTING);
> +
>        sk = g_io_channel_unix_get_fd(chan);
>
>        gw->rfcomm = g_io_channel_ref(chan);
> --
> 1.7.1
>
> --

Ack

-- 
Luiz Augusto von Dentz

[PATCH v2 7/9] Fix invalid state at SCO disconnect in gateway

[Frédéric Dalleau]

If SCO disconnects after RFCOMM, do not change state to connected.
---
 audio/gateway.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/audio/gateway.c b/audio/gateway.c
index c3c4019..d41bbc3 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -191,7 +191,9 @@ static gboolean sco_io_cb(GIOChannel *chan, GIOCondition cond,
 	g_io_channel_shutdown(gw->sco, TRUE, NULL);
 	g_io_channel_unref(gw->sco);
 	gw->sco = NULL;
-	change_state(dev, GATEWAY_STATE_CONNECTED);
+
+	if (gw->rfcomm)
+		change_state(dev, GATEWAY_STATE_CONNECTED);
 
 	return FALSE;
 }
-- 
1.7.1

Re: [PATCH v2 7/9] Fix invalid state at SCO disconnect in gateway

[Luiz Augusto von Dentz]

Hi Frédéric,

2011/8/22 Frédéric Dalleau <frederic.dalleau@linux.intel.com>:
> If SCO disconnects after RFCOMM, do not change state to connected.
> ---
>  audio/gateway.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/audio/gateway.c b/audio/gateway.c
> index c3c4019..d41bbc3 100644
> --- a/audio/gateway.c
> +++ b/audio/gateway.c
> @@ -191,7 +191,9 @@ static gboolean sco_io_cb(GIOChannel *chan, GIOCondition cond,
>        g_io_channel_shutdown(gw->sco, TRUE, NULL);
>        g_io_channel_unref(gw->sco);
>        gw->sco = NULL;
> -       change_state(dev, GATEWAY_STATE_CONNECTED);
> +
> +       if (gw->rfcomm)
> +               change_state(dev, GATEWAY_STATE_CONNECTED);
>
>        return FALSE;
>  }
> --
> 1.7.1
>

This looks like a workaround, the watch should not be active anymore
if rfcomm is disconnected so there is a bug somewhere else.

-- 
Luiz Augusto von Dentz

[PATCH v2 8/9] Introduce gateway locking mechanism

[Frédéric Dalleau]

---
 audio/gateway.c |   38 ++++++++++++++++++++++++++++++++++++++
 audio/gateway.h |    8 ++++++++
 2 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/audio/gateway.c b/audio/gateway.c
index d41bbc3..e1db84b 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -70,6 +70,7 @@ struct gateway {
 	struct hf_agent *agent;
 	DBusMessage *msg;
 	int version;
+	gateway_lock_t lock;
 };
 
 struct gateway_state_callback {
@@ -875,3 +876,40 @@ gboolean gateway_remove_state_cb(unsigned int id)
 
 	return FALSE;
 }
+
+gateway_lock_t gateway_get_lock(struct audio_device *dev)
+{
+	struct gateway *gw = dev->gateway;
+
+	return gw->lock;
+}
+
+gboolean gateway_lock(struct audio_device *dev, gateway_lock_t lock)
+{
+	struct gateway *gw = dev->gateway;
+
+	if (gw->lock & lock)
+		return FALSE;
+
+	gw->lock |= lock;
+
+	return TRUE;
+}
+
+gboolean gateway_unlock(struct audio_device *dev, gateway_lock_t lock)
+{
+	struct gateway *gw = dev->gateway;
+
+	if (!(gw->lock & lock))
+		return FALSE;
+
+	gw->lock &= ~lock;
+
+	if (gw->lock)
+		return TRUE;
+
+	if (gw->state == GATEWAY_STATE_PLAYING)
+		gateway_suspend_stream(dev);
+
+	return TRUE;
+}
diff --git a/audio/gateway.h b/audio/gateway.h
index 32b5d6d..2dca32a 100644
--- a/audio/gateway.h
+++ b/audio/gateway.h
@@ -33,6 +33,11 @@ typedef enum {
 	GATEWAY_STATE_PLAYING,
 } gateway_state_t;
 
+typedef enum {
+	GATEWAY_LOCK_READ = 1,
+	GATEWAY_LOCK_WRITE = 1 << 1,
+} gateway_lock_t;
+
 typedef void (*gateway_state_cb) (struct audio_device *dev,
 					gateway_state_t old_state,
 					gateway_state_t new_state,
@@ -56,3 +61,6 @@ int gateway_get_sco_fd(struct audio_device *dev);
 void gateway_suspend_stream(struct audio_device *dev);
 unsigned int gateway_add_state_cb(gateway_state_cb cb, void *user_data);
 gboolean gateway_remove_state_cb(unsigned int id);
+gateway_lock_t gateway_get_lock(struct audio_device *dev);
+gboolean gateway_lock(struct audio_device *dev, gateway_lock_t lock);
+gboolean gateway_unlock(struct audio_device *dev, gateway_lock_t lock);
-- 
1.7.1

Re: [PATCH v2 8/9] Introduce gateway locking mechanism

[Luiz Augusto von Dentz]

Hi Frédéric,

2011/8/22 Frédéric Dalleau <frederic.dalleau@linux.intel.com>:
> ---
>  audio/gateway.c |   38 ++++++++++++++++++++++++++++++++++++++
>  audio/gateway.h |    8 ++++++++
>  2 files changed, 46 insertions(+), 0 deletions(-)
>
> diff --git a/audio/gateway.c b/audio/gateway.c
> index d41bbc3..e1db84b 100644
> --- a/audio/gateway.c
> +++ b/audio/gateway.c
> @@ -70,6 +70,7 @@ struct gateway {
>        struct hf_agent *agent;
>        DBusMessage *msg;
>        int version;
> +       gateway_lock_t lock;
>  };
>
>  struct gateway_state_callback {
> @@ -875,3 +876,40 @@ gboolean gateway_remove_state_cb(unsigned int id)
>
>        return FALSE;
>  }
> +
> +gateway_lock_t gateway_get_lock(struct audio_device *dev)
> +{
> +       struct gateway *gw = dev->gateway;
> +
> +       return gw->lock;
> +}
> +
> +gboolean gateway_lock(struct audio_device *dev, gateway_lock_t lock)
> +{
> +       struct gateway *gw = dev->gateway;
> +
> +       if (gw->lock & lock)
> +               return FALSE;
> +
> +       gw->lock |= lock;
> +
> +       return TRUE;
> +}
> +
> +gboolean gateway_unlock(struct audio_device *dev, gateway_lock_t lock)
> +{
> +       struct gateway *gw = dev->gateway;
> +
> +       if (!(gw->lock & lock))
> +               return FALSE;
> +
> +       gw->lock &= ~lock;
> +
> +       if (gw->lock)
> +               return TRUE;
> +
> +       if (gw->state == GATEWAY_STATE_PLAYING)
> +               gateway_suspend_stream(dev);
> +
> +       return TRUE;
> +}
> diff --git a/audio/gateway.h b/audio/gateway.h
> index 32b5d6d..2dca32a 100644
> --- a/audio/gateway.h
> +++ b/audio/gateway.h
> @@ -33,6 +33,11 @@ typedef enum {
>        GATEWAY_STATE_PLAYING,
>  } gateway_state_t;
>
> +typedef enum {
> +       GATEWAY_LOCK_READ = 1,
> +       GATEWAY_LOCK_WRITE = 1 << 1,
> +} gateway_lock_t;
> +
>  typedef void (*gateway_state_cb) (struct audio_device *dev,
>                                        gateway_state_t old_state,
>                                        gateway_state_t new_state,
> @@ -56,3 +61,6 @@ int gateway_get_sco_fd(struct audio_device *dev);
>  void gateway_suspend_stream(struct audio_device *dev);
>  unsigned int gateway_add_state_cb(gateway_state_cb cb, void *user_data);
>  gboolean gateway_remove_state_cb(unsigned int id);
> +gateway_lock_t gateway_get_lock(struct audio_device *dev);
> +gboolean gateway_lock(struct audio_device *dev, gateway_lock_t lock);
> +gboolean gateway_unlock(struct audio_device *dev, gateway_lock_t lock);
> --
> 1.7.1
>
> --

Ack

-- 
Luiz Augusto von Dentz

[PATCH v2 9/9] Implement media_transport for HFP_HS_UUID

[Frédéric Dalleau]

The transport is used by pulseaudio to suspend and resume streams.
---
 audio/transport.c |  130 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 130 insertions(+), 0 deletions(-)

diff --git a/audio/transport.c b/audio/transport.c
index f915262..2739199 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -42,6 +42,7 @@
 #include "transport.h"
 #include "a2dp.h"
 #include "headset.h"
+#include "gateway.h"
 
 #ifndef DBUS_TYPE_UNIX_FD
 #define DBUS_TYPE_UNIX_FD -1
@@ -436,6 +437,115 @@ static void cancel_headset(struct media_transport *transport, guint id)
 	headset_cancel_stream(transport->device, id);
 }
 
+static void gateway_resume_complete(struct audio_device *dev, GError *err,
+							void *user_data)
+{
+	struct media_owner *owner = user_data;
+	struct media_request *req = owner->pending;
+	struct media_transport *transport = owner->transport;
+	int fd;
+	uint16_t imtu, omtu;
+	gboolean ret;
+
+	req->id = 0;
+
+	if (dev == NULL)
+		goto fail;
+
+	if (err) {
+		error("Failed to resume gateway: error %s", err->message);
+		goto fail;
+	}
+
+	fd = gateway_get_sco_fd(dev);
+	if (fd < 0)
+		goto fail;
+
+	imtu = 48;
+	omtu = 48;
+
+	media_transport_set_fd(transport, fd, imtu, omtu);
+
+	if (g_strstr_len(owner->accesstype, -1, "r") == NULL)
+		imtu = 0;
+
+	if (g_strstr_len(owner->accesstype, -1, "w") == NULL)
+		omtu = 0;
+
+	ret = g_dbus_send_reply(transport->conn, req->msg,
+						DBUS_TYPE_UNIX_FD, &fd,
+						DBUS_TYPE_UINT16, &imtu,
+						DBUS_TYPE_UINT16, &omtu,
+						DBUS_TYPE_INVALID);
+	if (ret == FALSE)
+		goto fail;
+
+	media_owner_remove(owner);
+
+	return;
+
+fail:
+	media_transport_remove(transport, owner);
+}
+
+static guint resume_gateway(struct media_transport *transport,
+				struct media_owner *owner)
+{
+	struct audio_device *device = transport->device;
+
+	if (transport->in_use == TRUE)
+		goto done;
+
+	transport->in_use = gateway_lock(device, GATEWAY_LOCK_READ |
+						GATEWAY_LOCK_WRITE);
+	if (transport->in_use == FALSE)
+		return 0;
+
+done:
+	return gateway_request_stream(device, gateway_resume_complete,
+					owner);
+}
+
+static gboolean gateway_suspend_complete(gpointer user_data)
+{
+	struct media_owner *owner = user_data;
+	struct media_transport *transport = owner->transport;
+
+	/* Release always succeeds */
+	if (owner->pending) {
+		owner->pending->id = 0;
+		media_request_reply(owner->pending, transport->conn, 0);
+		media_owner_remove(owner);
+	}
+
+	transport->in_use = FALSE;
+	media_transport_remove(transport, owner);
+	return FALSE;
+}
+
+static guint suspend_gateway(struct media_transport *transport,
+						struct media_owner *owner)
+{
+	struct audio_device *device = transport->device;
+	static int id = 1;
+
+	if (!owner) {
+		gateway_unlock(device, GATEWAY_LOCK_READ | GATEWAY_LOCK_WRITE);
+		transport->in_use = FALSE;
+		return 0;
+	}
+
+	gateway_suspend_stream(device);
+	gateway_unlock(device, GATEWAY_LOCK_READ | GATEWAY_LOCK_WRITE);
+	g_idle_add(gateway_suspend_complete, owner);
+	return id++;
+}
+
+static void cancel_gateway(struct media_transport *transport, guint id)
+{
+	gateway_cancel_stream(transport->device, id);
+}
+
 static void media_owner_exit(DBusConnection *connection, void *user_data)
 {
 	struct media_owner *owner = user_data;
@@ -672,6 +782,13 @@ static int set_property_headset(struct media_transport *transport,
 	return -EINVAL;
 }
 
+static int set_property_gateway(struct media_transport *transport,
+						const char *property,
+						DBusMessageIter *value)
+{
+	return -EINVAL;
+}
+
 static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
 								void *data)
 {
@@ -740,6 +857,12 @@ static void get_properties_headset(struct media_transport *transport,
 	dict_append_entry(dict, "Routing", DBUS_TYPE_STRING, &routing);
 }
 
+static void get_properties_gateway(struct media_transport *transport,
+						DBusMessageIter *dict)
+{
+	/* None */
+}
+
 void transport_get_properties(struct media_transport *transport,
 							DBusMessageIter *iter)
 {
@@ -881,6 +1004,13 @@ struct media_transport *media_transport_create(DBusConnection *conn,
 		transport->nrec_id = headset_add_nrec_cb(device,
 							headset_nrec_changed,
 							transport);
+	} else if (strcasecmp(uuid, HFP_HS_UUID) == 0 ||
+			strcasecmp(uuid, HSP_HS_UUID) == 0) {
+		transport->resume = resume_gateway;
+		transport->suspend = suspend_gateway;
+		transport->cancel = cancel_gateway;
+		transport->get_properties = get_properties_gateway;
+		transport->set_property = set_property_gateway;
 	} else
 		goto fail;
 
-- 
1.7.1

Re: [PATCH v2 9/9] Implement media_transport for HFP_HS_UUID

[Luiz Augusto von Dentz]

Hi Frédéric,

2011/8/22 Frédéric Dalleau <frederic.dalleau@linux.intel.com>:
> The transport is used by pulseaudio to suspend and resume streams.
> ---
>  audio/transport.c |  130 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 130 insertions(+), 0 deletions(-)
>
> diff --git a/audio/transport.c b/audio/transport.c
> index f915262..2739199 100644
> --- a/audio/transport.c
> +++ b/audio/transport.c
> @@ -42,6 +42,7 @@
>  #include "transport.h"
>  #include "a2dp.h"
>  #include "headset.h"
> +#include "gateway.h"
>
>  #ifndef DBUS_TYPE_UNIX_FD
>  #define DBUS_TYPE_UNIX_FD -1
> @@ -436,6 +437,115 @@ static void cancel_headset(struct media_transport *transport, guint id)
>        headset_cancel_stream(transport->device, id);
>  }
>
> +static void gateway_resume_complete(struct audio_device *dev, GError *err,
> +                                                       void *user_data)
> +{
> +       struct media_owner *owner = user_data;
> +       struct media_request *req = owner->pending;
> +       struct media_transport *transport = owner->transport;
> +       int fd;
> +       uint16_t imtu, omtu;
> +       gboolean ret;
> +
> +       req->id = 0;
> +
> +       if (dev == NULL)
> +               goto fail;
> +
> +       if (err) {
> +               error("Failed to resume gateway: error %s", err->message);
> +               goto fail;
> +       }
> +
> +       fd = gateway_get_sco_fd(dev);
> +       if (fd < 0)
> +               goto fail;
> +
> +       imtu = 48;
> +       omtu = 48;
> +
> +       media_transport_set_fd(transport, fd, imtu, omtu);
> +
> +       if (g_strstr_len(owner->accesstype, -1, "r") == NULL)
> +               imtu = 0;
> +
> +       if (g_strstr_len(owner->accesstype, -1, "w") == NULL)
> +               omtu = 0;
> +
> +       ret = g_dbus_send_reply(transport->conn, req->msg,
> +                                               DBUS_TYPE_UNIX_FD, &fd,
> +                                               DBUS_TYPE_UINT16, &imtu,
> +                                               DBUS_TYPE_UINT16, &omtu,
> +                                               DBUS_TYPE_INVALID);
> +       if (ret == FALSE)
> +               goto fail;
> +
> +       media_owner_remove(owner);
> +
> +       return;
> +
> +fail:
> +       media_transport_remove(transport, owner);
> +}
> +
> +static guint resume_gateway(struct media_transport *transport,
> +                               struct media_owner *owner)
> +{
> +       struct audio_device *device = transport->device;
> +
> +       if (transport->in_use == TRUE)
> +               goto done;
> +
> +       transport->in_use = gateway_lock(device, GATEWAY_LOCK_READ |
> +                                               GATEWAY_LOCK_WRITE);
> +       if (transport->in_use == FALSE)
> +               return 0;
> +
> +done:
> +       return gateway_request_stream(device, gateway_resume_complete,
> +                                       owner);
> +}
> +
> +static gboolean gateway_suspend_complete(gpointer user_data)
> +{
> +       struct media_owner *owner = user_data;
> +       struct media_transport *transport = owner->transport;
> +
> +       /* Release always succeeds */
> +       if (owner->pending) {
> +               owner->pending->id = 0;
> +               media_request_reply(owner->pending, transport->conn, 0);
> +               media_owner_remove(owner);
> +       }
> +
> +       transport->in_use = FALSE;
> +       media_transport_remove(transport, owner);
> +       return FALSE;
> +}
> +
> +static guint suspend_gateway(struct media_transport *transport,
> +                                               struct media_owner *owner)
> +{
> +       struct audio_device *device = transport->device;
> +       static int id = 1;
> +
> +       if (!owner) {
> +               gateway_unlock(device, GATEWAY_LOCK_READ | GATEWAY_LOCK_WRITE);
> +               transport->in_use = FALSE;
> +               return 0;
> +       }
> +
> +       gateway_suspend_stream(device);
> +       gateway_unlock(device, GATEWAY_LOCK_READ | GATEWAY_LOCK_WRITE);
> +       g_idle_add(gateway_suspend_complete, owner);
> +       return id++;
> +}
> +
> +static void cancel_gateway(struct media_transport *transport, guint id)
> +{
> +       gateway_cancel_stream(transport->device, id);
> +}
> +
>  static void media_owner_exit(DBusConnection *connection, void *user_data)
>  {
>        struct media_owner *owner = user_data;
> @@ -672,6 +782,13 @@ static int set_property_headset(struct media_transport *transport,
>        return -EINVAL;
>  }
>
> +static int set_property_gateway(struct media_transport *transport,
> +                                               const char *property,
> +                                               DBusMessageIter *value)
> +{
> +       return -EINVAL;
> +}
> +
>  static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
>                                                                void *data)
>  {
> @@ -740,6 +857,12 @@ static void get_properties_headset(struct media_transport *transport,
>        dict_append_entry(dict, "Routing", DBUS_TYPE_STRING, &routing);
>  }
>
> +static void get_properties_gateway(struct media_transport *transport,
> +                                               DBusMessageIter *dict)
> +{
> +       /* None */
> +}
> +
>  void transport_get_properties(struct media_transport *transport,
>                                                        DBusMessageIter *iter)
>  {
> @@ -881,6 +1004,13 @@ struct media_transport *media_transport_create(DBusConnection *conn,
>                transport->nrec_id = headset_add_nrec_cb(device,
>                                                        headset_nrec_changed,
>                                                        transport);
> +       } else if (strcasecmp(uuid, HFP_HS_UUID) == 0 ||
> +                       strcasecmp(uuid, HSP_HS_UUID) == 0) {
> +               transport->resume = resume_gateway;
> +               transport->suspend = suspend_gateway;
> +               transport->cancel = cancel_gateway;
> +               transport->get_properties = get_properties_gateway;
> +               transport->set_property = set_property_gateway;
>        } else
>                goto fail;
>
> --
> 1.7.1
>
> --

Looks good, ack.

-- 
Luiz Augusto von Dentz

Re: [PATCH v2 0/9] Add support for HandsfreeGateway to Media

[Johan Hedberg]

Hi Frédéric,

On Mon, Aug 22, 2011, Frédéric Dalleau wrote:
> This patch will enable HandsfreeGateway interface to work with PulseAudio.
> This is similar to what is done for Headset interface.
> 
> Also there is some minor fixes in the pulse audio part required.
> I've updated the patch on the PA list to v3
> 
> Best regards,
> Frédéric
> 
> Frédéric Dalleau (9):
>   Fix double free in error case in endpoint_reply
>   Add state change callback to HandsfreeGateway
>   Use state change callback from media interface
>   Update g_strcmp0 to strcasecmp
>   Fix RFCOMM disconnect on suspend request
>   Set state to "connecting" when AG connects.
>   Fix invalid state at SCO disconnect in gateway
>   Introduce gateway locking mechanism
>   Implement media_transport for HFP_HS_UUID
> 
>  audio/gateway.c   |  107 ++++++++++++++++++++++++++++++++++++++++++-
>  audio/gateway.h   |   15 ++++++
>  audio/media.c     |   61 ++++++++++++++++++++++++-
>  audio/transport.c |  130 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 309 insertions(+), 4 deletions(-)

All patches applied except 7/9 (for which I'm waiting for a response to
Luiz's feedback). Thanks.

Johan

Re: [PATCH v2 0/9] Add support for HandsfreeGateway to Media

[Dalleau, Frederic]

Hi Johan and Luiz,

> All patches applied except 7/9 (for which I'm waiting for a response to
> Luiz's feedback). Thanks.

Thanks for review, I'm looking further into this one!

Regards,
Frédéric