[pulseaudio-discuss] [PATCH v2] bluetooth: ofono: Use Acquire method if available

[Luiz Augusto von Dentz]

Hi Georg,

On Mon, May 8, 2017 at 9:28 PM, Georg Chini <georg at chini.tk> wrote:
> On 08.05.2017 10:37, Luiz Augusto von Dentz wrote:
>>
>> From: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
>>
>> Attempt to use Acquire method if available falling back to Connect in
>> case it fails.
>> ---
>>   src/modules/bluetooth/backend-ofono.c | 59
>> +++++++++++++++++++++++++----------
>>   1 file changed, 43 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/modules/bluetooth/backend-ofono.c
>> b/src/modules/bluetooth/backend-ofono.c
>> index 6e9a366..d098402 100644
>> --- a/src/modules/bluetooth/backend-ofono.c
>> +++ b/src/modules/bluetooth/backend-ofono.c
>> @@ -150,6 +150,46 @@ static int socket_accept(int sock)
>>       return 0;
>>   }
>>   +static int card_acquire(struct hf_audio_card *card) {
>> +    pa_bluetooth_transport *t = card->transport;
>> +    DBusMessage *m, *r;
>> +    DBusError err;
>> +
>> +    if (card->connecting)
>> +        return -EAGAIN;
>> +
>> +    /* Try acquiring the stream first */
>> +    dbus_error_init(&err);
>> +    pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
>> "org.ofono.HandsfreeAudioCard", "Acquire"));
>> +    r =
>> dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(card->backend->connection),
>> m, -1, &err);
>> +    if (!r) {
>> +        if (!pa_streq(err.name, DBUS_ERROR_UNKNOWN_METHOD)) {
>> +            pa_log_error("Failed to acquire %s: %s", err.name,
>> err.message);
>> +            return -1;
>> +        }
>> +    } else if ((dbus_message_get_args(r, NULL,
>> +                         DBUS_TYPE_UNIX_FD, &card->fd,
>> +                         DBUS_TYPE_BYTE, &card->codec,
>> +                         DBUS_TYPE_INVALID) == true)) {
>> +        return card->fd;
>> +    } else
>> +        return -1;
>> +
>> +    /* Fallback to Connect as this might be an old version of ofono */
>> +    card->connecting = true;
>> +
>> +    dbus_error_init(&err);
>> +    pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
>> "org.ofono.HandsfreeAudioCard", "Connect"));
>> +    r =
>> dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(card->backend->connection),
>> m, -1, &err);
>> +    if (!r)
>> +        return -1;
>> +
>> +    if (card->connecting)
>> +        return -EAGAIN;
>> +
>> +    return card->fd;
>> +}
>> +
>>   static int hf_audio_agent_transport_acquire(pa_bluetooth_transport *t,
>> bool optional, size_t *imtu, size_t *omtu) {
>>       struct hf_audio_card *card = t->userdata;
>>       int err;
>> @@ -157,22 +197,9 @@ static int
>> hf_audio_agent_transport_acquire(pa_bluetooth_transport *t, bool opti
>>       pa_assert(card);
>>         if (!optional && card->fd < 0) {
>> -        DBusMessage *m, *r;
>> -        DBusError derr;
>> -
>> -        if (card->connecting)
>> -            return -EAGAIN;
>> -
>> -        card->connecting = true;
>> -
>> -        dbus_error_init(&derr);
>> -        pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path,
>> "org.ofono.HandsfreeAudioCard", "Connect"));
>> -        r =
>> dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(card->backend->connection),
>> m, -1, &derr);
>> -        if (!r)
>> -            return -1;
>> -
>> -        if (card->connecting)
>> -            return -EAGAIN;
>> +        err = card_acquire(card);
>> +        if (err < 0)
>> +            return err;
>>       }
>>         /* The correct block size should take into account the SCO MTU
>> from
>
>
> In general, it looks OK. However, what seems missing are two things which
> are normally
> done from hf_audio_agent_new_connection():
>
> 1) card->transport->codec is not set. Or did you mean to use
> card->transport->codec
> in the dbus_message_get_args() call?

It is there already:

    } else if ((dbus_message_get_args(r, NULL,
                         DBUS_TYPE_UNIX_FD, &card->fd,
                         DBUS_TYPE_BYTE, &card->codec,
                         DBUS_TYPE_INVALID) == true)) {

> 2) pa_bluetooth_transport_set_state() is not called, so the transport state
> is not updated
> properly.
>
> The call to pa_bluetooth_transport_set_state() is a bit of a problem,
> because it is expected
> to be run from the main thread and the transport_acquired() function is
> called from both
> threads. Maybe you need to check if you are in main or I/O thread and either
> call it directly
> or send a message to the main thread.

And none of the existing transport do that anyway, so that perhaps
shall be handled separately as it is not unique to ofono backend.

>
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss



-- 
Luiz Augusto von Dentz