[pulseaudio-discuss] [PATCH] card-restore: Add the ability to save and restore the latency offset.
Tanu Kaskinen
tanuk at iki.fi
Tue Jul 3 02:50:38 PDT 2012
On Mon, 2012-07-02 at 14:18 +0200, poljar (Damir Jelić) wrote:
> From: "poljar (Damir Jelić)" <poljarinho at gmail.com>
>
> module-card-restore now saves the latency offsets.
>
> This change includes a entry version bump.
>
> The entry now consists of a port count and a port name and offset for
> every port that belongs to the relevant card.
> ---
> src/modules/module-card-restore.c | 122 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 109 insertions(+), 13 deletions(-)
>
> diff --git a/src/modules/module-card-restore.c b/src/modules/module-card-restore.c
> index 1079a72..b21aaae 100644
> --- a/src/modules/module-card-restore.c
> +++ b/src/modules/module-card-restore.c
> @@ -68,11 +68,18 @@ struct userdata {
> pa_database *database;
> };
>
> -#define ENTRY_VERSION 1
> +#define ENTRY_VERSION 2
> +
> +struct port_info {
> + char *name;
> + int64_t offset;
> +};
>
> struct entry {
> uint8_t version;
> char *profile;
> + uint32_t port_count;
> + struct port_info *port_info;
> };
>
> static void save_time_callback(pa_mainloop_api*a, pa_time_event* e, const struct timeval *t, void *userdata) {
> @@ -107,6 +114,16 @@ static void entry_free(struct entry* e) {
> pa_assert(e);
>
> pa_xfree(e->profile);
> +
> + if (e->port_count > 0) {
> + unsigned i;
> + for (i = 0; i < e->port_count; i++)
> + pa_xfree(e->port_info[i].name);
> + }
> +
> + if (e->port_info)
> + pa_xfree(e->port_info);
> +
> pa_xfree(e);
> }
>
> @@ -114,6 +131,7 @@ static pa_bool_t entry_write(struct userdata *u, const char *name, const struct
> pa_tagstruct *t;
> pa_datum key, data;
> pa_bool_t r;
> + unsigned i;
>
> pa_assert(u);
> pa_assert(name);
> @@ -122,6 +140,12 @@ static pa_bool_t entry_write(struct userdata *u, const char *name, const struct
> t = pa_tagstruct_new(NULL, 0);
> pa_tagstruct_putu8(t, e->version);
> pa_tagstruct_puts(t, e->profile);
> + pa_tagstruct_putu32(t, e->port_count);
> +
> + for (i = 0; i < e->port_count; i++) {
> + pa_tagstruct_puts(t, e->port_info[i].name);
> + pa_tagstruct_puts64(t, e->port_info[i].offset);
> + }
>
> key.data = (char *) name;
> key.size = strlen(name);
> @@ -201,6 +225,30 @@ static struct entry* entry_read(struct userdata *u, const char *name) {
>
> e->profile = pa_xstrdup(profile);
>
> + if (e->version >= 2) {
> + const char *port_name = NULL;
> + int64_t port_offset = 0;
> + unsigned i;
> +
> + if (pa_tagstruct_getu32(t, &e->port_count) < 0)
> + goto fail;
> +
> + if (e->port_count > 0) {
> + e->port_info = pa_xnew(struct port_info, e->port_count);
> +
> + for (i = 0; i < e->port_count; i++) {
> + if (pa_tagstruct_gets(t, &port_name) < 0 ||
> + pa_tagstruct_gets64(t, &port_offset) < 0) {
> + e->port_count = i;
> + goto fail;
> + }
> +
> + e->port_info[i].name = pa_xstrdup(port_name);
> + e->port_info[i].offset = port_offset;
> + }
> + }
> + }
> +
> if (!pa_tagstruct_eof(t))
> goto fail;
>
> @@ -238,6 +286,7 @@ static void subscribe_callback(pa_core *c, pa_subscription_event_type_t t, uint3
> struct userdata *u = userdata;
> struct entry *entry, *old;
> pa_card *card;
> + unsigned i = 0;
>
> pa_assert(c);
> pa_assert(u);
> @@ -249,15 +298,44 @@ static void subscribe_callback(pa_core *c, pa_subscription_event_type_t t, uint3
> if (!(card = pa_idxset_get_by_index(c->cards, idx)))
> return;
>
> + entry = entry_new();
> +
> if (!card->save_profile)
> - return;
> + entry->profile = pa_xstrdup("");
If save_profile is false, the profile in the database should not be
changed to "". Instead, it should be left to the old value.
> + else
> + entry->profile = pa_xstrdup(card->active_profile->name);
>
> - entry = entry_new();
> - entry->profile = pa_xstrdup(card->active_profile->name);
> + entry->port_count = pa_hashmap_size(card->ports);
> +
> + if (entry->port_count > 0) {
> + void *state;
> + pa_device_port *p;
> +
> + entry->port_info = pa_xnew(struct port_info, entry->port_count);
> +
> + PA_HASHMAP_FOREACH(p, card->ports, state) {
> + entry->port_info[i].name = pa_xstrdup(p->name);
> + entry->port_info[i].offset = p->latency_offset;
> + i++;
> + }
> + }
>
> if ((old = entry_read(u, card->name))) {
> + bool dirty = false;
>
> - if (pa_streq(old->profile, entry->profile)) {
> + if (!pa_streq(old->profile, entry->profile) &&
> + card->save_profile)
> + dirty = true;
> +
> + else
> + for (i = 0; i < old->port_count; i++)
> + if (!pa_streq(old->port_info[i].name, entry->port_info[i].name) ||
> + old->port_info[i].offset != entry->port_info[i].offset) {
> + dirty = true;
> + break;
> + }
This crashes (or does something else strange) if old->port_count is
bigger than entry->port_count. Similarly, if old->port_count is less
smaller than entry->port_count, dirty may be left to false when it
shouldn't.
Also, if the port counts match, this may trigger redundant writes if the
port order is different. This is probably not a big deal, since avoiding
redundant writes is only an optimization, but if you want to avoid this,
I recommend using a hashmap instead of an array for storing the port
info structs. (I guess it's actually very unlikely that the ports just
change order without changing their names too, because the hash function
doesn't tend to change over time, and thus the port order in the
pa_card.ports hashmap should stay the same.)
> +
> + if (!dirty) {
> entry_free(old);
> entry_free(entry);
> return;
> @@ -266,7 +344,10 @@ static void subscribe_callback(pa_core *c, pa_subscription_event_type_t t, uint3
> entry_free(old);
> }
>
> - pa_log_info("Storing profile for card %s.", card->name);
> + if (!card->save_profile)
> + pa_log_info("Storing profile and port latency offsets for card %s.", card->name);
> + else
> + pa_log_info("Port latency offsets for card %s.", card->name);
Given the if condition, the messages are the wrong way around (you
should drop the ! operator from the condition).
The latter message is missing "Storing " from the beginning.
>
> if (entry_write(u, card->name, entry))
> trigger_save(u);
> @@ -279,14 +360,29 @@ static pa_hook_result_t card_new_hook_callback(pa_core *c, pa_card_new_data *new
>
> pa_assert(new_data);
>
> - if ((e = entry_read(u, new_data->name)) && e->profile[0]) {
> + if ((e = entry_read(u, new_data->name))) {
Minor refactoring suggestion: make this
if (!(e = entry_read(u, new_data->name)))
return PA_HOOK_OK;
That saves one indentation level from the rest of the function.
--
Tanu
More information about the pulseaudio-discuss
mailing list