[pulseaudio-commits] 5 commits - src/modules

Tanu Kaskinen tanuk at kemper.freedesktop.org
Thu Nov 22 14:27:23 PST 2012


 src/modules/bluetooth/bluetooth-util.c            |   46 +++++++++++++++++-----
 src/modules/bluetooth/bluetooth-util.h            |    2 
 src/modules/bluetooth/module-bluetooth-device.c   |   28 +++++++++++--
 src/modules/bluetooth/module-bluetooth-discover.c |    5 --
 4 files changed, 64 insertions(+), 17 deletions(-)

New commits:
commit 41055145d7563c20fd58ed4b0a9b349c2363bb02
Author: Tanu Kaskinen <tanuk at iki.fi>
Date:   Thu Nov 22 15:20:28 2012 +0100

    bluetooth: Unload device module when no audio profiles connected
    
    Without this patch, device modules will be left around after the
    device has been disconnected and when they are reconnected, the
    discovery module will load duplicate device module instances.
    
    BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=57239

diff --git a/src/modules/bluetooth/module-bluetooth-device.c b/src/modules/bluetooth/module-bluetooth-device.c
index 8a9d39f..dd1bb86 100644
--- a/src/modules/bluetooth/module-bluetooth-device.c
+++ b/src/modules/bluetooth/module-bluetooth-device.c
@@ -146,6 +146,7 @@ struct userdata {
     char *accesstype;
     pa_hook_slot *transport_removed_slot;
     pa_hook_slot *device_removed_slot;
+    pa_hook_slot *discovery_slot;
 
     pa_bluetooth_discovery *discovery;
     pa_bool_t auto_connect;
@@ -2522,6 +2523,23 @@ static pa_hook_result_t device_removed_cb(pa_bluetooth_device *d, void *call_dat
     return PA_HOOK_OK;
 }
 
+/* Run from main thread */
+static pa_hook_result_t discovery_hook_cb(pa_bluetooth_discovery *y, const pa_bluetooth_device *d, struct userdata *u) {
+    pa_assert(u);
+    pa_assert(d);
+
+    if (d != u->device)
+        return PA_HOOK_OK;
+
+    if (pa_bluetooth_device_any_audio_connected(d))
+        return PA_HOOK_OK;
+
+    pa_log_debug("Unloading module, because device %s doesn't have any audio profiles connected anymore.", d->path);
+    pa_module_unload(u->core, u->module, true);
+
+    return PA_HOOK_OK;
+}
+
 int pa__init(pa_module* m) {
     pa_modargs *ma;
     uint32_t channels;
@@ -2595,6 +2613,9 @@ int pa__init(pa_module* m) {
     u->device_removed_slot = pa_hook_connect(&device->hooks[PA_BLUETOOTH_DEVICE_HOOK_REMOVED], PA_HOOK_NORMAL,
                                              (pa_hook_cb_t) device_removed_cb, u);
 
+    u->discovery_slot = pa_hook_connect(pa_bluetooth_discovery_hook(u->discovery), PA_HOOK_NORMAL,
+                                        (pa_hook_cb_t) discovery_hook_cb, u);
+
     u->device = device;
 
     /* Add the card structure. This will also initialize the default profile */
@@ -2684,10 +2705,11 @@ void pa__done(pa_module *m) {
 
     stop_thread(u);
 
-    if (u->device_removed_slot) {
+    if (u->discovery_slot)
+        pa_hook_slot_free(u->discovery_slot);
+
+    if (u->device_removed_slot)
         pa_hook_slot_free(u->device_removed_slot);
-        u->device_removed_slot = NULL;
-    }
 
     if (USE_SCO_OVER_PCM(u))
         restore_sco_volume_callbacks(u);

commit 0bd428df04dfb5d280b9c90bffb40059b75a4428
Author: Mikel Astiz <mikel.astiz at bmw-carit.de>
Date:   Thu Nov 22 15:20:27 2012 +0100

    bluetooth: Run the discovery hook only when necessary
    
    This is a minor optimization too, but the main benefit is that it's
    makes the code easier to understand (I hope), since run_callback()
    won't be called at times when it's not needed.

diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c
index 19e6b47..14368da 100644
--- a/src/modules/bluetooth/bluetooth-util.c
+++ b/src/modules/bluetooth/bluetooth-util.c
@@ -516,6 +516,7 @@ static void get_properties_reply(DBusPendingCall *pending, void *userdata) {
     pa_bluetooth_device *d;
     pa_bluetooth_discovery *y;
     int valid;
+    bool old_any_connected;
 
     pa_assert_se(p = userdata);
     pa_assert_se(y = p->context_data);
@@ -536,6 +537,9 @@ static void get_properties_reply(DBusPendingCall *pending, void *userdata) {
 
     pa_assert(p->call_data == d);
 
+    if (d != NULL)
+        old_any_connected = pa_bluetooth_device_any_audio_connected(d);
+
     valid = dbus_message_get_type(r) == DBUS_MESSAGE_TYPE_ERROR ? -1 : 1;
 
     if (dbus_message_is_method_call(p->message, "org.bluez.Device", "GetProperties"))
@@ -609,7 +613,7 @@ static void get_properties_reply(DBusPendingCall *pending, void *userdata) {
     }
 
 finish:
-    if (d != NULL)
+    if (d != NULL && old_any_connected != pa_bluetooth_device_any_audio_connected(d))
         run_callback(d, FALSE);
 
 finish2:
@@ -845,6 +849,7 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
 
         if ((d = pa_hashmap_get(y->devices, dbus_message_get_path(m)))) {
             DBusMessageIter arg_i;
+            bool old_any_connected = pa_bluetooth_device_any_audio_connected(d);
 
             if (!dbus_message_iter_init(m, &arg_i)) {
                 pa_log("Failed to parse PropertyChanged: %s", err.message);
@@ -876,7 +881,8 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
                     goto fail;
             }
 
-            run_callback(d, FALSE);
+            if (old_any_connected != pa_bluetooth_device_any_audio_connected(d))
+                run_callback(d, FALSE);
         }
 
         return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;

commit ebf5f29bb32921e89ad7acb4c0d0d55ca4cf89a0
Author: Tanu Kaskinen <tanuk at iki.fi>
Date:   Thu Nov 22 15:20:26 2012 +0100

    bluetooth: Add helper pa_bluetooth_device_any_audio_connected()
    
    The new helper function makes it easier to check whether any audio
    profiles are connected. That information is needed by the discovery
    module for deciding whether a new device module should be loaded. The
    device module should use this information too to unload itself at the
    right time, but that's currently not implemented.

diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c
index 565bfce..19e6b47 100644
--- a/src/modules/bluetooth/bluetooth-util.c
+++ b/src/modules/bluetooth/bluetooth-util.c
@@ -1000,6 +1000,31 @@ pa_bluetooth_transport* pa_bluetooth_device_get_transport(pa_bluetooth_device *d
     return NULL;
 }
 
+bool pa_bluetooth_device_any_audio_connected(const pa_bluetooth_device *d) {
+    pa_assert(d);
+
+    if (d->dead || !device_is_audio_ready(d))
+        return false;
+
+    /* Deliberately ignore audio_sink_state and headset_state since they are
+     * reflected in audio_state. This is actually very important in order to
+     * make module-card-restore work well with headsets: if the headset
+     * supports both HSP and A2DP, one of those profiles is connected first and
+     * then the other, and lastly the Audio interface becomes connected.
+     * Checking only audio_state means that this function will return false at
+     * the time when only the first connection has been made. This is good,
+     * because otherwise, if the first connection is for HSP and we would
+     * already load a new device module instance, and module-card-restore tries
+     * to restore the A2DP profile, that would fail because A2DP is not yet
+     * connected. Waiting until the Audio interface gets connected means that
+     * both headset profiles will be connected when the device module is
+     * loaded. */
+    return
+        d->audio_state >= PA_BT_AUDIO_STATE_CONNECTED ||
+        d->audio_source_state >= PA_BT_AUDIO_STATE_CONNECTED ||
+        d->hfgw_state >= PA_BT_AUDIO_STATE_CONNECTED;
+}
+
 int pa_bluetooth_transport_acquire(pa_bluetooth_transport *t, const char *accesstype, size_t *imtu, size_t *omtu) {
     DBusMessage *m, *r;
     DBusError err;
diff --git a/src/modules/bluetooth/bluetooth-util.h b/src/modules/bluetooth/bluetooth-util.h
index 1ec9e8c..874baa0 100644
--- a/src/modules/bluetooth/bluetooth-util.h
+++ b/src/modules/bluetooth/bluetooth-util.h
@@ -144,6 +144,7 @@ pa_bluetooth_device* pa_bluetooth_discovery_get_by_address(pa_bluetooth_discover
 
 pa_bluetooth_transport* pa_bluetooth_discovery_get_transport(pa_bluetooth_discovery *y, const char *path);
 pa_bluetooth_transport* pa_bluetooth_device_get_transport(pa_bluetooth_device *d, enum profile profile);
+bool pa_bluetooth_device_any_audio_connected(const pa_bluetooth_device *d);
 
 int pa_bluetooth_transport_acquire(pa_bluetooth_transport *t, const char *accesstype, size_t *imtu, size_t *omtu);
 void pa_bluetooth_transport_release(pa_bluetooth_transport *t, const char *accesstype);
diff --git a/src/modules/bluetooth/module-bluetooth-discover.c b/src/modules/bluetooth/module-bluetooth-discover.c
index aef9492..48d0bee 100644
--- a/src/modules/bluetooth/module-bluetooth-discover.c
+++ b/src/modules/bluetooth/module-bluetooth-discover.c
@@ -74,10 +74,7 @@ static pa_hook_result_t load_module_for_device(pa_bluetooth_discovery *y, const
 
     mi = pa_hashmap_get(u->hashmap, d->path);
 
-    if (!d->dead &&
-        (d->audio_state >= PA_BT_AUDIO_STATE_CONNECTED ||
-         d->audio_source_state >= PA_BT_AUDIO_STATE_CONNECTED ||
-         d->hfgw_state >= PA_BT_AUDIO_STATE_CONNECTED)) {
+    if (pa_bluetooth_device_any_audio_connected(d)) {
 
         if (!mi) {
             pa_module *m = NULL;

commit 59c8476d64677c852719d27e28251d325081c59b
Author: Mikel Astiz <mikel.astiz at bmw-carit.de>
Date:   Thu Nov 22 15:20:25 2012 +0100

    bluetooth: Rename former device_is_audio()
    
    Use a more accurate name for the function since it doesn't just check
    if it is an audio device (which can be detected quite early), but it
    also checks if the most relevant properties (device info, etc.) have
    been received.
    
    Besides, add the const qualifier to the pointer since it's not going to
    be modified.

diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c
index 84822aa..565bfce 100644
--- a/src/modules/bluetooth/bluetooth-util.c
+++ b/src/modules/bluetooth/bluetooth-util.c
@@ -187,7 +187,7 @@ static void device_free(pa_bluetooth_device *d) {
     pa_xfree(d);
 }
 
-static pa_bool_t device_is_audio(pa_bluetooth_device *d) {
+static pa_bool_t device_is_audio_ready(const pa_bluetooth_device *d) {
     pa_assert(d);
 
     return
@@ -467,7 +467,7 @@ static int parse_audio_property(pa_bluetooth_discovery *u, int *state, DBusMessa
 static void run_callback(pa_bluetooth_device *d, pa_bool_t dead) {
     pa_assert(d);
 
-    if (!device_is_audio(d))
+    if (!device_is_audio_ready(d))
         return;
 
     d->dead = dead;
@@ -949,7 +949,7 @@ pa_bluetooth_device* pa_bluetooth_discovery_get_by_address(pa_bluetooth_discover
 
     while ((d = pa_hashmap_iterate(y->devices, &state, NULL)))
         if (pa_streq(d->address, address))
-            return device_is_audio(d) ? d : NULL;
+            return device_is_audio_ready(d) ? d : NULL;
 
     return NULL;
 }
@@ -965,7 +965,7 @@ pa_bluetooth_device* pa_bluetooth_discovery_get_by_path(pa_bluetooth_discovery *
         pa_bluetooth_discovery_sync(y);
 
     if ((d = pa_hashmap_get(y->devices, path)))
-        if (device_is_audio(d))
+        if (device_is_audio_ready(d))
             return d;
 
     return NULL;

commit 6545cc77f48cb9b5fddfa9b85d75068e9df70df4
Author: Tanu Kaskinen <tanuk at iki.fi>
Date:   Thu Nov 22 15:20:24 2012 +0100

    bluetooth: Ignore Device.Connected
    
    The Device.Connected was only used for tracking whether a device module
    should be loaded, but that information is already included in the
    individual profile state properties. The property can therefore be
    completely ignored without any loss in functionality.

diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c
index 90736e9..84822aa 100644
--- a/src/modules/bluetooth/bluetooth-util.c
+++ b/src/modules/bluetooth/bluetooth-util.c
@@ -128,7 +128,6 @@ static pa_bluetooth_device* device_new(pa_bluetooth_discovery *discovery, const
     d->transports = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
     d->paired = -1;
     d->alias = NULL;
-    d->device_connected = -1;
     PA_LLIST_HEAD_INIT(pa_bluetooth_uuid, d->uuids);
     d->address = NULL;
     d->class = -1;
@@ -348,8 +347,6 @@ static int parse_device_property(pa_bluetooth_device *d, DBusMessageIter *i) {
 
             if (pa_streq(key, "Paired"))
                 d->paired = !!value;
-            else if (pa_streq(key, "Connected"))
-                d->device_connected = !!value;
             else if (pa_streq(key, "Trusted"))
                 d->trusted = !!value;
 
diff --git a/src/modules/bluetooth/bluetooth-util.h b/src/modules/bluetooth/bluetooth-util.h
index c570685..1ec9e8c 100644
--- a/src/modules/bluetooth/bluetooth-util.h
+++ b/src/modules/bluetooth/bluetooth-util.h
@@ -110,7 +110,6 @@ struct pa_bluetooth_device {
     pa_hashmap *transports;
     int paired;
     char *alias;
-    int device_connected;
     PA_LLIST_HEAD(pa_bluetooth_uuid, uuids);
     char *address;
     int class;
diff --git a/src/modules/bluetooth/module-bluetooth-discover.c b/src/modules/bluetooth/module-bluetooth-discover.c
index 2dc7cf5..aef9492 100644
--- a/src/modules/bluetooth/module-bluetooth-discover.c
+++ b/src/modules/bluetooth/module-bluetooth-discover.c
@@ -74,7 +74,7 @@ static pa_hook_result_t load_module_for_device(pa_bluetooth_discovery *y, const
 
     mi = pa_hashmap_get(u->hashmap, d->path);
 
-    if (!d->dead && d->device_connected > 0 &&
+    if (!d->dead &&
         (d->audio_state >= PA_BT_AUDIO_STATE_CONNECTED ||
          d->audio_source_state >= PA_BT_AUDIO_STATE_CONNECTED ||
          d->hfgw_state >= PA_BT_AUDIO_STATE_CONNECTED)) {



More information about the pulseaudio-commits mailing list