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

[João Paulo Rechi Vita]

On Fri, Jul 26, 2013 at 8:56 AM, Tanu Kaskinen
<tanu.kaskinen at linux.intel.com> wrote:
>
> 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). */
>

I don't know, this was introduced in ebadda816d2f6faaf0130c6d4482be34b0c9b57d.

>> +        }
>> +
>> +        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.
>

Oops.

-- 
João Paulo Rechi Vita
http://about.me/jprvita