[pulseaudio-discuss] [PATCH 2/2] module-ducking: Applying the ducking effect for specified streams

[Tanu Kaskinen]

On Mon, 2012-10-22 at 12:22 -0200, Flávio Ceolin wrote:
> Hi Tanu,
> 
> On Fri, Oct 19, 2012 at 3:33 PM, Tanu Kaskinen <tanuk at iki.fi> wrote:
> > On Tue, 2012-10-09 at 15:16 -0300, Flavio Ceolin wrote:
> >> +        "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).
> >
> 
> So, instead of accept a linear value, i'll give to user three options
> dB, % and integer, right ?

Right. For bonus points, add pa_parse_volume() in
pulsecore/core-util.[ch] and use it from pa_modargs_get_value_volume().

> >> +                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.
> 
> If multiple modules can apply volume factors, should exists some kind
> of priority between them ?
> is there a similar case in pulseaudio that i can take a look ?

I don't think there needs to be any priority, since all of the factors
can be applied simultaneously. For example, one module can apply -10 dB
attenuation and another module can apply -5 dB attenuation. When
combined, the result is -15 dB attenuation, which is expected.

Actually, maybe there's no need to have a list of factors, because a
module can remember what factor it applied, and when it doesn't want to
apply it anymore, it can undo it. There might be rounding errors,
though, so when volume_factor starts at PA_VOLUME_NORM, after a
temporary volume factor has been applied and then undone by a module,
volume_factor may end up being something else than PA_VOLUME_NORM. So,
I'd go with a list after all...

Combining factors can be done simply with pa_sw_cvolume_multiply().

-- 
Tanu