2011-08-22 15:30:38

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v2 0/9] Add support for HandsfreeGateway to Media

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(-)



2011-08-24 12:06:22

by Dalleau, Frederic

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

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

2011-08-24 11:24:52

by Johan Hedberg

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

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

2011-08-24 10:59:02

by Luiz Augusto von Dentz

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

Hi Fr?d?ric,

2011/8/22 Fr?d?ric Dalleau <[email protected]>:
> 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

2011-08-24 10:57:43

by Luiz Augusto von Dentz

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

Hi Fr?d?ric,

2011/8/22 Fr?d?ric Dalleau <[email protected]>:
> 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

2011-08-24 10:55:13

by Luiz Augusto von Dentz

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

Hi Fr?d?ric,

2011/8/22 Fr?d?ric Dalleau <[email protected]>:
> ---
> ?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

2011-08-24 10:54:31

by Luiz Augusto von Dentz

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

Hi Fr?d?ric,

2011/8/22 Fr?d?ric Dalleau <[email protected]>:
> 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

2011-08-24 10:54:02

by Luiz Augusto von Dentz

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

Hi Fr?d?ric,

2011/8/22 Fr?d?ric Dalleau <[email protected]>:
> 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

2011-08-24 10:53:31

by Luiz Augusto von Dentz

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

Hi Fr?d?ric,

2011/8/22 Fr?d?ric Dalleau <[email protected]>:
> ---
> ?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

2011-08-24 10:53:04

by Luiz Augusto von Dentz

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

Hi Fr?d?ric,

2011/8/22 Fr?d?ric Dalleau <[email protected]>:
> 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

2011-08-24 10:51:56

by Luiz Augusto von Dentz

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

Hi Fr?d?ric,

2011/8/22 Fr?d?ric Dalleau <[email protected]>:
> 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 [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

Ack

--
Luiz Augusto von Dentz

2011-08-24 10:48:28

by Luiz Augusto von Dentz

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

Hi Fr?d?ric,

2011/8/22 Fr?d?ric Dalleau <[email protected]>:
> 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

2011-08-22 15:30:47

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v2 9/9] Implement media_transport for HFP_HS_UUID

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


2011-08-22 15:30:46

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v2 8/9] Introduce gateway locking mechanism

---
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


2011-08-22 15:30:45

by Frédéric DALLEAU

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

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


2011-08-22 15:30:44

by Frédéric DALLEAU

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

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


2011-08-22 15:30:43

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v2 5/9] Fix RFCOMM disconnect on suspend request

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


2011-08-22 15:30:42

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v2 4/9] Update g_strcmp0 to strcasecmp

---
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


2011-08-22 15:30:41

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v2 3/9] Use state change callback from media interface

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


2011-08-22 15:30:40

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v2 2/9] Add state change callback to HandsfreeGateway

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


2011-08-22 15:30:39

by Frédéric DALLEAU

[permalink] [raw]
Subject: [PATCH v2 1/9] Fix double free in error case in endpoint_reply

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