[pulseaudio-discuss] [RFCv3 18/20] bluetooth: Call HandsfreeAudioCard.Connect() on non-optional acquire

[Mikel Astiz]

Hi João,

On Mon, Apr 22, 2013 at 3:30 PM, João Paulo Rechi Vita
<jprvita at openbossa.org> wrote:
> On Mon, Apr 22, 2013 at 4:43 AM, Mikel Astiz <mikel.astiz.oss at gmail.com> wrote:
>> Hi João Paulo,
>>
>> On Mon, Apr 22, 2013 at 5:07 AM,  <jprvita at gmail.com> wrote:
>>> From: João Paulo Rechi Vita <jprvita at openbossa.org>
>>>
>>> When the HFP sink/source is resumed and the transport is not acquired a
>>> new Audio Connection should be started by the HF. This is needed to
>>> support audio call transfer from the AG to the HF triggered by the HF.
>>> ---
>>>  src/modules/bluetooth/bluetooth-util.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c
>>> index a00dac7..0fd4c8e 100644
>>> --- a/src/modules/bluetooth/bluetooth-util.c
>>> +++ b/src/modules/bluetooth/bluetooth-util.c
>>> @@ -1873,6 +1873,12 @@ int pa_bluetooth_transport_acquire(pa_bluetooth_transport *t, bool optional, siz
>>>          if (t->profile == PROFILE_HFGW) {
>>>              struct handsfree_card *hf_card = pa_hashmap_get(t->device->discovery->hf_cards, t->path);
>>>
>>> +            if (!optional) {
>>> +                pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path, "org.ofono.HandsfreeAudioCard", "Connect"));
>>> +                pa_assert_se(dbus_connection_send(pa_dbus_connection_get(t->device->discovery->connection), m, NULL));
>>> +                return -1;
>>> +            }
>>> +
>>
>> I don't see how this improves the problem considering you're supposed
>> to block and return the fd, along with imtu and omtu.
>>
>
> Sorry, I wrote this a bit late in the evening and didn't make myself
> clear enough. The solution to the problem is actually implemented by
> this commit plus the previous one. The whole  idea is that when the

It's not clear to me what v3 17/20 is achieving, since I thought
returning -1 would be enough without having to skip the call to
process_msg(). If this is assumption is wrong, patch v3 17/20 should
probably be considered a fix, independent of the rest of the patchset,
and be submitted to the stable branch.

> sink/source is resumed we do a non-optional acquire, and if that
> acquire fails we do not resume it (what makes sense even without
> taking this problem into consideration, since there is no stream fd to
> be handled). This behavior is implemented in the previous commit.

Your description above seems to (or should) fit the existing approach,
but you still need the blocking call. Otherwise you can't know whether
the non-optional acquire failed or not. And this information is
needed, because the sink/source resume depends on this operation, and
should fail otherwise.

>
> In this commit we handle the non-optional acquire for (PROFILE_HFGW &&
> BLUEZ_VERSION_5), calling Connect() and failing the acquire, since we
> can't return a valid stream fd at this point. The Connect() will

This is exactly what is not acceptable without major changes in
module-bluetooth-device. It was possible with BlueZ 4, and it seems to
me that it's more difficult with oFono only because the Connect()
method does not return the fd.

> trigger an AT+BCC command to the AG and the AG will start an audio
> connection establishment that will end up in a call to
> NewConnection(), so we'll handle this through the regular
> NewConnection() flow.

This would be valid provided that you could block the thread until the
NewConnection() is received.

>
> This idea has worked on my tests, the only drawback is that since
> we're not resuming the sink/source right away, the
> sink-input/source-output might get moved to the fallback sink/source.
> I'm not sure if this is a problem from the client perspective.

I believe it's a big issue not only from the practical perspective,
but also from the design's point of view. All resumes would initially
fail and then get automatically triggered afterwards.

>
> Regarding blocking at this point, I think this may not be a good idea
> because this is called from I/O thread context.

It's been like this for some time and there's no known related issues.
Note that the I/O thread can't do much while an acquire operation is
in progress.

Cheers,
Mikel