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
