[PATCH 5/5] card-restore: Only use hooks for the events.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Tanu.
Thanks for the review.

Here is v2 of the patch, I hope I fixed all the issues and didn't
introduce new ones. :p

I also found a bug where I don't initialize entry->profile (because
card->save->profile is false) and then check if two entries are equal.
Since entry->profile for one of them is NULL this resulted in a crash.

Patch attached.
-------------- next part --------------
>From 468a97fefe38dd39b2ea71232b0e039198e95e87 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?poljar=20=28Damir=20Jeli=C4=87=29?= <poljarinho@xxxxxxxxx>
Date: Tue, 15 Jan 2013 23:51:30 +0100
Subject: [PATCH v2] card-restore: Only use hooks for the events.

Notification events can be error prone, this patch removes the use of
notification events from card-restore and replaces them with hooks.
---
 src/modules/module-card-restore.c | 241 ++++++++++++++++++++++++++++----------
 1 file changed, 176 insertions(+), 65 deletions(-)

diff --git a/src/modules/module-card-restore.c b/src/modules/module-card-restore.c
index 643e074..51923fd 100644
--- a/src/modules/module-card-restore.c
+++ b/src/modules/module-card-restore.c
@@ -62,10 +62,14 @@ static const char* const valid_modargs[] = {
 struct userdata {
     pa_core *core;
     pa_module *module;
-    pa_subscription *subscription;
     pa_hook_slot *card_new_hook_slot;
+    pa_hook_slot *card_put_hook_slot;
+    pa_hook_slot *card_profile_hook_slot;
+    pa_hook_slot *port_new_hook_slot;
+    pa_hook_slot *port_offset_hook_slot;
     pa_time_event *save_time_event;
     pa_database *database;
+    bool hooks_connected;
 };
 
 #define ENTRY_VERSION 2
@@ -110,6 +114,28 @@ static struct entry* entry_new(void) {
     return r;
 }
 
+static struct port_info *port_info_new(pa_device_port *port) {
+    struct port_info *p_info;
+
+    if (port) {
+        p_info = pa_xnew(struct port_info, 1);
+        p_info->name = pa_xstrdup(port->name);
+        p_info->offset = port->latency_offset;
+    } else
+        p_info = pa_xnew0(struct port_info, 1);
+
+    return p_info;
+}
+
+static void port_info_update(struct port_info *p_info, pa_device_port *port) {
+    pa_assert(p_info);
+    pa_assert(port);
+
+    pa_xfree(p_info->name);
+    p_info->name = pa_xstrdup(port->name);
+    p_info->offset = port->latency_offset;
+}
+
 static void port_info_free(struct port_info *p_info, void *userdata) {
     pa_assert(p_info);
 
@@ -126,6 +152,48 @@ static void entry_free(struct entry* e) {
     pa_xfree(e);
 }
 
+static struct entry *entry_from_card(pa_card *card) {
+    struct port_info *p_info;
+    struct entry *entry;
+    pa_device_port *port;
+    void *state;
+
+    pa_assert(card);
+
+    entry = entry_new();
+    if (card->save_profile)
+        entry->profile = pa_xstrdup(card->active_profile->name);
+
+    PA_HASHMAP_FOREACH(port, card->ports, state) {
+        p_info = port_info_new(port);
+        pa_assert_se(pa_hashmap_put(entry->ports, p_info->name, p_info) >= 0);
+    }
+
+    return entry;
+}
+
+static bool entrys_equal(struct entry *a, struct entry *b) {
+    struct port_info *Ap_info, *Bp_info;
+    void *state;
+
+    pa_assert(a);
+    pa_assert(b);
+
+    if (!pa_streq(a->profile, b->profile) ||
+            pa_hashmap_size(a->ports) != pa_hashmap_size(b->ports))
+        return false;
+
+    PA_HASHMAP_FOREACH(Ap_info, a->ports, state) {
+        if ((Bp_info = pa_hashmap_get(b->ports, Ap_info->name))) {
+            if (Ap_info->offset != Bp_info->offset)
+                return false;
+        } else
+            return false;
+    }
+
+    return true;
+}
+
 static pa_bool_t entry_write(struct userdata *u, const char *name, const struct entry *e) {
     pa_tagstruct *t;
     pa_datum key, data;
@@ -245,7 +313,7 @@ static struct entry* entry_read(struct userdata *u, const char *name) {
                 pa_tagstruct_gets64(t, &port_offset) < 0)
                 goto fail;
 
-            p_info = pa_xnew(struct port_info, 1);
+            p_info = port_info_new(NULL);
             p_info->name = pa_xstrdup(port_name);
             p_info->offset = port_offset;
 
@@ -286,82 +354,125 @@ fail:
     return NULL;
 }
 
-static void subscribe_callback(pa_core *c, pa_subscription_event_type_t t, uint32_t idx, void *userdata) {
-    struct userdata *u = userdata;
+static void show_full_info(pa_card *card) {
+    pa_assert(card);
+
+    if (card->save_profile)
+        pa_log_info("Storing profile and port latency offsets for card %s.", card->name);
+    else
+        pa_log_info("Storing port latency offsets for card %s.", card->name);
+}
+
+static pa_hook_result_t card_put_hook_callback(pa_core *c, pa_card *card, struct userdata *u) {
     struct entry *entry, *old;
-    void *state;
-    pa_card *card;
-    pa_device_port *p;
-    struct port_info *p_info;
 
-    pa_assert(c);
-    pa_assert(u);
+    pa_assert(card);
 
-    if (t != (PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_NEW) &&
-        t != (PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_CHANGE))
-        return;
+    entry = entry_from_card(card);
 
-    if (!(card = pa_idxset_get_by_index(c->cards, idx)))
-        return;
+    if ((old = entry_read(u, card->name))) {
+        if (!card->save_profile)
+            entry->profile = pa_xstrdup(old->profile);
+        if (entrys_equal(entry, old))
+            goto finish;
+    }
 
-    entry = entry_new();
+    show_full_info(card);
 
-    if (card->save_profile)
-        entry->profile = pa_xstrdup(card->active_profile->name);
+    if (entry_write(u, card->name, entry))
+        trigger_save(u);
 
-    PA_HASHMAP_FOREACH(p, card->ports, state) {
-        p_info = pa_xnew(struct port_info, 1);
-        p_info->name = pa_xstrdup(p->name);
-        p_info->offset = p->latency_offset;
+finish:
+    entry_free(entry);
+    if (old)
+        entry_free(old);
 
-        pa_assert_se(pa_hashmap_put(entry->ports, p_info->name, p_info) >= 0);
+    return PA_HOOK_OK;
+}
+
+static pa_hook_result_t card_profile_change_callback(pa_core *c, pa_card *card, struct userdata *u) {
+    struct entry *entry;
+
+    pa_assert(card);
+
+    if (!card->save_profile)
+        return PA_HOOK_OK;
+
+    if ((entry = entry_read(u, card->name))) {
+        entry->profile = pa_xstrdup(card->active_profile->name);
+        pa_log_info("Storing card profile for card %s.", card->name);
+    } else {
+        entry = entry_from_card(card);
+        show_full_info(card);
     }
 
-    if ((old = entry_read(u, card->name))) {
-        bool dirty = false;
+    if (entry_write(u, card->name, entry))
+        trigger_save(u);
 
-        if (!card->save_profile)
-            entry->profile = pa_xstrdup(old->profile);
+    entry_free(entry);
+    return PA_HOOK_OK;
+}
+
+static pa_hook_result_t card_port_add_callback(pa_core *c, pa_device_port *port, struct userdata *u) {
+    struct entry *entry;
+    pa_card *card;
 
-        if (!pa_streq(old->profile, entry->profile) ||
-                pa_hashmap_size(old->ports) != pa_hashmap_size(entry->ports))
-            dirty = true;
+    pa_assert(port);
+    card = port->card;
 
+    if ((entry = entry_read(u, card->name))) {
+        struct port_info *p_info;
+
+        if ((p_info = pa_hashmap_get(entry->ports, port->name)))
+            port_info_update(p_info, port);
         else {
-            struct port_info *old_p_info;
-
-            PA_HASHMAP_FOREACH(old_p_info, old->ports, state) {
-                if ((p_info = pa_hashmap_get(entry->ports, old_p_info->name))) {
-                    if (p_info->offset != old_p_info->offset) {
-                        dirty = true;
-                        break;
-                    }
-
-                } else {
-                    dirty = true;
-                    break;
-                }
-            }
+            p_info = port_info_new(port);
+            pa_assert_se(pa_hashmap_put(entry->ports, p_info->name, p_info) >= 0);
         }
 
-        if (!dirty) {
-            entry_free(old);
-            entry_free(entry);
-            return;
-        }
+        pa_log_info("Storing port info for port %s on card %s.", port->name, card->name);
 
-        entry_free(old);
+    } else {
+        entry = entry_from_card(card);
+        show_full_info(card);
     }
 
-    if (card->save_profile)
-        pa_log_info("Storing profile and port latency offsets for card %s.", card->name);
-    else
-        pa_log_info("Storing port latency offsets for card %s.", card->name);
+    if (entry_write(u, card->name, entry))
+        trigger_save(u);
+
+    entry_free(entry);
+    return PA_HOOK_OK;
+}
+
+static pa_hook_result_t port_offset_change_callback(pa_core *c, pa_device_port *port, struct userdata *u) {
+    struct entry *entry;
+    pa_card *card;
+
+    pa_assert(port);
+    card = port->card;
+
+    if ((entry = entry_read(u, card->name))) {
+        struct port_info *p_info;
+
+        if ((p_info = pa_hashmap_get(entry->ports, port->name)))
+            p_info->offset = port->latency_offset;
+        else {
+            p_info = port_info_new(port);
+            pa_assert_se(pa_hashmap_put(entry->ports, p_info->name, p_info) >= 0);
+        }
+
+        pa_log_info("Storing latency offset for port %s on card %s.", port->name, card->name);
+
+    } else {
+        entry_from_card(card);
+        show_full_info(card);
+    }
 
     if (entry_write(u, card->name, entry))
         trigger_save(u);
 
     entry_free(entry);
+    return PA_HOOK_OK;
 }
 
 static pa_hook_result_t card_new_hook_callback(pa_core *c, pa_card_new_data *new_data, struct userdata *u) {
@@ -403,8 +514,6 @@ int pa__init(pa_module*m) {
     pa_modargs *ma = NULL;
     struct userdata *u;
     char *fname;
-    pa_card *card;
-    uint32_t idx;
 
     pa_assert(m);
 
@@ -417,9 +526,12 @@ int pa__init(pa_module*m) {
     u->core = m->core;
     u->module = m;
 
-    u->subscription = pa_subscription_new(m->core, PA_SUBSCRIPTION_MASK_CARD, subscribe_callback, u);
-
     u->card_new_hook_slot = pa_hook_connect(&m->core->hooks[PA_CORE_HOOK_CARD_NEW], PA_HOOK_EARLY, (pa_hook_cb_t) card_new_hook_callback, u);
+    u->card_put_hook_slot = pa_hook_connect(&m->core->hooks[PA_CORE_HOOK_CARD_PUT], PA_HOOK_NORMAL, (pa_hook_cb_t) card_put_hook_callback, u);
+    u->card_profile_hook_slot = pa_hook_connect(&m->core->hooks[PA_CORE_HOOK_CARD_PROFILE_CHANGED], PA_HOOK_NORMAL, (pa_hook_cb_t) card_profile_change_callback, u);
+    u->port_new_hook_slot = pa_hook_connect(&m->core->hooks[PA_CORE_HOOK_PORT_ADDED], PA_HOOK_NORMAL, (pa_hook_cb_t) card_port_add_callback, u);
+    u->port_offset_hook_slot = pa_hook_connect(&m->core->hooks[PA_CORE_HOOK_PORT_LATENCY_OFFSET_CHANGED], PA_HOOK_NORMAL, (pa_hook_cb_t) port_offset_change_callback, u);
+    u->hooks_connected = true;
 
     if (!(fname = pa_state_path("card-database", TRUE)))
         goto fail;
@@ -433,9 +545,6 @@ int pa__init(pa_module*m) {
     pa_log_info("Successfully opened database file '%s'.", fname);
     pa_xfree(fname);
 
-    PA_IDXSET_FOREACH(card, m->core->cards, idx)
-        subscribe_callback(m->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_NEW, card->index, u);
-
     pa_modargs_free(ma);
     return 0;
 
@@ -456,11 +565,13 @@ void pa__done(pa_module*m) {
     if (!(u = m->userdata))
         return;
 
-    if (u->subscription)
-        pa_subscription_free(u->subscription);
-
-    if (u->card_new_hook_slot)
+    if (u->hooks_connected) {
         pa_hook_slot_free(u->card_new_hook_slot);
+        pa_hook_slot_free(u->card_put_hook_slot);
+        pa_hook_slot_free(u->card_profile_hook_slot);
+        pa_hook_slot_free(u->port_new_hook_slot);
+        pa_hook_slot_free(u->port_offset_hook_slot);
+    }
 
     if (u->save_time_event)
         u->core->mainloop->time_free(u->save_time_event);
-- 
1.8.1.1



[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux