On 02/20/2013 07:24 PM, Tanu Kaskinen wrote:
> I was writing function pa_device_port_update_proplist(), and I wanted
> it to send change notifications only if the proplist actually changes.
> pa_proplist_update() doesn't provide any indication about whether the
> proplist changed or not, so some kind of a solution was needed.
>
> The simple solution would be to create a copy of the port proplist
> before calling pa_proplist_update() and check if the copy equals the
> port proplist after calling pa_proplist_update(). That felt overly
> wasteful, however: it would mean copying the whole property list and
> comparing every property in it whenever someone changes even just one
> property.
>
> So, I invented a more complex solution: a pa_proplist_update_info
> object that holds a description of per-property operations to be
> applied to a property list. pa_proplist_apply_update_info() iterates
> through the operations and applies them one by one, keeping track of
> whether the operations cause actual changes.
I guess that's one way to solve it. I would probably have gone for the
slightly simpler solution of just keeping a flag inside the proplist.
The proplist object itself will set the flag whenever a "real" change
occurs, and it can be manually reset by just calling, say
pa_proplist_reset_change_flag() or so.
> ---
> src/map-file | 4 +
I don't think we need to add this to the external API unless somebody
complains about missing that feature.
> src/pulse/proplist.c | 220 ++++++++++++++++++++++++++++++++++++++++++++++----
> src/pulse/proplist.h | 70 ++++++++++++++++
> 3 files changed, 279 insertions(+), 15 deletions(-)
>
> diff --git a/src/map-file b/src/map-file
> index 91d61c2..3bd5ddb 100644
> --- a/src/map-file
> +++ b/src/map-file
> @@ -224,6 +224,7 @@ pa_operation_set_state_callback;
> pa_operation_unref;
> pa_parse_sample_format;
> pa_path_get_filename;
> +pa_proplist_apply_update_info;
> pa_proplist_clear;
> pa_proplist_contains;
> pa_proplist_copy;
> @@ -246,6 +247,9 @@ pa_proplist_to_string_sep;
> pa_proplist_unset;
> pa_proplist_unset_many;
> pa_proplist_update;
> +pa_proplist_update_info_add;
> +pa_proplist_update_info_free;
> +pa_proplist_update_info_new;
> pa_rtclock_now;
> pa_sample_format_is_be;
> pa_sample_format_is_le;
> diff --git a/src/pulse/proplist.c b/src/pulse/proplist.c
> index abd551b..5236179 100644
> --- a/src/pulse/proplist.c
> +++ b/src/pulse/proplist.c
> @@ -35,12 +35,22 @@
>
> #include "proplist.h"
>
> +struct pa_proplist_update_info {
> + pa_hashmap *items;
> +};
> +
> struct property {
> char *key;
> void *value;
> size_t nbytes;
> };
>
> +struct update_info_item {
> + pa_proplist_operation_t operation;
> + char *key;
> + char *sets_value;
> +};
> +
> #define MAKE_HASHMAP(p) ((pa_hashmap*) (p))
> #define MAKE_PROPLIST(p) ((pa_proplist*) (p))
>
> @@ -73,11 +83,33 @@ void pa_proplist_free(pa_proplist* p) {
> pa_hashmap_free(MAKE_HASHMAP(p), (pa_free_cb_t) property_free);
> }
>
> +/* Returns true if the proplist changed. */
> +static bool pa_proplist_sets_internal(pa_proplist *proplist, const char *key, const char *value) {
> + size_t nbytes;
> + struct property *property;
> +
> + nbytes = strlen(value) + 1;
> + property = pa_hashmap_get(MAKE_HASHMAP(proplist), key);
> +
> + if (!property) {
> + property = pa_xnew(struct property, 1);
> + property->key = pa_xstrdup(key);
> + pa_hashmap_put(MAKE_HASHMAP(proplist), property->key, property);
> + } else {
> + if (nbytes == property->nbytes && !memcmp(value, property->value, nbytes))
> + return false;
> +
> + pa_xfree(property->value);
> + }
> +
> + property->value = pa_xstrdup(value);
> + property->nbytes = nbytes;
> +
> + return true;
> +}
> +
> /** Will accept only valid UTF-8 */
> int pa_proplist_sets(pa_proplist *p, const char *key, const char *value) {
> - struct property *prop;
> - pa_bool_t add = FALSE;
> -
> pa_assert(p);
> pa_assert(key);
> pa_assert(value);
> @@ -85,18 +117,7 @@ int pa_proplist_sets(pa_proplist *p, const char *key, const char *value) {
> if (!pa_proplist_key_valid(key) || !pa_utf8_valid(value))
> return -1;
>
> - if (!(prop = pa_hashmap_get(MAKE_HASHMAP(p), key))) {
> - prop = pa_xnew(struct property, 1);
> - prop->key = pa_xstrdup(key);
> - add = TRUE;
> - } else
> - pa_xfree(prop->value);
> -
> - prop->value = pa_xstrdup(value);
> - prop->nbytes = strlen(value)+1;
> -
> - if (add)
> - pa_hashmap_put(MAKE_HASHMAP(p), prop->key, prop);
> + pa_proplist_sets_internal(p, key, value);
>
> return 0;
> }
> @@ -708,3 +729,172 @@ int pa_proplist_equal(pa_proplist *a, pa_proplist *b) {
>
> return 1;
> }
> +
> +static struct update_info_item *update_info_item_new(pa_proplist_operation_t operation, const char *key) {
> + struct update_info_item *item;
> +
> + pa_assert(key);
> +
> + item = pa_xnew0(struct update_info_item, 1);
> + item->operation = operation;
> + item->key = pa_xstrdup(key);
> +
> + return item;
> +}
> +
> +static void update_info_item_free(struct update_info_item *item) {
> + pa_assert(item);
> +
> + pa_xfree(item->sets_value);
> + pa_xfree(item->key);
> + pa_xfree(item);
> +}
> +
> +pa_proplist_update_info *pa_proplist_update_info_new(void) {
> + pa_proplist_update_info *info;
> +
> + info = pa_xnew0(pa_proplist_update_info, 1);
> + info->items = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
> +
> + return info;
> +}
> +
> +void pa_proplist_update_info_free(pa_proplist_update_info *info) {
> + pa_assert(info);
> +
> + pa_hashmap_free(info->items, (pa_free_cb_t) update_info_item_free);
> + pa_xfree(info);
> +}
> +
> +int pa_proplist_update_info_add(pa_proplist_update_info *info, int operation, ...) {
> + /* This function will first validate and copy the function arguments into
> + * the new_items hashmap. After all inputs have been validated, the
> + * new_items contents are moved into info->items. If validation errors
> + * occur, info won't be altered. */
> +
> + pa_proplist_operation_t op;
> + va_list ap;
> + pa_hashmap *new_items;
> + const char *key;
> + struct update_info_item *new_item;
> + void *state;
> +
> + pa_assert(info);
> +
> + op = operation;
> +
> + if (op == PA_PROPLIST_OPERATION_INVALID)
> + return 0;
> +
> + va_start(ap, operation);
> + new_items = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
> +
> + /* The "copy and validate arguments" loop. */
> + while (op != PA_PROPLIST_OPERATION_INVALID) {
> + switch (op) {
> + case PA_PROPLIST_OPERATION_UNSET: {
> + struct update_info_item *item;
> +
> + key = va_arg(ap, const char *);
> +
> + if (!key || !pa_proplist_key_valid(key))
> + goto fail;
> +
> + item = pa_hashmap_get(new_items, key);
> +
> + if (item)
> + item->operation = PA_PROPLIST_OPERATION_UNSET;
> + else {
> + item = update_info_item_new(PA_PROPLIST_OPERATION_UNSET, key);
> + pa_hashmap_put(new_items, key, item);
> + }
> +
> + break;
> + }
> +
> + case PA_PROPLIST_OPERATION_SETS: {
> + const char *value;
> + struct update_info_item *item;
> +
> + key = va_arg(ap, const char *);
> +
> + if (!key || !pa_proplist_key_valid(key))
> + goto fail;
> +
> + value = va_arg(ap, const char *);
> +
> + if (!value || !pa_utf8_valid(value))
> + goto fail;
> +
> + item = pa_hashmap_get(new_items, key);
> +
> + if (item)
> + item->operation = PA_PROPLIST_OPERATION_SETS;
> + else {
> + item = update_info_item_new(PA_PROPLIST_OPERATION_SETS, key);
> + pa_hashmap_put(new_items, key, item);
> + }
> +
> + pa_xfree(item->sets_value);
> + item->sets_value = pa_xstrdup(value);
> +
> + break;
> + }
> +
> + default:
> + goto fail;
> + }
> +
> + op = va_arg(ap, int);
> + }
> +
> + va_end(ap);
> +
> + /* The "move items from new_items to info->items" loop. */
> + PA_HASHMAP_FOREACH(new_item, new_items, state) {
> + struct update_info_item *old_item;
> +
> + old_item = pa_hashmap_remove(info->items, new_item->key);
> +
> + if (old_item)
> + update_info_item_free(old_item);
> +
> + pa_hashmap_put(info->items, new_item->key, new_item);
> + }
> +
> + pa_hashmap_free(new_items, NULL);
> +
> + return 0;
> +
> +fail:
> + va_end(ap);
> + pa_hashmap_free(new_items, (pa_free_cb_t) update_info_item_free);
> +
> + return -PA_ERR_INVALID;
> +}
> +
> +int pa_proplist_apply_update_info(pa_proplist *proplist, pa_proplist_update_info *info) {
> + struct update_info_item *item;
> + void *state;
> + bool changed = false;
> +
> + pa_assert(proplist);
> + pa_assert(info);
> +
> + PA_HASHMAP_FOREACH(item, info->items, state) {
> + switch (item->operation) {
> + case PA_PROPLIST_OPERATION_INVALID:
> + pa_assert_not_reached();
> +
> + case PA_PROPLIST_OPERATION_UNSET:
> + changed |= pa_proplist_unset(proplist, item->key) >= 0;
> + break;
> +
> + case PA_PROPLIST_OPERATION_SETS:
> + changed |= pa_proplist_sets_internal(proplist, item->key, item->sets_value);
> + break;
> + }
> + }
> +
> + return changed;
> +}
> diff --git a/src/pulse/proplist.h b/src/pulse/proplist.h
> index cb53cf4..a2bbeff 100644
> --- a/src/pulse/proplist.h
> +++ b/src/pulse/proplist.h
> @@ -273,6 +273,28 @@ PA_C_DECL_BEGIN
> * as keys and arbitrary data as values. \since 0.9.11 */
> typedef struct pa_proplist pa_proplist;
>
> +/** A description of changes to be applied to a property list. \since 4.0 */
> +typedef struct pa_proplist_update_info pa_proplist_update_info;
> +
> +/** Operation type enumeration for pa_proplist_update_info_add(). \since 4.0 */
> +typedef enum {
> + PA_PROPLIST_OPERATION_INVALID = -1,
> + /**< Invalid operation type. */
> +
> + PA_PROPLIST_OPERATION_UNSET = 0,
> + /**< Unset a property. Takes the property name as a parameter. */
> +
> + PA_PROPLIST_OPERATION_SETS
> + /**< Set a string property. Takes the property name and the new value as
> + * parameters. Accepts only UTF-8 strings. */
> +} pa_proplist_operation_t;
> +
> +/** \cond fulldocs */
> +#define PA_PROPLIST_OPERATION_INVALID PA_PROPLIST_OPERATION_INVALID
> +#define PA_PROPLIST_OPERATION_UNSET PA_PROPLIST_OPERATION_UNSET
> +#define PA_PROPLIST_OPERATION_SETS PA_PROPLIST_OPERATION_SETS
> +/** \endcond */
> +
> /** Allocate a property list. \since 0.9.11 */
> pa_proplist* pa_proplist_new(void);
>
> @@ -406,6 +428,54 @@ int pa_proplist_isempty(pa_proplist *p);
> * \since 0.9.16 */
> int pa_proplist_equal(pa_proplist *a, pa_proplist *b);
>
> +/** Allocate an empty update info object. \since 4.0 */
> +pa_proplist_update_info *pa_proplist_update_info_new(void);
> +
> +/** Frees "info". \since 4.0 */
> +void pa_proplist_update_info_free(pa_proplist_update_info *info);
> +
> +/** Add update operations to "info". The arguments are a list of operations,
> + * each item in the list consisting of the operation type and parameters
> + * specific for that operation. The operation type must be one of the constants
> + * defined in the #pa_proplist_operation_t enumeration. The operation
> + * parameters are documented in the documentation of #pa_proplist_operation_t.
> + * The list is terminated with #PA_PROPLIST_OPERATION_INVALID. Any strings
> + * given as operation parameters are copied. Returns 0 on success, or
> + * -PA_ERR_INVALID in case of invalid arguments. In case of failure, "info" is
> + * not modified at all.
> + *
> + * Example:
> + *
> + * char *foo = ...;
> + * char *bar = ...;
> + * pa_proplist *proplist = ...;
> + * pa_proplist_update_info *info = pa_proplist_info_new();
> + * int changed;
> + *
> + * if (foo && bar)
> + * pa_proplist_update_info_add(info,
> + * PA_PROPLIST_OPERATION_SETS, "some.property", foo,
> + * PA_PROPLIST_OPERATION_SETS, "some.other.property", bar,
> + * PA_PROPLIST_OPERATION_INVALID);
> + * else
> + * pa_proplist_update_info_add(info,
> + * PA_PROPLIST_OPERATION_UNSET, "some.property",
> + * PA_PROPLIST_OPEARTION_UNSET, "some.other.property",
> + * PA_PROPLIST_OPERATION_INVALID);
> + *
> + * changed = pa_proplist_apply_update_info(proplist, info);
> + * pa_proplist_update_info_free(info);
> + *
> + * if (changed)
> + * printf("Property list changed!\n");
> + *
> + * \since 4.0 */
> +int pa_proplist_update_info_add(pa_proplist_update_info *info, int operation, ...);
> +
> +/** Apply a list of updates to "proplist". Returns a non-zero value if the
> + * property list changed. \since 4.0 */
> +int pa_proplist_apply_update_info(pa_proplist *proplist, pa_proplist_update_info *info);
> +
> PA_C_DECL_END
>
> #endif
>
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
