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

[Georg Chini]

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?

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.