summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwm4 <wm4@nowhere>2019-11-28 21:19:41 +0100
committerwm4 <wm4@nowhere>2019-11-29 12:14:43 +0100
commit591494b271f17c4bfae0cfb860ae12bdad98a2ca (patch)
tree90278cadcea4fb2e79a7d05ce159784f36be5361
parent1e6c57d4e437b403eab89d07ab61b872253b2c35 (diff)
downloadmpv-591494b271f17c4bfae0cfb860ae12bdad98a2ca.tar.bz2
mpv-591494b271f17c4bfae0cfb860ae12bdad98a2ca.tar.xz
m_config: add fine-grained option reporting mechanism
This adds m_config_cache_get_next_changed() and the change_flags field in m_config_cache. Both can be used to determine whether specific options changed (rather than the entire sub-group). Not sure if I'm very happy with that. The former rather compact update_options() is now a bit of a mess, because it needs to be incremental. m_config_cache_get_next_changed() will not be too nice to use, and change_flags still relies on global "allocation" of change flags (see UPDATE_* defines in m_option.h). If C weren't such a primitive language that smells like grandpa, it would be nice to define per-option change callbacks or so. This compares options by value to determine whether they have changed. This makes it slower in theory, but in practice it probably doesn't matter (options are rarely changed after initialization). The alternative would have been per-option change counters (wastes too much memory; not a practical problem but too ugly), or keep all m_config_caches in a global list and have bitmaps with per-option change bits (sounds complicated). I guess the current way is OK. Technically, this changes semantics slightly by ignoring setting an option to the same value. Technically this wasn't a no-op, although the effect was almost almost no-op. Some code would actually become cleaner by ignoring such redundant change events, and them being no-op is probably also what the user would normally assume.
-rw-r--r--options/m_config.c140
-rw-r--r--options/m_config.h29
2 files changed, 135 insertions, 34 deletions
diff --git a/options/m_config.c b/options/m_config.c
index e3f6a8e7b7..c6e0d02fc8 100644
--- a/options/m_config.c
+++ b/options/m_config.c
@@ -102,6 +102,9 @@ struct config_cache {
struct m_config_shadow *shadow; // global metadata
uint64_t ts; // timestamp of this data copy
bool in_list; // part of m_config_shadow->listeners[]
+ int upd_group; // for "incremental" change notification
+ int upd_opt;
+
// --- Implicitly synchronized by setting/unsetting wakeup_cb.
struct mp_dispatch_queue *wakeup_dispatch_queue;
@@ -611,6 +614,19 @@ static void add_sub_group(struct m_config *config, const char *name_prefix,
shadow->groups[group_index].group_count = shadow->num_groups - group_index;
}
+static uint64_t get_option_change_mask(struct m_config_shadow *shadow,
+ int group_index, int group_root,
+ const struct m_option *opt)
+{
+ uint64_t changed = opt->flags & UPDATE_OPTS_MASK;
+ while (group_index != group_root) {
+ struct m_config_group *g = &shadow->groups[group_index];
+ changed |= g->group->change_flags;
+ group_index = g->parent_group;
+ }
+ return changed;
+}
+
struct m_config_option *m_config_get_co_raw(const struct m_config *config,
struct bstr name)
{
@@ -1285,46 +1301,74 @@ struct m_config_cache *m_config_cache_alloc(void *ta_parent,
cache->opts = in->data->gdata[0].udata;
+ in->upd_group = -1;
+
return cache;
}
-static bool update_options(struct m_config_data *dst, struct m_config_data *src)
+static void update_next_option(struct m_config_cache *cache, void **p_opt)
{
- assert(dst->shadow == src->shadow);
+ struct config_cache *in = cache->internal;
+ struct m_config_data *dst = in->data;
+ struct m_config_data *src = in->src;
- bool res = false;
+ assert(src->group_index == 0); // must be the option root currently
- // Must be from same root, but they can have arbitrary overlap.
- int group_s = MPMAX(dst->group_index, src->group_index);
- int group_e = MPMIN(dst->group_index + dst->num_gdata,
- src->group_index + src->num_gdata);
- assert(group_s >= 0 && group_e <= dst->shadow->num_groups);
- for (int n = group_s; n < group_e; n++) {
- struct m_config_group *g = &dst->shadow->groups[n];
- const struct m_option *opts = g->group->opts;
- struct m_group_data *gsrc = m_config_gdata(src, n);
- struct m_group_data *gdst = m_config_gdata(dst, n);
- assert(gsrc && gdst);
+ *p_opt = NULL;
- if (gdst->ts >= gsrc->ts)
- continue;
- gdst->ts = gsrc->ts;
- res = true;
-
- for (int i = 0; opts && opts[i].name; i++) {
- const struct m_option *opt = &opts[i];
+ while (in->upd_group < dst->group_index + dst->num_gdata) {
+ struct m_group_data *gsrc = m_config_gdata(src, in->upd_group);
+ struct m_group_data *gdst = m_config_gdata(dst, in->upd_group);
+ assert(gsrc && gdst);
- if (opt->offset >= 0 && opt->type->size) {
- m_option_copy(opt, gdst->udata + opt->offset,
- gsrc->udata + opt->offset);
+ if (gdst->ts < gsrc->ts) {
+ struct m_config_group *g = &dst->shadow->groups[in->upd_group];
+ const struct m_option *opts = g->group->opts;
+
+ while (opts && opts[in->upd_opt].name) {
+ const struct m_option *opt = &opts[in->upd_opt];
+
+ if (opt->offset >= 0 && opt->type->size) {
+ void *dsrc = gsrc->udata + opt->offset;
+ void *ddst = gdst->udata + opt->offset;
+
+ if (!m_option_equal(opt, ddst, dsrc)) {
+ uint64_t ch = get_option_change_mask(dst->shadow,
+ in->upd_group, dst->group_index, opt);
+
+ if (cache->debug) {
+ char *vdst = m_option_print(opt, ddst);
+ char *vsrc = m_option_print(opt, dsrc);
+ mp_warn(cache->debug, "Option '%s' changed from "
+ "'%s' to' %s' (flags = 0x%"PRIx64")\n",
+ opt->name, vdst, vsrc, ch);
+ talloc_free(vdst);
+ talloc_free(vsrc);
+ }
+
+ m_option_copy(opt, ddst, dsrc);
+ cache->change_flags |= ch;
+
+ in->upd_opt++; // skip this next time
+ *p_opt = ddst;
+ return;
+ }
+ }
+
+ in->upd_opt++;
}
+
+ gdst->ts = gsrc->ts;
}
+
+ in->upd_group++;
+ in->upd_opt = 0;
}
- return res;
+ in->upd_group = -1;
}
-bool m_config_cache_update(struct m_config_cache *cache)
+static bool cache_check_update(struct m_config_cache *cache)
{
struct config_cache *in = cache->internal;
struct m_config_shadow *shadow = in->shadow;
@@ -1336,13 +1380,47 @@ bool m_config_cache_update(struct m_config_cache *cache)
return false;
in->ts = new_ts;
+ in->upd_group = in->data->group_index;
+ in->upd_opt = 0;
+ return true;
+}
+
+bool m_config_cache_update(struct m_config_cache *cache)
+{
+ struct config_cache *in = cache->internal;
+ struct m_config_shadow *shadow = in->shadow;
+
+ if (!cache_check_update(cache))
+ return false;
pthread_mutex_lock(&shadow->lock);
- bool res = update_options(in->data, in->src);
+ bool res = false;
+ while (1) {
+ void *p;
+ update_next_option(cache, &p);
+ if (!p)
+ break;
+ res = true;
+ }
pthread_mutex_unlock(&shadow->lock);
return res;
}
+bool m_config_cache_get_next_changed(struct m_config_cache *cache, void **opt)
+{
+ struct config_cache *in = cache->internal;
+ struct m_config_shadow *shadow = in->shadow;
+
+ *opt = NULL;
+ if (!cache_check_update(cache) && in->upd_group < 0)
+ return false;
+
+ pthread_mutex_lock(&shadow->lock);
+ update_next_option(cache, opt);
+ pthread_mutex_unlock(&shadow->lock);
+ return !!*opt;
+}
+
void m_config_notify_change_co(struct m_config *config,
struct m_config_option *co)
{
@@ -1369,13 +1447,7 @@ void m_config_notify_change_co(struct m_config *config,
pthread_mutex_unlock(&shadow->lock);
}
- int changed = co->opt->flags & UPDATE_OPTS_MASK;
- int group_index = co->group_index;
- while (group_index >= 0) {
- struct m_config_group *g = &shadow->groups[group_index];
- changed |= g->group->change_flags;
- group_index = g->parent_group;
- }
+ int changed = get_option_change_mask(shadow, co->group_index, 0, co->opt);
if (config->option_change_callback) {
config->option_change_callback(config->option_change_callback_ctx, co,
diff --git a/options/m_config.h b/options/m_config.h
index 42734e8000..e2dcc0e51c 100644
--- a/options/m_config.h
+++ b/options/m_config.h
@@ -266,6 +266,14 @@ struct m_config_cache {
// The struct as indicated by m_config_cache_alloc's group parameter.
// (Internally the same as internal->gdata[0]->udata.)
void *opts;
+ // Accumulated change flags. The user can set this to 0 to unset all flags.
+ // They are set when calling any of the update functions. A flag is only set
+ // once the new value is visible in ->opts.
+ uint64_t change_flags;
+
+ // Set to non-NULL for logging all option changes as they are retrieved
+ // with one of the update functions (like m_config_cache_update()).
+ struct mp_log *debug;
// Do not access.
struct config_cache *internal;
@@ -277,6 +285,9 @@ struct m_config_cache {
// Keep in mind that a m_config_cache object is not thread-safe; it merely
// provides thread-safe access to the global options. All API functions for
// the same m_config_cache object must synchronized, unless otherwise noted.
+// This does not create an initial change event (m_config_cache_update() will
+// return false), but note that a change might be asynchronously signaled at any
+// time.
// ta_parent: parent for the returned allocation
// global: option data source
// group: the option group to return. This can be GLOBAL_CONFIG for the global
@@ -307,8 +318,26 @@ void m_config_cache_set_dispatch_change_cb(struct m_config_cache *cache,
// some options have changed).
// Keep in mind that while the cache->opts pointer does not change, the option
// data itself will (e.g. string options might be reallocated).
+// New change flags are or-ed into cache->change_flags with this call (if you
+// use them, you should probably do cache->change_flags=0 before this call).
bool m_config_cache_update(struct m_config_cache *cache);
+// Check for changes and return fine grained change information.
+// Warning: this conflicts with m_config_cache_update(). If you call
+// m_config_cache_update(), all options will be marked as "not changed",
+// and this function will return false. Also, calling this function and
+// then m_config_cache_update() is not supported, and may skip updating
+// some fields.
+// This returns true as long as there is a changed option, and false if all
+// changed options have been returned.
+// If multiple options have changed, the new option value is visible only once
+// this function has returned the change for it.
+// out_ptr: pointer to a void*, which is set to the cache->opts field associated
+// with the changed option if the function returns true; set to NULL
+// if no option changed.
+// returns: *out_ptr!=NULL (true if there was a changed option)
+bool m_config_cache_get_next_changed(struct m_config_cache *cache, void **out_ptr);
+
// Like m_config_cache_alloc(), but return the struct (m_config_cache->opts)
// directly, with no way to update the config. Basically this returns a copy
// with a snapshot of the current option values.