[PATCHv2 27/60] bluetooth: Create pa_bluetooth_transport for BlueZ 5 support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2013-08-13 at 01:54 -0300, jprvita at gmail.com wrote:
> From: Jo?o Paulo Rechi Vita <jprvita at openbossa.org>
> 
> Create the pa_bluetooth_transport structure to store information about
> the bluetooth transport and utility functions to manipulate this
> structure. The acquire() and release() operations are function pointers
> in the pa_bluetooth_transport structure to make possible for different
> transport backends to provide different implementations of these
> operations. Thre is also a userdata field for the transport backend
> provide data for the acquire/release functions.
> ---
>  src/modules/bluetooth/bluez5-util.c | 145 ++++++++++++++++++++++++++++++++++++
>  src/modules/bluetooth/bluez5-util.h |  43 +++++++++++
>  2 files changed, 188 insertions(+)
> 
> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> index 1972a92..69eb759 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -36,6 +36,7 @@
>  #include "bluez5-util.h"
>  
>  #define BLUEZ_SERVICE "org.bluez"
> +#define BLUEZ_MEDIA_TRANSPORT_INTERFACE BLUEZ_SERVICE ".MediaTransport1"
>  
>  struct pa_bluetooth_discovery {
>      PA_REFCNT_DECLARE;
> @@ -45,8 +46,131 @@ struct pa_bluetooth_discovery {
>      bool filter_added;
>      pa_hook hooks[PA_BLUETOOTH_HOOK_MAX];
>      pa_hashmap *devices;
> +    pa_hashmap *transports;
>  };
>  
> +pa_bluetooth_transport *pa_bluetooth_transport_new(pa_bluetooth_device *d, const char *owner, const char *path,
> +                                                          pa_bluetooth_profile_t p, const uint8_t *config, size_t size) {

Unaligned parameter list.

There is some precedence for wrapping long parameter lists like this:

pa_bluetooth_transport *pa_bluetooth_transport_new(
        pa_bluetooth_device *d,
        const char *owner,
        const char *path,
        pa_bluetooth_profile_t p,
        const uint8_t *config,
        size_t size) {

> +    pa_bluetooth_transport *t;
> +
> +    t = pa_xnew0(pa_bluetooth_transport, 1);
> +    t->device = d;
> +    t->owner = pa_xstrdup(owner);
> +    t->path = pa_xstrdup(path);
> +    t->profile = p;
> +    t->config_size = size;
> +
> +    if (size > 0) {
> +        t->config = pa_xnew(uint8_t, size);
> +        memcpy(t->config, config, size);
> +    }
> +
> +    pa_assert_se(pa_hashmap_put(d->discovery->transports, t->path, t) >= 0);
> +
> +    return t;
> +}
> +
> +static void transport_state_changed(pa_bluetooth_transport *t, pa_bluetooth_transport_state_t state) {
> +    bool old_any_connected;
> +
> +    if (t->state == state)
> +        return;
> +
> +    old_any_connected = pa_bluetooth_device_any_transport_connected(t->device);
> +
> +    pa_log_debug("Transport %s state changed from %d to %d", t->path, t->state, state);

Human-readable states, please.

> +
> +    t->state = state;
> +    pa_hook_fire(&t->device->discovery->hooks[PA_BLUETOOTH_HOOK_TRANSPORT_STATE_CHANGED], t);
> +
> +    if (old_any_connected != pa_bluetooth_device_any_transport_connected(t->device))

undefined reference to `pa_bluetooth_device_any_transport_connected'

> +        pa_hook_fire(&t->device->discovery->hooks[PA_BLUETOOTH_HOOK_DEVICE_CONNECTION_CHANGED], t->device);
> +}
> +
> +void pa_bluetooth_transport_put(pa_bluetooth_transport *t) {
> +    transport_state_changed(t, PA_BLUETOOTH_TRANSPORT_STATE_IDLE);
> +}
> +
> +void pa_bluetooth_transport_free(pa_bluetooth_transport *t) {
> +    pa_assert(t);
> +
> +    pa_hashmap_remove(t->device->discovery->transports, t->path);
> +    pa_xfree(t->owner);
> +    pa_xfree(t->path);
> +    pa_xfree(t->config);
> +    pa_xfree(t);
> +}
> +
> +static int bluez5_transport_acquire_cb(pa_bluetooth_transport *t, bool optional, size_t *imtu, size_t *omtu) {
> +    DBusMessage *m, *r;
> +    DBusError err;
> +    int ret;
> +    uint16_t i, o;
> +    const char *method = optional ? "TryAcquire" : "Acquire";
> +
> +    pa_assert(t);
> +    pa_assert(t->device);
> +    pa_assert(t->device->discovery);
> +
> +    pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path, BLUEZ_MEDIA_TRANSPORT_INTERFACE, method));
> +
> +    dbus_error_init(&err);
> +
> +    r = dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection), m, -1, &err);
> +    if (!r) {
> +        if (optional && pa_streq(err.name, "org.bluez.Error.NotAvailable"))
> +            pa_log_info("Failed optional acquire of unavailable transport %s", t->path);
> +        else
> +            pa_log_error("Transport %s() failed for transport %s (%s)", method, t->path, err.message);
> +
> +        dbus_error_free(&err);
> +        return -1;
> +    }
> +
> +    if (!dbus_message_get_args(r, &err, DBUS_TYPE_UNIX_FD, &ret, DBUS_TYPE_UINT16, &i, DBUS_TYPE_UINT16, &o,
> +                               DBUS_TYPE_INVALID)) {
> +        pa_log_error("Failed to parse %s() reply: %s", method, err.message);
> +        dbus_error_free(&err);
> +        ret = -1;
> +        goto finish;
> +    }
> +
> +    if (imtu)
> +        *imtu = i;
> +
> +    if (omtu)
> +        *omtu = o;
> +
> +finish:
> +    dbus_message_unref(r);
> +    return ret;
> +}
> +
> +static void bluez5_transport_release_cb(pa_bluetooth_transport *t) {
> +    DBusMessage *m;
> +    DBusError err;
> +
> +    pa_assert(t);
> +    pa_assert(t->device);
> +    pa_assert(t->device->discovery);
> +
> +    dbus_error_init(&err);
> +
> +    if (t->state <= PA_BLUETOOTH_TRANSPORT_STATE_IDLE) {
> +        pa_log_info("Transport %s auto-released by BlueZ or already released", t->path);
> +        return;
> +    }
> +
> +    pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path, BLUEZ_MEDIA_TRANSPORT_INTERFACE, "Release"));
> +    dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection), m, -1, &err);
> +
> +    if (dbus_error_is_set(&err)) {
> +        pa_log_error("Failed to release transport %s: %s", t->path, err.message);
> +        dbus_error_free(&err);
> +    } else
> +        pa_log_info("Transport %s released", t->path);
> +}
> +
>  static pa_bluetooth_device* device_create(pa_bluetooth_discovery *y, const char *path) {
>      pa_bluetooth_device *d;
>  
> @@ -93,8 +217,23 @@ pa_bluetooth_device* pa_bluetooth_discovery_get_device_by_address(pa_bluetooth_d
>  }
>  
>  static void device_free(pa_bluetooth_device *d) {
> +    unsigned i;
> +
>      pa_assert(d);
>  
> +    for (i = 0; i < PA_BLUETOOTH_PROFILE_COUNT; i++) {
> +        pa_bluetooth_transport *t;
> +
> +        if (!(t = d->transports[i]))
> +            continue;
> +
> +        d->transports[i] = NULL;
> +        transport_state_changed(t, PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED);
> +        /* TODO: check if there is no risk of calling
> +         * transport_connection_changed_cb() when the transport is already freed */
> +        pa_bluetooth_transport_free(t);

IMO pa_bluetooth_transport_free() should be called by the transport
code, because the backend also creates the transport. The backend may
need a callback for getting notified about the device going away. Or
perhaps there should be pa_bluetooth_transport_kill(), with a kill()
callback in the backend. This would be similar to how sink inputs are
killed if the sink they're connected to goes away.

If the backend is responsible for calling pa_bluetooth_transport_free(),
the core should still ensure that t->device is set to NULL and that the
transport is removed from discovery->transports (feel free to create
pa_bluetooth_transport_unlink() for this purpose, if you want).

As for the TODO item, I don't know what it means. There is no
transport_connection_changed_cb() function.

> +    }
> +
>      d->discovery = NULL;
>      pa_xfree(d->path);
>      pa_xfree(d->alias);
> @@ -189,6 +328,7 @@ pa_bluetooth_discovery* pa_bluetooth_discovery_get(pa_core *c) {
>      PA_REFCNT_INIT(y);
>      y->core = c;
>      y->devices = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
> +    y->transports = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
>  
>      for (i = 0; i < PA_BLUETOOTH_HOOK_MAX; i++)
>          pa_hook_init(&y->hooks[i], y);
> @@ -249,6 +389,11 @@ void pa_bluetooth_discovery_unref(pa_bluetooth_discovery *y) {
>          pa_hashmap_free(y->devices, NULL);
>      }
>  
> +    if (y->transports) {
> +        pa_assert(pa_hashmap_isempty(y->transports));
> +        pa_hashmap_free(y->transports, NULL);
> +    }
> +
>      if (y->connection) {
>          pa_dbus_remove_matches(pa_dbus_connection_get(y->connection),
>              "type='signal',sender='org.freedesktop.DBus',interface='org.freedesktop.DBus',member='NameOwnerChanged'"
> diff --git a/src/modules/bluetooth/bluez5-util.h b/src/modules/bluetooth/bluez5-util.h
> index 18b20c4..c3f2f2f 100644
> --- a/src/modules/bluetooth/bluez5-util.h
> +++ b/src/modules/bluetooth/bluez5-util.h
> @@ -24,14 +24,50 @@
>  
>  #include <pulsecore/core.h>
>  
> +typedef struct pa_bluetooth_transport pa_bluetooth_transport;
>  typedef struct pa_bluetooth_device pa_bluetooth_device;
>  typedef struct pa_bluetooth_discovery pa_bluetooth_discovery;
>  
>  typedef enum pa_bluetooth_hook {
>      PA_BLUETOOTH_HOOK_DEVICE_CONNECTION_CHANGED,          /* Call data: pa_bluetooth_device */
> +    PA_BLUETOOTH_HOOK_TRANSPORT_STATE_CHANGED,            /* Call data: pa_bluetooth_transport */
>      PA_BLUETOOTH_HOOK_MAX
>  } pa_bluetooth_hook_t;
>  
> +typedef enum profile {
> +    PROFILE_A2DP_SINK,
> +    PROFILE_A2DP_SOURCE,
> +    PROFILE_OFF

Missing PA_BLUETOOTH_ prefix.

> +} pa_bluetooth_profile_t;
> +#define PA_BLUETOOTH_PROFILE_COUNT PROFILE_OFF
> +
> +typedef enum pa_bluetooth_transport_state {
> +    PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED,
> +    PA_BLUETOOTH_TRANSPORT_STATE_IDLE,
> +    PA_BLUETOOTH_TRANSPORT_STATE_PLAYING
> +} pa_bluetooth_transport_state_t;
> +
> +typedef int (*pa_bluetooth_transport_acquire_cb)(pa_bluetooth_transport *t, bool optional, size_t *imtu, size_t *omtu);
> +typedef void (*pa_bluetooth_transport_release_cb)(pa_bluetooth_transport *t);
> +
> +struct pa_bluetooth_transport {
> +    pa_bluetooth_device *device;
> +
> +    char *owner;
> +    char *path;
> +    pa_bluetooth_profile_t profile;
> +
> +    uint8_t codec;
> +    uint8_t *config;
> +    size_t config_size;
> +
> +    pa_bluetooth_transport_state_t state;
> +
> +    pa_bluetooth_transport_acquire_cb acquire;
> +    pa_bluetooth_transport_release_cb release;
> +    void *userdata;
> +};
> +
>  struct pa_bluetooth_device {
>      pa_bluetooth_discovery *discovery;
>  
> @@ -43,8 +79,15 @@ struct pa_bluetooth_device {
>      char *remote;
>      char *local;
>      int class_of_device;
> +
> +    pa_bluetooth_transport *transports[PA_BLUETOOTH_PROFILE_COUNT];
>  };
>  
> +pa_bluetooth_transport *pa_bluetooth_transport_new(pa_bluetooth_device *d, const char *owner, const char *path,
> +                                                          pa_bluetooth_profile_t p, const uint8_t *config, size_t size);

Unaligned parameter list.

-- 
Tanu



[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux