[pulseaudio-commits] 8 commits - src/modules

Georg Chini gchini at kemper.freedesktop.org
Thu May 4 11:35:59 UTC 2017


 src/modules/bluetooth/backend-ofono.c          |   36 +++++---
 src/modules/bluetooth/module-bluez5-device.c   |  109 ++++++++++++++++++++-----
 src/modules/bluetooth/module-bluez5-discover.c |    2 
 3 files changed, 118 insertions(+), 29 deletions(-)

New commits:
commit 998dfdf4cc85d3475c65b4863e18f83efd96e056
Author: Georg Chini <georg at chini.tk>
Date:   Thu May 4 13:14:51 2017 +0200

    bluez5-device: Correctly handle suspend/resume cyle for audio gateway role of ofono backend
    
    When the ofono backend released a tranport during suspend of sink or source, the
    transport state was not changed to IDLE. Therefore pa_bluetooth_transport_set_state()
    would return immediately when trying to resume. Even though the transport was acquired
    correctly, setup_stream() would never be called and the resume failed.
    
    This patch sets the transport state to IDLE when the transport is released. On resume,
    the first call to transport_acquire() will be done from the message handler of the
    *_SET_STATE message when source or sink are set to RUNNING. This call will only request
    the setup of the connection, so setup_stream() cannot be called.
    When the transport changes the state to PLAYING in hf_audio_agent_new_connection(),
    handle_transport_state_change() is called. Because the sink or source state is already
    RUNNING, the pa_{source,sink}_suspend() call will not lead to a state change message
    and the I/O thread must be signaled explicitely to setup the stream.
    
    The first setup of the device would also fail, which was only visible when the profile
    was restored after connecting the headset. When trying to restore the headset_head_unit
    profile, the profile was shortly set to off, so the headset always returned to a2dp.
    
    This patch allows a delayed setup for the headset_head_unit profile, so that the profile
    can successfully be restored.

diff --git a/src/modules/bluetooth/backend-ofono.c b/src/modules/bluetooth/backend-ofono.c
index c39c6ce2..6e9a3664 100644
--- a/src/modules/bluetooth/backend-ofono.c
+++ b/src/modules/bluetooth/backend-ofono.c
@@ -161,7 +161,7 @@ static int hf_audio_agent_transport_acquire(pa_bluetooth_transport *t, bool opti
         DBusError derr;
 
         if (card->connecting)
-            return -1;
+            return -EAGAIN;
 
         card->connecting = true;
 
@@ -172,7 +172,7 @@ static int hf_audio_agent_transport_acquire(pa_bluetooth_transport *t, bool opti
             return -1;
 
         if (card->connecting)
-            return -1;
+            return -EAGAIN;
     }
 
     /* The correct block size should take into account the SCO MTU from
diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
index 2f0ec976..867def74 100644
--- a/src/modules/bluetooth/module-bluez5-device.c
+++ b/src/modules/bluetooth/module-bluez5-device.c
@@ -80,6 +80,14 @@ enum {
     BLUETOOTH_MESSAGE_MAX
 };
 
+enum {
+    PA_SOURCE_MESSAGE_SETUP_STREAM = PA_SOURCE_MESSAGE_MAX,
+};
+
+enum {
+    PA_SINK_MESSAGE_SETUP_STREAM = PA_SINK_MESSAGE_MAX,
+};
+
 typedef struct bluetooth_msg {
     pa_msgobject parent;
     pa_card *card;
@@ -112,6 +120,7 @@ struct userdata {
     pa_bluetooth_device *device;
     pa_bluetooth_transport *transport;
     bool transport_acquired;
+    bool stream_setup_done;
 
     pa_card *card;
     pa_sink *sink;
@@ -731,6 +740,7 @@ static void teardown_stream(struct userdata *u) {
     }
 
     pa_log_debug("Audio stream torn down");
+    u->stream_setup_done = false;
 }
 
 static int transport_acquire(struct userdata *u, bool optional) {
@@ -743,7 +753,7 @@ static int transport_acquire(struct userdata *u, bool optional) {
 
     u->stream_fd = u->transport->acquire(u->transport, optional, &u->read_link_mtu, &u->write_link_mtu);
     if (u->stream_fd < 0)
-        return -1;
+        return u->stream_fd;
 
     u->transport_acquired = true;
     pa_log_info("Transport %s acquired: fd %d", u->transport->path, u->stream_fd);
@@ -765,6 +775,11 @@ static void transport_release(struct userdata *u) {
     u->transport_acquired = false;
 
     teardown_stream(u);
+
+    /* Set transport state to idle if this was not already done by the remote end closing
+     * the file descriptor. Only do this when called from the I/O thread */
+    if (pa_thread_mq_get() != NULL && u->transport->state == PA_BLUETOOTH_TRANSPORT_STATE_PLAYING)
+        pa_asyncmsgq_post(pa_thread_mq_get()->outq, PA_MSGOBJECT(u->msg), BLUETOOTH_MESSAGE_STREAM_FD_HUP, NULL, 0, NULL, NULL);
 }
 
 /* Run from I/O thread */
@@ -802,6 +817,10 @@ static void setup_stream(struct userdata *u) {
     struct pollfd *pollfd;
     int one;
 
+    /* return if stream is already set up */
+    if (u->stream_setup_done)
+        return;
+
     pa_log_info("Transport %s resuming", u->transport->path);
 
     transport_config_mtu(u);
@@ -825,11 +844,26 @@ static void setup_stream(struct userdata *u) {
 
     u->read_index = u->write_index = 0;
     u->started_at = 0;
+    u->stream_setup_done = true;
 
     if (u->source)
         u->read_smoother = pa_smoother_new(PA_USEC_PER_SEC, 2*PA_USEC_PER_SEC, true, true, 10, pa_rtclock_now(), true);
 }
 
+/* Called from I/O thread, returns true if the transport was acquired or
+ * a connection was requested successfully. */
+static bool setup_transport_and_stream(struct userdata *u) {
+    int transport_error;
+
+    transport_error = transport_acquire(u, false);
+    if (transport_error < 0) {
+        if (transport_error != -EAGAIN)
+            return false;
+    } else
+        setup_stream(u);
+    return true;
+}
+
 /* Run from IO thread */
 static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t offset, pa_memchunk *chunk) {
     struct userdata *u = PA_SOURCE(o)->userdata;
@@ -865,12 +899,8 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
                         break;
 
                     /* Resume the device if the sink was suspended as well */
-                    if (!u->sink || !PA_SINK_IS_OPENED(u->sink->thread_info.state)) {
-                        if (transport_acquire(u, false) < 0)
-                            failed = true;
-                        else
-                            setup_stream(u);
-                    }
+                    if (!u->sink || !PA_SINK_IS_OPENED(u->sink->thread_info.state))
+                        failed = !setup_transport_and_stream(u);
 
                     /* We don't resume the smoother here. Instead we
                      * wait until the first packet arrives */
@@ -898,6 +928,11 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
 
             return 0;
         }
+
+        case PA_SOURCE_MESSAGE_SETUP_STREAM:
+            setup_stream(u);
+            return 0;
+
     }
 
     r = pa_source_process_msg(o, code, data, offset, chunk);
@@ -970,8 +1005,15 @@ static int add_source(struct userdata *u) {
             case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
                 data.suspend_cause = PA_SUSPEND_USER;
                 break;
-            case PA_BLUETOOTH_PROFILE_A2DP_SINK:
             case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+                /* u->stream_fd contains the error returned by the last transport_acquire()
+                 * EAGAIN means we are waiting for a NewConnection signal */
+                if (u->stream_fd == -EAGAIN)
+                    data.suspend_cause = PA_SUSPEND_USER;
+                else
+                    pa_assert_not_reached();
+                break;
+            case PA_BLUETOOTH_PROFILE_A2DP_SINK:
             case PA_BLUETOOTH_PROFILE_OFF:
                 pa_assert_not_reached();
                 break;
@@ -1029,12 +1071,8 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
                         break;
 
                     /* Resume the device if the source was suspended as well */
-                    if (!u->source || !PA_SOURCE_IS_OPENED(u->source->thread_info.state)) {
-                        if (transport_acquire(u, false) < 0)
-                            failed = true;
-                        else
-                            setup_stream(u);
-                    }
+                    if (!u->source || !PA_SOURCE_IS_OPENED(u->source->thread_info.state))
+                        failed = !setup_transport_and_stream(u);
 
                     break;
 
@@ -1061,6 +1099,10 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
 
             return 0;
         }
+
+        case PA_SINK_MESSAGE_SETUP_STREAM:
+            setup_stream(u);
+            return 0;
     }
 
     r = pa_sink_process_msg(o, code, data, offset, chunk);
@@ -1132,10 +1174,17 @@ static int add_sink(struct userdata *u) {
             case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
                 data.suspend_cause = PA_SUSPEND_USER;
                 break;
+            case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+                /* u->stream_fd contains the error returned by the last transport_acquire()
+                 * EAGAIN means we are waiting for a NewConnection signal */
+                if (u->stream_fd == -EAGAIN)
+                    data.suspend_cause = PA_SUSPEND_USER;
+                else
+                    pa_assert_not_reached();
+                break;
             case PA_BLUETOOTH_PROFILE_A2DP_SINK:
                 /* Profile switch should have failed */
             case PA_BLUETOOTH_PROFILE_A2DP_SOURCE:
-            case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
             case PA_BLUETOOTH_PROFILE_OFF:
                 pa_assert_not_reached();
                 break;
@@ -1292,8 +1341,13 @@ static int setup_transport(struct userdata *u) {
 
     if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE || u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY)
         transport_acquire(u, true); /* In case of error, the sink/sources will be created suspended */
-    else if (transport_acquire(u, false) < 0)
-        return -1; /* We need to fail here until the interactions with module-suspend-on-idle and alike get improved */
+    else {
+        int transport_error;
+
+        transport_error = transport_acquire(u, false);
+        if (transport_error < 0 && transport_error != -EAGAIN)
+            return -1; /* We need to fail here until the interactions with module-suspend-on-idle and alike get improved */
+    }
 
     transport_config(u);
 
@@ -2001,6 +2055,14 @@ static void handle_transport_state_change(struct userdata *u, struct pa_bluetoot
         if (u->source) {
             pa_log_debug("Resuming source %s because its transport state changed to playing", u->source->name);
 
+            /* When the ofono backend resumes source or sink when in the audio gateway role, the
+             * state of source or sink may already be RUNNING before the transport is acquired via
+             * hf_audio_agent_new_connection(), so the pa_source_suspend() call will not lead to a
+             * state change message. In this case we explicitely need to signal the I/O thread to
+             * set up the stream. */
+            if (PA_SOURCE_IS_OPENED(u->source->state))
+                pa_asyncmsgq_send(u->source->asyncmsgq, PA_MSGOBJECT(u->source), PA_SOURCE_MESSAGE_SETUP_STREAM, NULL, 0, NULL);
+
             /* We remove the IDLE suspend cause, because otherwise
              * module-loopback doesn't uncork its streams. FIXME: Messing with
              * the IDLE suspend cause here is wrong, the correct way to handle
@@ -2014,6 +2076,10 @@ static void handle_transport_state_change(struct userdata *u, struct pa_bluetoot
         if (u->sink) {
             pa_log_debug("Resuming sink %s because its transport state changed to playing", u->sink->name);
 
+            /* Same comment as above */
+            if (PA_SINK_IS_OPENED(u->sink->state))
+                pa_asyncmsgq_send(u->sink->asyncmsgq, PA_MSGOBJECT(u->sink), PA_SINK_MESSAGE_SETUP_STREAM, NULL, 0, NULL);
+
             /* FIXME: See the previous comment. */
             pa_sink_suspend(u->sink, false, PA_SUSPEND_IDLE|PA_SUSPEND_USER);
         }
@@ -2215,6 +2281,7 @@ int pa__init(pa_module* m) {
 
     u->msg->parent.process_msg = device_process_msg;
     u->msg->card = u->card;
+    u->stream_setup_done = false;
 
     if (u->profile != PA_BLUETOOTH_PROFILE_OFF)
         if (init_profile(u) < 0)

commit f44f2996b4ca3d8dc3b9b8362825aff5a225c221
Author: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
Date:   Thu May 4 13:12:59 2017 +0200

    bluetooth: ofono: Fix failing to parse valid NewConnection
    
    When suspending due to idle timeout the transport will not change its
    state, also in case the fd is closed due to POLLERR/POLLHUP events
    the release shall check if the fd is still set otherwise it will fail
    to be acquired again.

diff --git a/src/modules/bluetooth/backend-ofono.c b/src/modules/bluetooth/backend-ofono.c
index c8109d9f..c39c6ce2 100644
--- a/src/modules/bluetooth/backend-ofono.c
+++ b/src/modules/bluetooth/backend-ofono.c
@@ -201,14 +201,11 @@ static void hf_audio_agent_transport_release(pa_bluetooth_transport *t) {
 
     pa_assert(card);
 
-    if (t->state <= PA_BLUETOOTH_TRANSPORT_STATE_IDLE) {
+    if (card->fd < 0) {
         pa_log_info("Transport %s already released", t->path);
         return;
     }
 
-    if (card->fd < 0)
-        return;
-
     /* shutdown to make sure connection is dropped immediately */
     shutdown(card->fd, SHUT_RDWR);
     close(card->fd);
@@ -546,7 +543,7 @@ static DBusMessage *hf_audio_agent_new_connection(DBusConnection *c, DBusMessage
 
     card->connecting = false;
 
-    if (!card || codec != HFP_AUDIO_CODEC_CVSD || card->transport->state == PA_BLUETOOTH_TRANSPORT_STATE_PLAYING) {
+    if (!card || codec != HFP_AUDIO_CODEC_CVSD || card->fd >= 0) {
         pa_log_warn("New audio connection invalid arguments (path=%s fd=%d, codec=%d)", path, fd, codec);
         pa_assert_se(r = dbus_message_new_error(m, "org.ofono.Error.InvalidArguments", "Invalid arguments in method call"));
         shutdown(fd, SHUT_RDWR);

commit 826bb358cc818e5687c266becf68d72fb56d08c9
Author: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
Date:   Thu May 4 13:11:27 2017 +0200

    bluetooth: ofono: Close fd if cannot be accepted
    
    Always close fd that cannot be accepted otherwise it will be left open
    without being attached to any transport.

diff --git a/src/modules/bluetooth/backend-ofono.c b/src/modules/bluetooth/backend-ofono.c
index 33fee851..c8109d9f 100644
--- a/src/modules/bluetooth/backend-ofono.c
+++ b/src/modules/bluetooth/backend-ofono.c
@@ -549,6 +549,8 @@ static DBusMessage *hf_audio_agent_new_connection(DBusConnection *c, DBusMessage
     if (!card || codec != HFP_AUDIO_CODEC_CVSD || card->transport->state == PA_BLUETOOTH_TRANSPORT_STATE_PLAYING) {
         pa_log_warn("New audio connection invalid arguments (path=%s fd=%d, codec=%d)", path, fd, codec);
         pa_assert_se(r = dbus_message_new_error(m, "org.ofono.Error.InvalidArguments", "Invalid arguments in method call"));
+        shutdown(fd, SHUT_RDWR);
+        close(fd);
         return r;
     }
 

commit 69c212f8c13794f70899d1fe6baec1b6e3c92032
Author: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
Date:   Thu May 4 12:58:54 2017 +0200

    bluetooth: Auto recover if profile is 'off'
    
    This means something went wrong, which in case of ofono backend it is
    probably due to the profile not connecting immediately, but it can be
    safely restored in that case the transport is playing which means the
    profile has recovered connectivity.

diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
index a96da17d..2f0ec976 100644
--- a/src/modules/bluetooth/module-bluez5-device.c
+++ b/src/modules/bluetooth/module-bluez5-device.c
@@ -2060,8 +2060,14 @@ static pa_hook_result_t transport_state_changed_cb(pa_bluetooth_discovery *y, pa
     if (t == u->transport && t->state <= PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED)
         pa_assert_se(pa_card_set_profile(u->card, pa_hashmap_get(u->card->profiles, "off"), false) >= 0);
 
-    if (t->device == u->device)
+    if (t->device == u->device) {
+        /* Auto recover from errors causing the profile to be set to off */
+        if (u->profile == PA_BLUETOOTH_PROFILE_OFF && t->state == PA_BLUETOOTH_TRANSPORT_STATE_PLAYING) {
+            pa_log_debug("Switching to profile %s", pa_bluetooth_profile_to_string(t->profile));
+            pa_assert_se(pa_card_set_profile(u->card, pa_hashmap_get(u->card->profiles, pa_bluetooth_profile_to_string(t->profile)), false) >= 0);
+        }
         handle_transport_state_change(u, t);
+    }
 
     return PA_HOOK_OK;
 }

commit 956e72135e9ed67a010399761b379c23e81ec3ea
Author: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
Date:   Thu May 4 12:53:32 2017 +0200

    bluetooth: Don't default to native backend
    
    The native backend is limited to HSP only which may not work with devices
    that can only do HFP so if oFono is enabled it shall be used as default.

diff --git a/src/modules/bluetooth/module-bluez5-discover.c b/src/modules/bluetooth/module-bluez5-discover.c
index bc5dbd49..97ff9435 100644
--- a/src/modules/bluetooth/module-bluez5-discover.c
+++ b/src/modules/bluetooth/module-bluez5-discover.c
@@ -93,7 +93,7 @@ static pa_hook_result_t device_connection_changed_cb(pa_bluetooth_discovery *y,
 }
 
 #ifdef HAVE_BLUEZ_5_NATIVE_HEADSET
-const char *default_headset_backend = "native";
+const char *default_headset_backend = "auto";
 #else
 const char *default_headset_backend = "ofono";
 #endif

commit f45fc03e3d5d33d1a3046b9233e3aea8227a3c57
Author: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
Date:   Thu May 4 12:52:18 2017 +0200

    bluetooth: ofono: Detect if Connect has been called
    
    This detects if profile has already been called and we are waiting
    the response.

diff --git a/src/modules/bluetooth/backend-ofono.c b/src/modules/bluetooth/backend-ofono.c
index ff1fbcab..33fee851 100644
--- a/src/modules/bluetooth/backend-ofono.c
+++ b/src/modules/bluetooth/backend-ofono.c
@@ -65,6 +65,7 @@ struct hf_audio_card {
     char *remote_address;
     char *local_address;
 
+    bool connecting;
     int fd;
     uint8_t codec;
 
@@ -156,12 +157,22 @@ static int hf_audio_agent_transport_acquire(pa_bluetooth_transport *t, bool opti
     pa_assert(card);
 
     if (!optional && card->fd < 0) {
-        DBusMessage *m;
+        DBusMessage *m, *r;
+        DBusError derr;
 
+        if (card->connecting)
+            return -1;
+
+        card->connecting = true;
+
+        dbus_error_init(&derr);
         pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path, "org.ofono.HandsfreeAudioCard", "Connect"));
-        pa_assert_se(dbus_connection_send(pa_dbus_connection_get(card->backend->connection), m, NULL));
+        r = dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(card->backend->connection), m, -1, &derr);
+        if (!r)
+            return -1;
 
-        return -1;
+        if (card->connecting)
+            return -1;
     }
 
     /* The correct block size should take into account the SCO MTU from
@@ -533,6 +544,8 @@ static DBusMessage *hf_audio_agent_new_connection(DBusConnection *c, DBusMessage
 
     card = pa_hashmap_get(backend->cards, path);
 
+    card->connecting = false;
+
     if (!card || codec != HFP_AUDIO_CODEC_CVSD || card->transport->state == PA_BLUETOOTH_TRANSPORT_STATE_PLAYING) {
         pa_log_warn("New audio connection invalid arguments (path=%s fd=%d, codec=%d)", path, fd, codec);
         pa_assert_se(r = dbus_message_new_error(m, "org.ofono.Error.InvalidArguments", "Invalid arguments in method call"));

commit cf23f07ec768003442d192cd28ddaf0bd1358962
Author: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
Date:   Thu May 4 12:51:00 2017 +0200

    bluetooth: ofono: Fix asserting when connecting as AG
    
    Check the card Type property instead of assuming ofono would only be used
    for HF role.

diff --git a/src/modules/bluetooth/backend-ofono.c b/src/modules/bluetooth/backend-ofono.c
index e18772d0..ff1fbcab 100644
--- a/src/modules/bluetooth/backend-ofono.c
+++ b/src/modules/bluetooth/backend-ofono.c
@@ -209,6 +209,7 @@ static void hf_audio_agent_card_found(pa_bluetooth_backend *backend, const char
     const char *key, *value;
     struct hf_audio_card *card;
     pa_bluetooth_device *d;
+    pa_bluetooth_profile_t p = PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY;
 
     pa_assert(backend);
     pa_assert(path);
@@ -240,6 +241,9 @@ static void hf_audio_agent_card_found(pa_bluetooth_backend *backend, const char
         } else if (pa_streq(key, "LocalAddress")) {
             pa_xfree(card->local_address);
             card->local_address = pa_xstrdup(value);
+        } else if (pa_streq(key, "Type")) {
+            if (pa_streq(value, "gateway"))
+                p = PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT;
         }
 
         pa_log_debug("%s: %s", key, value);
@@ -253,7 +257,7 @@ static void hf_audio_agent_card_found(pa_bluetooth_backend *backend, const char
         goto fail;
     }
 
-    card->transport = pa_bluetooth_transport_new(d, backend->ofono_bus_id, path, PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY, NULL, 0);
+    card->transport = pa_bluetooth_transport_new(d, backend->ofono_bus_id, path, p, NULL, 0);
     card->transport->acquire = hf_audio_agent_transport_acquire;
     card->transport->release = hf_audio_agent_transport_release;
     card->transport->userdata = card;

commit fe43e726044c58f6c0ad1ca7b8c7fa77ecb7f557
Author: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
Date:   Thu May 4 12:47:58 2017 +0200

    bluetooth: ofono: Don't attempt to connect if fd is ready
    
    If SCO fd is already set don't attempt to connect again.

diff --git a/src/modules/bluetooth/backend-ofono.c b/src/modules/bluetooth/backend-ofono.c
index 755df9e8..e18772d0 100644
--- a/src/modules/bluetooth/backend-ofono.c
+++ b/src/modules/bluetooth/backend-ofono.c
@@ -155,7 +155,7 @@ static int hf_audio_agent_transport_acquire(pa_bluetooth_transport *t, bool opti
 
     pa_assert(card);
 
-    if (!optional) {
+    if (!optional && card->fd < 0) {
         DBusMessage *m;
 
         pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path, "org.ofono.HandsfreeAudioCard", "Connect"));



More information about the pulseaudio-commits mailing list