[pulseaudio-discuss] [RFCv2 13/17] bluetooth: Only call *Acquire() if using BlueZ' Media API

[Mikel Astiz]

Hi João Paulo,

On Thu, Apr 18, 2013 at 4:04 PM, João Paulo Rechi Vita
<jprvita at openbossa.org> wrote:
> On Thu, Apr 18, 2013 at 10:43 AM, Mikel Astiz <mikel.astiz.oss at gmail.com> wrote:
>> Hi João Paulo,
>>
>> On Mon, Apr 15, 2013 at 11:53 PM,  <jprvita at gmail.com> wrote:
>>> From: João Paulo Rechi Vita <jprvita at openbossa.org>
>>>
>>> When the transport backend is oFono the stream file descriptor is passed
>>> through the NewConnection() method call. Additionally, set the kernel
>>> default SCO MTU value for block size (since this information is not
>>> available) and the transport codec, which also comes in the
>>> NewConnection() method call.
>>> ---
>>>  src/modules/bluetooth/bluetooth-util.c | 20 ++++++++++++++++++++
>>>  1 file changed, 20 insertions(+)
>>>
>>> diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c
>>> index ae98029..d72137b 100644
>>> --- a/src/modules/bluetooth/bluetooth-util.c
>>> +++ b/src/modules/bluetooth/bluetooth-util.c
>>> @@ -1854,6 +1854,26 @@ int pa_bluetooth_transport_acquire(pa_bluetooth_transport *t, bool optional, siz
>>>      } else {
>>>          pa_assert(t->device->discovery->version == BLUEZ_VERSION_5);
>>>
>>> +        if (t->profile == PROFILE_HFGW) {
>>> +            struct handsfree_card *hf_card = pa_hashmap_get(t->device->discovery->hf_cards, t->path);
>>> +
>>> +            /* The correct block size should take into account the SCO MTU from
>>> +             * the Bluetooth adapter and (for adapters in the USB bus) the MxPS
>>> +             * value from the Isoc USB endpoint in use by btusb and should be
>>> +             * made available to userspace by the Bluetooth kernel subsystem.
>>> +             * The empiric value will be used meanwhile. */
>>
>> Why not take the MTU from the socket options?
>
> The MTU informed through the SCO socket option informs the adapter
> MTU, it doesn't take the USB endpoint MxPS into consideration. So if
> we use that value the USB endpoint gets flooded (I imagine) and resets
> the Bluetooth adapter. Vinicius is investigating this issue, but
> hardcoding this value to 48 is the best we can do so far :(

You're right that it doesn't take the MxPS into consideration but
neither does the hardcoded 48.

My experience is that 96 works better than 48, for example, specially
when 2 SCO streams exist.

>
>>
>>> +            if (imtu)
>>> +                *imtu = 48;
>>> +            if (omtu)
>>> +                *omtu = 48;
>>> +
>>> +            if (hf_card) {
>>> +                t->codec = hf_card->codec;
>>> +                return hf_card->fd;
>>
>> As already mentioned before, this is not enough. You're handling the
>> optional case only, while PA also relies on triggering an SCO
>> connection actively and blocking until we get the fd.
>>
>
> Yes, I left this use case to be handled later on and I'm working on it

That's fine but we should definitely address this issue before merging
this patchset.

> right now, to see what we can do. How are you doing to trigger this?
> I'm trying with module-sine, setting the bluetooth sink as the
> fallback one, but this doesn't seem to trigger a sink resume.

You can use pacmd and commands like suspend-source/sink.

The reason is that within this role (role HF, PA-card-profile "hfgw"),
the suspend is considered user-driven (PA_SUSPEND_USER), and won't
automatically resume.

Cheers,
Mikel