[pulseaudio-discuss] [PATCH v1 3/4] sink: Inherit the volume from the active port.

[Tanu Kaskinen]

On Sun, 2013-03-17 at 21:48 +0100, poljar (Damir Jelić) wrote:
> Since the port now holds the volume information we can simply inherit
> the volume from the active port if the volume isn't set while creating
> the sink.
> 
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=55262
> ---
>  src/pulsecore/sink.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)

One thing missing: the sink reference volume can change in
pa_sink_put(). This change needs to be propagated to the active port.

> diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
> index 6ebe956..a56a0b9 100644
> --- a/src/pulsecore/sink.c
> +++ b/src/pulsecore/sink.c
> @@ -277,7 +277,6 @@ pa_sink* pa_sink_new(
>      s->n_corked = 0;
>      s->input_to_master = NULL;
>  
> -    s->reference_volume = s->real_volume = data->volume;
>      pa_cvolume_reset(&s->soft_volume, s->sample_spec.channels);
>      s->base_volume = PA_VOLUME_NORM;
>      s->n_volume_steps = PA_VOLUME_NORM+1;
> @@ -310,10 +309,18 @@ pa_sink* pa_sink_new(
>                  s->active_port = p;
>      }
>  
> -    if (s->active_port)
> +    if (s->active_port) {
>          s->latency_offset = s->active_port->latency_offset;
> -    else
> +
> +        if (pa_cvolume_valid(&s->active_port->volume) && !data->volume_is_set)

If you follow my suggestions for the previous patches, the port volume
will always be valid, so no need to check it here.

> +            s->reference_volume = s->real_volume = s->active_port->volume;
> +        else
> +            s->reference_volume = s->real_volume = data->volume;
> +
> +    } else {
>          s->latency_offset = 0;
> +        s->reference_volume = s->real_volume = data->volume;
> +    }
>  
>      s->save_volume = data->save_volume;
>      s->save_muted = data->save_muted;
> @@ -1972,6 +1979,8 @@ void pa_sink_set_volume(
>          }
>  
>          pa_cvolume_remap(&new_reference_volume, &s->channel_map, &root_sink->channel_map);
> +        if (s->active_port)
> +            pa_device_port_set_volume(s->active_port, new_reference_volume);

A better place for this would be in update_reference_volume(). That's
where the check will catch all changes to the reference volume. If the
check is here, several volume update cases will be missed.

-- 
Tanu