[pulseaudio-discuss] [PATCH v4 36/41] bluetooth: Handle changes to BlueZ 5 transports state

[João Paulo Rechi Vita]

On Sat, Sep 21, 2013 at 11:50 AM, Tanu Kaskinen
<tanu.kaskinen at linux.intel.com> wrote:
> On Wed, 2013-09-18 at 16:17 -0500, 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 d9e1fc6..2eaeb77 100644
>> --- a/src/modules/bluetooth/module-bluez5-device.c
>> +++ b/src/modules/bluetooth/module-bluez5-device.c
>> @@ -97,6 +97,7 @@ struct userdata {
>>      pa_core *core;
>>
>>      pa_hook_slot *device_connection_changed_slot;
>> +    pa_hook_slot *transport_state_changed_slot;
>>
>>      pa_bluetooth_discovery *discovery;
>>      pa_bluetooth_device *device;
>> @@ -1670,6 +1671,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, 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);
>
> This is now the third time I ask you to add the following 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've answered you before, this line was added by Mikel at some point
to the BlueZ 4 support. This whole code series is a refactor, I think
we shouldn't block this series because of issues like this one, which
are from the original code and can be very well addressed later on.
And actually, for the matter of adding this comment it makes more
sense that you add it in a separate commit, since the comment is
coming from you, not from me.

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