[pulseaudio-discuss] [PATCH v0 12/20] bluetooth: Acquire transport when becomes available

[Mikel Astiz]

Hi Luiz,

On Wed, Aug 22, 2012 at 3:20 PM, Luiz Augusto von Dentz
<luiz.dentz at gmail.com> wrote:
> Hi Mikel,
>
> On Wed, Aug 22, 2012 at 11:08 AM, Mikel Astiz <mikel.astiz.oss at gmail.com> wrote:
>> From: Mikel Astiz <mikel.astiz at bmw-carit.de>
>>
>> Try to acquire the transport as soon as the audio stream is started,
>> along with the availability flag update.
>> ---
>>  src/modules/bluetooth/module-bluetooth-device.c |   18 ++++++++++++++++++
>>  1 files changed, 18 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/modules/bluetooth/module-bluetooth-device.c b/src/modules/bluetooth/module-bluetooth-device.c
>> index 8a38053..c1a88b0 100644
>> --- a/src/modules/bluetooth/module-bluetooth-device.c
>> +++ b/src/modules/bluetooth/module-bluetooth-device.c
>> @@ -1248,6 +1248,7 @@ static pa_port_available_t audio_state_to_availability(pa_bt_audio_state_t state
>>  static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *userdata) {
>>      DBusError err;
>>      struct userdata *u;
>> +    bool acquire = FALSE;
>>
>>      pa_assert(bus);
>>      pa_assert(m);
>> @@ -1324,6 +1325,8 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
>>
>>              pa_assert_se(port = pa_hashmap_get(u->card->ports, "hfgw-input"));
>>              pa_device_port_set_available(port, available);
>> +
>> +            acquire = (available == PA_PORT_AVAILABLE_YES && u->profile == PROFILE_HFGW);
>>          }
>>      } else if (dbus_message_is_signal(m, "org.bluez.Headset", "PropertyChanged")) {
>>          pa_bt_audio_state_t state = parse_state_property_change(m);
>> @@ -1337,6 +1340,8 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
>>
>>              pa_assert_se(port = pa_hashmap_get(u->card->ports, "hsp-input"));
>>              pa_device_port_set_available(port, available);
>> +
>> +            acquire = (available == PA_PORT_AVAILABLE_YES && u->profile == PROFILE_HSP);
>>          }
>>      } else if (dbus_message_is_signal(m, "org.bluez.AudioSource", "PropertyChanged")) {
>>          pa_bt_audio_state_t state = parse_state_property_change(m);
>> @@ -1347,6 +1352,8 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
>>
>>              pa_assert_se(port = pa_hashmap_get(u->card->ports, "a2dp-input"));
>>              pa_device_port_set_available(port, available);
>> +
>> +            acquire = (available == PA_PORT_AVAILABLE_YES && u->profile == PROFILE_A2DP_SOURCE);
>>          }
>>      } else if (dbus_message_is_signal(m, "org.bluez.AudioSink", "PropertyChanged")) {
>>          pa_bt_audio_state_t state = parse_state_property_change(m);
>> @@ -1357,9 +1364,20 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
>>
>>              pa_assert_se(port = pa_hashmap_get(u->card->ports, "a2dp-output"));
>>              pa_device_port_set_available(port, available);
>> +
>> +            acquire = (available == PA_PORT_AVAILABLE_YES && u->profile == PROFILE_A2DP);
>>          }
>>      }
>>
>> +    if (acquire)
>> +        if (bt_transport_acquire(u, FALSE) >= 0) {
>> +            if (u->source)
>> +                pa_source_suspend(u->source, FALSE, PA_SUSPEND_IDLE);
>> +
>> +            if (u->sink)
>> +                pa_sink_suspend(u->sink, FALSE, PA_SUSPEND_IDLE);
>> +        }
>> +
>>  fail:
>>      dbus_error_free(&err);
>>
>> --
>> 1.7.7.6
>
>
> Hmm, this is in case that the profile is already set but for some
> reason acquired failed? Isn't it better leave this for port
> availability to trigger acquire?

Either acquire failed or the stream was suspended, for example from the UI.

If we have this transition to Playing, the stream is already active so
according to the proposed mapping, we need to be consistent with the
exposed state, meaning that the suspend flag is removed immediately
(which requires acquiring the transport). From the cover-letter: "The
sink-sources will be created as soon as the card profile is set, but
they will be suspended unless the audio is streaming/playing".

I think this direct mapping is important so other modules (and UIs)
can suspend/resume SCO easily.

Moving this code to module-bluetooth-profile would certainly be
possible but I think it adds some complexity and I don't see any
benefit right now. For example patch v0 20/20 would be more
difficult/ugly to implement.

Do we know of any case where the streaming transport shouldn't be acquired?

Cheers,
Mikel