commit 297aa7d188286c22bf8bb1f1fa43db6ea0787075 from: Tobias Heider date: Mon Feb 16 01:24:57 2026 UTC Fix refcounting mess Create a gobject for every device we get from sndioctl, coalescing in one step is too fragile. The problem is that we sometimes get the new controls first before the old ones are removed causing name conflicts. The old approach would have deleted the entire node at the end if that happened. # changes to be committed on branch main: commit - ba173f75b4425a2a7655fde75edf1c2dcf716a1d commit + 297aa7d188286c22bf8bb1f1fa43db6ea0787075 blob - 562f126da682607f64c39783beb9bb8087db7e62 blob + e3182a332db83397e5c43101d95680106c42fb0b --- ctlitem.c +++ ctlitem.c @@ -25,7 +25,6 @@ extern struct app_state s; typedef enum { PROP_LEVEL = 1, - PROP_MUTE, N_PROPS } SiomixerCtlItemProperty; @@ -33,13 +32,10 @@ static GParamSpec *properties[N_PROPS] = { NULL, }; struct _SiomixerCtlItem { GObject parent_instance; - char *group; - char *name; - unsigned ctladdr; + struct sioctl_desc desc; + GBinding *binding; unsigned timeout; - int maxlevel; int level; - int mute; }; G_DEFINE_TYPE (SiomixerCtlItem, siomixer_ctl_item, G_TYPE_OBJECT) @@ -50,11 +46,11 @@ static void siomixer_ctl_item_set_property(GObject *, const GValue *, GParamSpec *); static void siomixer_ctl_item_get_property(GObject *, guint, GValue *, GParamSpec *); +static void siomixer_ctl_item_dispose(GObject *); static char *_format_percent(GtkScale *, double, gpointer); static void siomixer_ctl_item_init(SiomixerCtlItem *); static void siomixer_ctl_item_class_init(SiomixerCtlItemClass *); -static void siomixer_ctl_item_finalize(GObject *); static gboolean _on_debounce_timeout(gpointer); static void _on_scale_value_changed(GObject *, gpointer); @@ -74,13 +70,10 @@ _format_percent(GtkScale *scale, double value, gpointe static void siomixer_ctl_item_init(SiomixerCtlItem *self) { - self->name = NULL; - self->group = NULL; - self->ctladdr = 0; + bzero(&self->desc, sizeof(self->desc)); self->timeout = 0; - self->maxlevel = -1; - self->level = -1; - self->mute = -1; + self->level = 0; + self->binding = NULL; } static void @@ -90,12 +83,10 @@ siomixer_ctl_item_class_init(SiomixerCtlItemClass *kla object_class->set_property = siomixer_ctl_item_set_property; object_class->get_property = siomixer_ctl_item_get_property; - object_class->finalize = siomixer_ctl_item_finalize; + object_class->dispose = siomixer_ctl_item_dispose; properties[PROP_LEVEL] = g_param_spec_int("level", "Level", "Volume level", 0, 255, 0, G_PARAM_READWRITE); - properties[PROP_MUTE] = - g_param_spec_int("mute", "Mute", "Volume mute", -1, 1, 0, G_PARAM_READWRITE); g_object_class_install_properties(object_class, N_PROPS, properties); @@ -110,9 +101,6 @@ siomixer_ctl_item_set_property(GObject *o, guint prope case PROP_LEVEL: self->level = g_value_get_int (value); break; - case PROP_MUTE: - self->mute = g_value_get_int (value); - break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (o, property_id, pspec); break; @@ -128,9 +116,6 @@ siomixer_ctl_item_get_property(GObject *o, guint prope case PROP_LEVEL: g_value_set_int(value, self->level); break; - case PROP_MUTE: - g_value_set_int(value, self->mute); - break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (o, property_id, pspec); break; @@ -138,20 +123,33 @@ siomixer_ctl_item_get_property(GObject *o, guint prope } static void -siomixer_ctl_item_finalize(GObject *o) +siomixer_ctl_item_dispose(GObject *o) { SiomixerCtlItem *self = SIOMIXER_CTL_ITEM(o); - free(self->name); - free(self->group); - G_OBJECT_CLASS(siomixer_ctl_item_parent_class)->finalize(o); + printf("dispose %p. self->binding: %s\n", o, self->binding ? "valid" : "NULL"); + if (self->binding) { + printf("Binding Refcount: %d\n", G_OBJECT(self->binding)->ref_count); + g_binding_unbind(self->binding); + g_object_unref(self->binding); + self->binding = NULL; + } + G_OBJECT_CLASS(siomixer_ctl_item_parent_class)->dispose(o); } +SiomixerCtlItem * +siomixer_ctl_item_new(struct sioctl_desc *d) +{ + SiomixerCtlItem *item = g_object_new(SIOMIXER_TYPE_CTL_ITEM, NULL); + memcpy(&item->desc, d, sizeof(item->desc)); + return item; +} + static gboolean _on_debounce_timeout(gpointer user_data) { SiomixerCtlItem *item = SIOMIXER_CTL_ITEM(user_data); - sioctl_setval(s.hdl, item->ctladdr, item->level); + sioctl_setval(s.hdl, item->desc.addr, item->level); item->timeout = 0; return G_SOURCE_REMOVE; } @@ -161,10 +159,13 @@ _on_scale_value_changed(GObject *o, gpointer user_data { SiomixerCtlItem *item = SIOMIXER_CTL_ITEM(o); +#if 0 /* Rate limit */ if (item->timeout != 0) return; item->timeout = g_timeout_add(150, _on_debounce_timeout, o); +#endif + sioctl_setval(s.hdl, item->desc.addr, item->level); } GtkWidget * @@ -173,21 +174,24 @@ siomixer_ctl_item_to_widget(gpointer o, gpointer user_ SiomixerCtlItem *item = SIOMIXER_CTL_ITEM(o); char *label = NULL; char *group = NULL; + char *unit = NULL; - /* We only care about scales */ - if (item->level == -1) - return NULL; + /* Scales only make sense for numeric values */ + g_assert(item->desc.type == SIOCTL_NUM); GtkWidget *box = gtk_box_new(GTK_ORIENTATION_VERTICAL, 5); - if (strlen(item->group)) - asprintf(&group, "%s/", item->group); - asprintf(&label, "%s%s", group ? group : "", item->name); + if (strlen(item->desc.group)) + asprintf(&group, "%s/", item->desc.group); + if (item->desc.node0.unit >= 0) + asprintf(&unit, "[%d]", item->desc.node0.unit); + asprintf(&label, "%s%s.%s%s", group ? group : "", item->desc.node0.name, + item->desc.func, unit ? unit : ""); GtkWidget *l = gtk_label_new(label); gtk_widget_set_vexpand(l, FALSE); gtk_box_append(GTK_BOX(box), l); - GtkWidget *gscale = gtk_scale_new_with_range(GTK_ORIENTATION_VERTICAL, 0, item->maxlevel, 1); + GtkWidget *gscale = gtk_scale_new_with_range(GTK_ORIENTATION_VERTICAL, 0, item->desc.maxval, 1); gtk_range_set_inverted(GTK_RANGE(gscale), TRUE); gtk_widget_set_vexpand(gscale, TRUE); gtk_scale_set_draw_value(GTK_SCALE(gscale), TRUE); @@ -195,38 +199,35 @@ siomixer_ctl_item_to_widget(gpointer o, gpointer user_ g_signal_connect(item, "notify::level", G_CALLBACK(_on_scale_value_changed), item); GtkAdjustment *a = gtk_range_get_adjustment(GTK_RANGE(gscale)); - g_object_bind_property(item, "level", a, "value", + item->binding = g_object_bind_property(item, "level", a, "value", G_BINDING_SYNC_CREATE | G_BINDING_BIDIRECTIONAL); - + gtk_box_append(GTK_BOX(box), gscale); free(label); free(group); + free(unit); return box; } -SiomixerCtlItem * -siomixer_ctl_item_new(char *group, char *name, unsigned addr) -{ - SiomixerCtlItem *item = g_object_new(SIOMIXER_TYPE_CTL_ITEM, NULL); - item->group = strdup(group); - item->name = strdup(name); - item->ctladdr = addr; - return item; -} - int siomixer_ctl_item_cmpdesc(struct sioctl_desc *d, SiomixerCtlItem *self) { int res; - - res = strcmp(d->group, self->group); + res = strcmp(d->group, self->desc.group); if (res != 0) return res; - res = strcmp(d->node0.name, self->name); + res = strcmp(d->node0.name, self->desc.node0.name); if (res != 0) return res; + res = strcmp(d->func, self->desc.func); + if (res != 0) + return res; + res = d->node0.unit - self->desc.node0.unit; + if (res != 0) + return res; + res = d->addr - self->desc.addr; return res; } @@ -238,34 +239,14 @@ siomixer_ctl_item_set_level(SiomixerCtlItem *self, int properties[PROP_LEVEL]); } -void -siomixer_ctl_item_set_maxlevel(SiomixerCtlItem *self, int level) -{ - self->maxlevel = level; -} - -void -siomixer_ctl_item_set_mute(SiomixerCtlItem *self, int mute) -{ - self->mute = mute; - g_object_notify_by_pspec(G_OBJECT(self), - properties[PROP_MUTE]); -} - int siomixer_ctl_item_get_level(SiomixerCtlItem *self) { return self->level; } -int -siomixer_ctl_item_get_maxlevel(SiomixerCtlItem *self) +unsigned +siomixer_ctl_item_get_addr(SiomixerCtlItem *self) { - return self->maxlevel; + return self->desc.addr; } - -int -siomixer_ctl_item_get_mute(SiomixerCtlItem *self) -{ - return self->mute; -} blob - 4650de767eed318e09849ee2ce78c2b4778ffd41 blob + d58370f9ea152367409c8d967026c3332a20a958 --- ctlitem.h +++ ctlitem.h @@ -24,15 +24,14 @@ G_DECLARE_FINAL_TYPE(SiomixerCtlItem, siomixer_ctl_ite GtkWidget *siomixer_ctl_item_to_widget(gpointer, gpointer); -SiomixerCtlItem * siomixer_ctl_item_new(char *, char *, unsigned); +SiomixerCtlItem * siomixer_ctl_item_new(struct sioctl_desc *); int siomixer_ctl_item_cmpdesc(struct sioctl_desc *, SiomixerCtlItem *); void siomixer_ctl_item_set_level(SiomixerCtlItem *, int); -void siomixer_ctl_item_set_maxlevel(SiomixerCtlItem *, int); void siomixer_ctl_item_set_mute(SiomixerCtlItem *, int); int siomixer_ctl_item_get_level(SiomixerCtlItem *); -int siomixer_ctl_item_get_maxlevel(SiomixerCtlItem *); int siomixer_ctl_item_get_mute(SiomixerCtlItem *); +unsigned siomixer_ctl_item_get_addr(SiomixerCtlItem *); #endif /* CTLITEM_H */ blob - 81716996c756b1f80953d68971779eedfdcc952c blob + 5b2c3476bf34ad945e8306531f0fd572b8b65631 --- siomixer.c +++ siomixer.c @@ -65,27 +65,25 @@ ondesc(void *arg, struct sioctl_desc *d, int curval) if (d == NULL) return; - /* We don't handle units for now */ - if (d->node0.unit > 0) - return; + n_items = g_list_model_get_n_items(G_LIST_MODEL(s.store)); + for (i = n_items - 1; i >= 0; i--) { + item = g_list_model_get_item(G_LIST_MODEL(s.store), i); + if (d->addr == siomixer_ctl_item_get_addr(item)) + g_list_store_remove(G_LIST_STORE(s.store), i); + } switch (d->type) { - case SIOCTL_SW: case SIOCTL_NUM: case SIOCTL_NONE: - printf("%d: %s %s %s %d\n", d->type, d->group, d->node0.name, d->func, d->node0.unit); /* * find the right position to insert or merge ctl */ n_items = g_list_model_get_n_items(G_LIST_MODEL(s.store)); - printf("nitems %d\n", n_items); for (i = 0; i < n_items; i++) { item = g_list_model_get_item(G_LIST_MODEL(s.store), i); cmp = siomixer_ctl_item_cmpdesc(d, item); - printf("cmp=%d\n", cmp); if (cmp == 0) { /* Update */ - printf("found at %d\n", i); break; } else if (cmp < 0) { item = NULL; @@ -93,47 +91,31 @@ ondesc(void *arg, struct sioctl_desc *d, int curval) } item = NULL; } + break; + default: + return; + } + + switch (d->type) { + case SIOCTL_NUM: if (item == NULL) { - printf("alloc new...\n"); - item = siomixer_ctl_item_new(d->group, d->node0.name, d->addr); + if (d->type == SIOCTL_NONE) { + g_error("should never happen\n"); + } + item = siomixer_ctl_item_new(d); g_list_store_insert(G_LIST_STORE(s.store), i, item); - } - break; - default: - printf("default: %s/%s.%s %d\n", d->group, d->node0.name, d->func, d->type); - return; - } - switch (d->type) { - case SIOCTL_SW: - case SIOCTL_NUM: - if (strcmp(d->func, "level") == 0) { - siomixer_ctl_item_set_level(item, curval); - siomixer_ctl_item_set_maxlevel(item, d->maxval); - } if (strcmp(d->func, "mute") == 0) { - siomixer_ctl_item_set_mute(item, curval); + /* We want the object to get freed on list removal */ + g_object_unref (item); } + siomixer_ctl_item_set_level(item, curval); break; case SIOCTL_NONE: - if (strcmp(d->func, "level") == 0) { - siomixer_ctl_item_set_level(item, -1); - } else if (strcmp(d->func, "mute") == 0) { - siomixer_ctl_item_set_mute(item, -1); - } - /* All controls unused -> free */ - if ((siomixer_ctl_item_get_level(item) == -1) && - (siomixer_ctl_item_get_mute(item) == -1)) { - printf(">>> delete at %d\n", i); + if (item != NULL) { g_list_store_remove(G_LIST_STORE(s.store), i); - printf(">>> after delete at %d\n", i); + n_items = g_list_model_get_n_items(G_LIST_MODEL(s.store)); } break; - case SIOCTL_VEC: - case SIOCTL_LIST: - case SIOCTL_SEL: - default: - printf("unhandled: %s %s %s %d\n", d->group, d->node0.name, d->func, d->node0.unit); - break; } } @@ -215,7 +197,6 @@ nextent(struct info *i, int mono) static void activate (GtkApplication *app, gpointer user_data) { - printf(">>> %s: __entry__ \n", __func__); GtkWidget *window; GtkWidget *scroll; GtkWidget *flowbox; @@ -327,14 +308,8 @@ main (int argc, char **argv) exit(1); } for (int i = 0; i < ss->nfds; i++) { - printf("%d:\n", i); - printf("fd: %d\n", ss->pfds[i].fd); - if (fcntl(ss->pfds[i].fd, F_GETFL) == -1) { - printf("Invalid FD"); - exit(1); - } - printf("events: %d\n", ss->pfds[i].events); - printf("revents: %d\n", ss->pfds[i].revents); + if (fcntl(ss->pfds[i].fd, F_GETFL) == -1) + g_error("Invalid FD"); g_source_add_poll(gs, (GPollFD *)&ss->pfds[i]); } g_source_attach(gs, NULL);