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
