[pulseaudio-discuss] [PATCH 2/2] module-ducking: Applying the ducking effect for specified streams
Tanu Kaskinen
tanuk at iki.fi
Fri Oct 19 11:33:10 PDT 2012
On Tue, 2012-10-09 at 15:16 -0300, Flavio Ceolin wrote:
> This module works pretty similar to the module-role-cork.
> It should be used as an alternative to that module. Basically
> it decreases the volume of the streams specified in ducking_roles
> in the presence of at least one stream specified in trigger_roles.
> Also, it's possible to choice the volume that will be used in the
> ducking streams and if it should operates in all devices or not.
>
> For basic reference: http://en.wikipedia.org/wiki/Ducking
> ---
> src/Makefile.am | 10 +-
> src/modules/module-role-ducking.c | 330 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 339 insertions(+), 1 deletion(-)
> create mode 100644 src/modules/module-role-ducking.c
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 6212c74..a12df63 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1053,7 +1053,8 @@ modlibexec_LTLIBRARIES += \
> module-switch-on-connect.la \
> module-switch-on-port-available.la \
> module-filter-apply.la \
> - module-filter-heuristics.la
> + module-filter-heuristics.la \
> + module-role-ducking.la
>
> if HAVE_ESOUND
> modlibexec_LTLIBRARIES += \
> @@ -1361,6 +1362,7 @@ SYMDEF_FILES = \
> module-raop-discover-symdef.h \
> module-gconf-symdef.h \
> module-position-event-sounds-symdef.h \
> + module-role-ducking-symdef.h \
> module-augment-properties-symdef.h \
> module-role-cork-symdef.h \
> module-console-kit-symdef.h \
> @@ -1758,6 +1760,12 @@ module_position_event_sounds_la_LDFLAGS = $(MODULE_LDFLAGS)
> module_position_event_sounds_la_LIBADD = $(MODULE_LIBADD)
> module_position_event_sounds_la_CFLAGS = $(AM_CFLAGS)
>
> +# Ducking effect in presence of a DUCKING stream
I think "Ducking effect based on stream roles" would be a more
descriptive comment.
> +module_role_ducking_la_SOURCES = modules/module-role-ducking.c
> +module_role_ducking_la_LDFLAGS = $(MODULE_LDFLAGS)
> +module_role_ducking_la_LIBADD = $(MODULE_LIBADD)
> +module_role_ducking_la_CFLAGS = $(AM_CFLAGS)
> +
> # Augment properties from XDG .desktop files
> module_augment_properties_la_SOURCES = modules/module-augment-properties.c
> module_augment_properties_la_LDFLAGS = $(MODULE_LDFLAGS)
> diff --git a/src/modules/module-role-ducking.c b/src/modules/module-role-ducking.c
> new file mode 100644
> index 0000000..428dc37
> --- /dev/null
> +++ b/src/modules/module-role-ducking.c
> @@ -0,0 +1,330 @@
> +/***
> + This file is part of PulseAudio.
> +
> + Copyright 2012 Flavio Ceolin <flavio.ceolin at profusion.mobi>
> +
> + PulseAudio is free software; you can redistribute it and/or modify
> + it under the terms of the GNU Lesser General Public License as published
> + by the Free Software Foundation; either version 2.1 of the License,
> + or (at your option) any later version.
> +
> + PulseAudio is distributed in the hope that it will be useful, but
> + WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public License
> + along with PulseAudio; if not, write to the Free Software
> + Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> + USA.
> +***/
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <pulse/volume.h>
> +#include <pulse/xmalloc.h>
> +
> +#include <pulsecore/macro.h>
> +#include <pulsecore/hashmap.h>
> +#include <pulsecore/hook-list.h>
> +#include <pulsecore/core.h>
> +#include <pulsecore/core-util.h>
> +#include <pulsecore/sink-input.h>
> +#include <pulsecore/modargs.h>
> +
> +#include "module-role-ducking-symdef.h"
> +
> +
> +PA_MODULE_AUTHOR("Flavio Ceolin <flavio.ceolin at profusion.mobi>");
> +PA_MODULE_DESCRIPTION("Apply ducking effect on streams with certain roles while others exist");
> +PA_MODULE_VERSION(PACKAGE_VERSION);
> +PA_MODULE_LOAD_ONCE(TRUE);
> +PA_MODULE_USAGE(
> + "trigger_roles=<Comma separated list of roles which will trigger a ducking> "
> + "ducking_roles=<Comma separated list of roles which will be duckinged> "
I don't think "duckinged" is a proper word. Use "ducked" instead (this
is not the only occurrence, so please replace all instances of
"duckinged").
> + "global=<Should we operate globally or only inside the same device?>"
> + "volume=<linear factor for the volume. 0.0 and less is muted while 1.0 is PA_VOLUME_NORM>");
I don't like the choice of using linear factor as the parameter type.
The "native" volume type is pa_volume_t. So, I'd like you to use either
that type as the parameter type, or if that's too un-user-friendly (a
linear factor isn't very user-friendly either, btw), then accept any of
these: a plain integer (pa_volume_t), a number followed by a percentage
sign or a number followed by "dB". There could be
pa_modargs_get_value_volume() that does the parsing (it could be useful
elsewhere too).
> +
> +static const char* const valid_modargs[] = {
> + "trigger_roles",
> + "ducking_roles",
> + "global",
> + "volume",
> + NULL
> +};
> +
> +struct userdata {
> + pa_core *core;
> + pa_hashmap *ducking_state;
I'd like a comment here about the key and value types.
> + pa_idxset *trigger_roles;
> + pa_idxset *ducking_roles;
> + pa_bool_t global:1;
> + pa_volume_t volume;
> + pa_hook_slot
> + *sink_input_put_slot,
> + *sink_input_unlink_slot,
> + *sink_input_move_start_slot,
> + *sink_input_move_finish_slot;
> +};
> +
> +static pa_bool_t shall_ducking(struct userdata *u, pa_sink *s, pa_sink_input *ignore) {
> + pa_sink_input *j;
> + uint32_t idx, role_idx;
> + const char *trigger_role;
> +
> + pa_assert(u);
> + pa_sink_assert_ref(s);
> +
> + for (j = PA_SINK_INPUT(pa_idxset_first(s->inputs, &idx)); j; j = PA_SINK_INPUT(pa_idxset_next(s->inputs, &idx))) {
The PA_IDXSET_FOREACH macro would be prettier.
> + const char *role;
> +
> + if (j == ignore)
> + continue;
> +
> + if (!(role = pa_proplist_gets(j->proplist, PA_PROP_MEDIA_ROLE)))
> + continue;
> +
> + PA_IDXSET_FOREACH(trigger_role, u->trigger_roles, role_idx) {
> + if (pa_streq(role, trigger_role)) {
> + pa_log_debug("Found a '%s' stream that will trigger the ducking.", trigger_role);
> + return TRUE;
> + }
> + }
> + }
> +
> + return FALSE;
> +}
> +
> +static inline void apply_ducking_to_sink(struct userdata *u, pa_sink *s, pa_sink_input *ignore, pa_bool_t ducking) {
The "inline" specifier seems out of place. The compiler knows probably
better than you whether it makes sense to inline this function or not,
and this is not performance critical code anyway.
> + pa_sink_input *j;
> + uint32_t idx, role_idx;
> + const char *ducking_role;
> + pa_bool_t trigger = FALSE;
> + pa_cvolume *v;
> +
> + pa_assert(u);
> + pa_sink_assert_ref(s);
> +
> + for (j = PA_SINK_INPUT(pa_idxset_first(s->inputs, &idx)); j; j = PA_SINK_INPUT(pa_idxset_next(s->inputs, &idx))) {
The PA_IDXSET_FOREACH macro would be prettier.
> + const char *role;
> +
> + if (j == ignore)
> + continue;
> +
> + if (!(role = pa_proplist_gets(j->proplist, PA_PROP_MEDIA_ROLE)))
> + continue;
> +
> + PA_IDXSET_FOREACH(ducking_role, u->ducking_roles, role_idx) {
> + if ((trigger = pa_streq(role, ducking_role)))
> + break;
> + }
> + if (!trigger)
> + continue;
> +
> + if (ducking) {
> + pa_cvolume aux;
> + v = pa_hashmap_get(u->ducking_state, j);
> +
> + pa_log_debug("Found a '%s' stream that should be duckinged.", ducking_role);
> + if (!v) {
> + v = pa_xmalloc(sizeof (pa_cvolume));
The convention is to use pa_xnew(1, pa_cvolume).
> + pa_hashmap_put(u->ducking_state, j, pa_sink_input_get_volume(j, v, FALSE));
> + }
> +
> + aux = *v;
> + pa_cvolume_set(&aux, aux.channels, u->volume);
> + pa_sink_input_set_volume(j, &aux, FALSE, FALSE);
I don't think the volume change should be visible outside, so
pa_sink_input_set_volume() shouldn't be called. Instead,
j->volume_factor should be set (it's a sort of "invisible" volume
modifier). There's no good way to do that, however. The volume factor
feature is used very little, and there's currently no code that needs to
update it on the fly - currently it's only set once during stream
creation and it stays the same for the lifetime of the stream. I think
pa_sink_input_set_volume_factor() needs to be created, which would also
update j->soft_volume (and j->thread_info.soft_volume).
Or even better, pa_sink_input.volume_factor could be replaced with a
list (or whatever container makes most sense) of volume factors. There
can be multiple modules, each independently applying volume factors. If
there is only one volume variable, those modules will overwrite each
other's volumes. Therefore, it's better to give each module its own
volume factor object.
> + } else {
> + v = pa_hashmap_remove(u->ducking_state, j);
> + if (v) {
> + pa_log_debug("Found a '%s' stream that should be unduckinged.", ducking_role);
> + pa_sink_input_set_volume(j, v, FALSE, FALSE);
> + pa_xfree(v);
> + }
> + }
> + }
> +}
> +
> +static void apply_ducking(struct userdata *u, pa_sink *s, pa_sink_input *ignore, pa_bool_t ducking) {
> + pa_assert(u);
> +
> + if (u->global) {
> + uint32_t idx;
> + PA_IDXSET_FOREACH(s, u->core->sinks, idx)
> + apply_ducking_to_sink(u, s, ignore, ducking);
> + } else
> + apply_ducking_to_sink(u, s, ignore, ducking);
> +}
> +
> +static pa_hook_result_t process(struct userdata *u, pa_sink_input *i, pa_bool_t create) {
"create" is a very confusing variable name, at least for me. It's not
clear at all what is being created, even after reading the code.
> + pa_bool_t ducking = FALSE;
> + const char *role;
> + pa_cvolume *v;
> +
> + pa_assert(u);
> + pa_sink_input_assert_ref(i);
> +
> + if (!create) {
> + v = pa_hashmap_remove(u->ducking_state, i);
> + pa_xfree(v);
> + }
> +
> + if (!(role = pa_proplist_gets(i->proplist, PA_PROP_MEDIA_ROLE)))
> + return PA_HOOK_OK;
> +
> + if (!i->sink)
> + return PA_HOOK_OK;
> +
> + ducking = shall_ducking(u, i->sink, create ? NULL : i);
> + apply_ducking(u, i->sink, create ? NULL : i, ducking);
> +
> + return PA_HOOK_OK;
> +}
> +
> +static pa_hook_result_t sink_input_put_cb(pa_core *core, pa_sink_input *i, struct userdata *u) {
> + pa_core_assert_ref(core);
> + pa_sink_input_assert_ref(i);
> +
> + return process(u, i, TRUE);
> +}
> +
> +static pa_hook_result_t sink_input_unlink_cb(pa_core *core, pa_sink_input *i, struct userdata *u) {
> + pa_sink_input_assert_ref(i);
> +
> + return process(u, i, FALSE);
> +}
> +
> +static pa_hook_result_t sink_input_move_start_cb(pa_core *core, pa_sink_input *i, struct userdata *u) {
> + pa_core_assert_ref(core);
> + pa_sink_input_assert_ref(i);
> +
> + return process(u, i, FALSE);
> +}
> +
> +static pa_hook_result_t sink_input_move_finish_cb(pa_core *core, pa_sink_input *i, struct userdata *u) {
> + pa_core_assert_ref(core);
> + pa_sink_input_assert_ref(i);
> +
> + return process(u, i, TRUE);
> +}
> +
> +int pa__init(pa_module *m) {
> + pa_modargs *ma = NULL;
> + struct userdata *u;
> + const char *roles;
> + pa_bool_t global = FALSE;
> + double volume = 0.15;
> +
> + pa_assert(m);
> +
> + if (!(ma = pa_modargs_new(m->argument, valid_modargs))) {
> + pa_log("Failed to parse module arguments");
> + goto fail;
> + }
> +
> + m->userdata = u = pa_xnew(struct userdata, 1);
> +
> + u->core = m->core;
> + u->ducking_state = pa_hashmap_new(pa_idxset_trivial_hash_func, pa_idxset_trivial_compare_func);
> +
> + u->trigger_roles = pa_idxset_new(NULL, NULL);
> + roles = pa_modargs_get_value(ma, "trigger_roles", NULL);
> + if (roles) {
> + const char *split_state = NULL;
> + char *n = NULL;
> + while ((n = pa_split(roles, ",", &split_state))) {
> + if (n[0] != '\0')
> + pa_idxset_put(u->trigger_roles, n, NULL);
> + else
> + pa_xfree(n);
> + }
> + }
> + if (pa_idxset_isempty(u->trigger_roles)) {
> + pa_log_debug("Using role 'phone' as trigger role.");
> + pa_idxset_put(u->trigger_roles, pa_xstrdup("phone"), NULL);
> + }
> +
> + u->ducking_roles = pa_idxset_new(NULL, NULL);
> + roles = pa_modargs_get_value(ma, "ducking_roles", NULL);
> + if (roles) {
> + const char *split_state = NULL;
> + char *n = NULL;
> + while ((n = pa_split(roles, ",", &split_state))) {
> + if (n[0] != '\0')
> + pa_idxset_put(u->ducking_roles, n, NULL);
> + else
> + pa_xfree(n);
> + }
> + }
> + if (pa_idxset_isempty(u->ducking_roles)) {
> + pa_log_debug("Using roles 'music' and 'video' as ducking roles.");
> + pa_idxset_put(u->ducking_roles, pa_xstrdup("music"), NULL);
> + pa_idxset_put(u->ducking_roles, pa_xstrdup("video"), NULL);
> + }
> +
> + if (pa_modargs_get_value_boolean(ma, "global", &global) < 0) {
> + pa_log("Invalid boolean parameter: global");
> + goto fail;
> + }
> + u->global = global;
> +
> + if (pa_modargs_get_value_double(ma, "volume", &volume) < 0) {
> + pa_log("Invalid unsigned integer parameter: volume");
The log message talks about unsigned integers, while the variable is a
double.
--
Tanu
More information about the pulseaudio-discuss
mailing list