On Wed, 2016-03-30 at 22:20 +0900, Sangchul Lee wrote:
> +struct userdata {
> +Â Â Â Â pa_core *core;
> +Â Â Â Â const char *name;
userdata doesn't need the name field any more. It's only referenced
once in pa_stream_interaction_init(), and that function can equally
well use m->name in place of u->name.
> -static void uncork_or_unduck(struct userdata *u, pa_sink_input *i, const char *interaction_role, bool corked) {
> +static void uncork_or_unduck(struct userdata *u, pa_sink_input *i, const char *interaction_role, bool corked, struct group *g) {
> Â
> Â Â Â Â Â if (u->duck) {
> -Â Â Â Â Â Â Â pa_log_debug("Found a '%s' stream that should be unducked", interaction_role);
> -Â Â Â Â Â Â Â pa_sink_input_remove_volume_factor(i, u->name);
> +Â Â Â Â Â Â Â pa_log_debug("Found a '%s' stream that should be unducked by '%s'", interaction_role, g->name);
Groups don't unduck streams, so the log message is not good language.
"In '%s', found a '%s' stream that should be unducked" would be better
(the first %s is the group name).
> Â static pa_hook_result_t process(struct userdata *u, pa_sink_input *i, bool create, bool new_stream) {
> Â Â Â Â Â const char *trigger_role;
> +Â Â Â Â uint32_t j;
> Â
> Â Â Â Â Â pa_assert(u);
> Â Â Â Â Â pa_sink_input_assert_ref(i);
> Â
> -Â Â Â Â if (!create)
> -Â Â Â Â Â Â Â Â pa_hashmap_remove(u->interaction_state, i);
> -
> Â Â Â Â Â if (!i->sink)
> Â Â Â Â Â Â Â Â Â return PA_HOOK_OK;
> Â
> -Â Â Â Â trigger_role = find_global_trigger_stream(u, i->sink, create ? NULL : i);
> -Â Â Â Â apply_interaction(u, i->sink, trigger_role, create ? NULL : i, new_stream);
> +Â Â Â Â for (j = 0; j < u->n_groups; j++) {
> +Â Â Â Â Â Â Â Â if (!create)
> +Â Â Â Â Â Â Â Â Â Â Â Â pa_hashmap_remove(u->groups[j]->interaction_state, i);
> +Â Â Â Â Â Â Â Â trigger_role = find_global_trigger_stream(u, i->sink, create ? NULL : i, u->groups[j]);
> +Â Â Â Â Â Â Â Â apply_interaction(u, i->sink, trigger_role, create ? NULL : i, new_stream, u->groups[j]);
> +Â Â Â Â }
The pa_hashmap_remove() call was moved after the if (!i->sink) check,
which means that if i->sink is not set, pa_hashmap_remove() doesn't any
more get called even if it should.
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (pa_parse_volume(n, &(u->groups[i++]->volume)) == -1) {
Use "< 0" instead of "== -1". Any negative number indicates a failure,
so comparing to only -1 makes it look like -1 would be somehow special.
The reader can't be expected to know that the function never returns
other error codes than -1.
--
Tanu
