[pulseaudio-discuss] [PATCH v1 1/4] device-port: Add volume to the ports struct.

[Tanu Kaskinen]

On Sun, 2013-03-17 at 21:48 +0100, poljar (Damir Jelić) wrote:
> This patch adds a volume to the ports.
> Now we can save and restore the volume per port.
> 
> This adds a new hook:
>     PA_CORE_HOOK_PORT_VOLUME_CHANGED
> 
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=55262
> ---
>  src/pulsecore/core.h        |  1 +
>  src/pulsecore/device-port.c | 15 +++++++++++++++
>  src/pulsecore/device-port.h |  3 +++
>  3 files changed, 19 insertions(+)
> 
> diff --git a/src/pulsecore/core.h b/src/pulsecore/core.h
> index 381897a..078c237 100644
> --- a/src/pulsecore/core.h
> +++ b/src/pulsecore/core.h
> @@ -118,6 +118,7 @@ typedef enum pa_core_hook {
>      PA_CORE_HOOK_PORT_AVAILABLE_CHANGED,
>      PA_CORE_HOOK_PORT_ADDED,
>      PA_CORE_HOOK_PORT_LATENCY_OFFSET_CHANGED,
> +    PA_CORE_HOOK_PORT_VOLUME_CHANGED,
>      PA_CORE_HOOK_MAX
>  } pa_core_hook_t;
>  
> diff --git a/src/pulsecore/device-port.c b/src/pulsecore/device-port.c
> index f16de3a..608effa 100644
> --- a/src/pulsecore/device-port.c
> +++ b/src/pulsecore/device-port.c
> @@ -84,6 +84,7 @@ pa_device_port *pa_device_port_new(pa_core *c, const char *name, const char *des
>      p->is_input = FALSE;
>      p->is_output = FALSE;
>      p->latency_offset = 0;
> +    pa_cvolume_init(&p->volume);

This initializes the volume to an invalid value. We should ensure that
by the time the port is linked the volume is valid. We can't assume that
there is always some module that will set it. I think it's best to
create pa_device_port_new_data, put volume and volume_is_set fields to
it, and then initialize the volume to a valid default value in
pa_device_port_new() if the new_data doesn't have volume set.

>      p->proplist = pa_proplist_new();
>  
>      return p;
> @@ -123,3 +124,17 @@ void pa_device_port_set_latency_offset(pa_device_port *p, int64_t offset) {
>      pa_subscription_post(core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_CHANGE, p->card->index);
>      pa_hook_fire(&core->hooks[PA_CORE_HOOK_PORT_LATENCY_OFFSET_CHANGED], p);
>  }
> +
> +void pa_device_port_set_volume(pa_device_port *p, pa_cvolume volume) {

Structs should be passed by reference, not by value.

> +    pa_core *core;
> +
> +    pa_assert(p);
> +
> +    if (pa_cvolume_equal(&p->volume, &volume))
> +        return;
> +
> +    p->volume = volume;
> +
> +    pa_assert_se(core = p->core);

Nitpick: a separate variable for core doesn't seem to buy anything.

-- 
Tanu