On Thu, Apr 18, 2013 at 11:50 AM, Mikel Astiz <mikel.astiz.oss at gmail.com> wrote:
> 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.
>
That depends on the adapter and USB, in my machine 96 doesn't work at
all. This problem needs to be solved on the kernel side first, making
an useful value available to userspace.
>>
>>>
>>>> + 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.
>
Ok, thanks for the hint, I'm able to trigger suspend/resume now.
--
Jo?o Paulo Rechi Vita
http://about.me/jprvita
