[pulseaudio-discuss] [PATCH 55/56] bluetooth: Handle changes to BlueZ 5 transports state

[Tanu Kaskinen]

On Fri, 2013-07-12 at 15:07 -0300, jprvita at gmail.com wrote:
> From: João Paulo Rechi Vita <jprvita at openbossa.org>
> 
> ---
>  src/modules/bluetooth/module-bluez5-device.c | 78 ++++++++++++++++++++++++++++
>  1 file changed, 78 insertions(+)
> 
> diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
> index a5ed128..d35aca3 100644
> --- a/src/modules/bluetooth/module-bluez5-device.c
> +++ b/src/modules/bluetooth/module-bluez5-device.c
> @@ -98,6 +98,7 @@ struct userdata {
>      pa_modargs *modargs;
>  
>      pa_hook_slot *device_connection_changed_slot;
> +    pa_hook_slot *transport_state_changed_slot;
>  
>      pa_bluetooth_discovery *discovery;
>      pa_bluetooth_device *device;
> @@ -1686,6 +1687,62 @@ static int add_card(struct userdata *u) {
>  }
>  
>  /* Run from main thread */
> +static void handle_transport_state_change(struct userdata *u, struct pa_bluetooth_transport *t) {
> +    bool acquire = false;
> +    bool release = false;
> +    pa_card_profile *cp;
> +    pa_device_port *port;
> +
> +    pa_assert(u);
> +    pa_assert(t);
> +
> +    /* Update profile availability */
> +    if (!(cp = pa_hashmap_get(u->card->profiles, pa_bluetooth_profile_to_string(t->profile))))
> +        return;
> +    pa_card_profile_set_available(cp, pa_bluetooth_transport_state_to_availability(t->state));
> +
> +    /* Update port availability */
> +    pa_assert_se(port = pa_hashmap_get(u->card->ports, u->output_port_name));
> +    pa_device_port_set_available(port, get_port_availability(u, PA_DIRECTION_OUTPUT));
> +    pa_assert_se(port = pa_hashmap_get(u->card->ports, u->input_port_name));
> +    pa_device_port_set_available(port, get_port_availability(u, PA_DIRECTION_INPUT));
> +
> +    /* Acquire or release transport as needed */
> +    acquire = (t->state == PA_BLUETOOTH_TRANSPORT_STATE_PLAYING && u->profile == t->profile);
> +    release = (t->state != PA_BLUETOOTH_TRANSPORT_STATE_PLAYING && u->profile == t->profile);
> +
> +    if (acquire && transport_acquire(u, true) >= 0) {
> +        if (u->source) {
> +            pa_log_debug("Resuming source %s because its transport state changed to playing", u->source->name);
> +            pa_source_suspend(u->source, false, PA_SUSPEND_IDLE|PA_SUSPEND_USER);

Do you know why PA_SUSPEND_IDLE is being used here? It looks wrong. The
bluez4 code uses it too, though. The reason is probably to allow
module-loopback to wake up if both of its endpoints are suspended due to
being idle. Proposed comment:

/* We remove the IDLE suspend cause, because otherwise module-loopback
doesn't uncork its streams. FIXME: Messing with the IDLE suspend cause
here is wrong, the correct way to handle this would probably be to
uncork the loopback streams not only when the other end is unsuspended,
but also when the other end's suspend cause changes to IDLE only
(currently there's no notification mechanism for suspend cause changes,
though). */

> +        }
> +
> +        if (u->sink) {
> +            pa_log_debug("Resuming sink %s because its transport state changed to playing", u->sink->name);
> +            pa_sink_suspend(u->sink, false, PA_SUSPEND_IDLE|PA_SUSPEND_USER);
> +        }
> +    }
> +
> +    if (release && u->transport_acquired) {
> +	/* FIXME: this release is racy, since the audio stream might have
> +	 * been set up again in the meantime (but not processed yet by PA).
> +	 * BlueZ should probably release the transport automatically, and in
> +	 * that case we would just mark the transport as released */

Tabs used for indentation.

-- 
Tanu