[pulseaudio-discuss] [PATCH 4/6] Notify port available status changes

[Tanu Kaskinen]

Review below. Slightly bigger complaints this time :/

-- 
Tanu


On Thu, 2011-10-27 at 16:45 +0200, David Henningsson wrote:
> The recommended way of setting available status is to call
> pa_device_port_set_available, which will send subscriptions to
> relevant card, sink and source. It will also fire a hook.
> 
> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> ---
>  src/pulsecore/core.h        |    1 +
>  src/pulsecore/device-port.c |   36 ++++++++++++++++++++++++++++++++++--
>  src/pulsecore/device-port.h |    5 +++++
>  3 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/src/pulsecore/core.h b/src/pulsecore/core.h
> index d0641cf..ba21fa9 100644
> --- a/src/pulsecore/core.h
> +++ b/src/pulsecore/core.h
> @@ -113,6 +113,7 @@ typedef enum pa_core_hook {
>      PA_CORE_HOOK_CARD_PUT,
>      PA_CORE_HOOK_CARD_UNLINK,
>      PA_CORE_HOOK_CARD_PROFILE_CHANGED,
> +    PA_CORE_HOOK_PORT_AVAILABLE_CHANGED,
>      PA_CORE_HOOK_MAX
>  } pa_core_hook_t;
>  
> diff --git a/src/pulsecore/device-port.c b/src/pulsecore/device-port.c
> index b30484a..e70cddd 100644
> --- a/src/pulsecore/device-port.c
> +++ b/src/pulsecore/device-port.c
> @@ -20,11 +20,43 @@
>    USA.
>  ***/
>  
> -
> -#include "device-port.h"
> +#include <pulsecore/device-port.h>

Why this change? See http://pulseaudio.org/wiki/CodingStyle - the old
way was correct.

> +#include <pulsecore/card.h>
>  
>  PA_DEFINE_PUBLIC_CLASS(pa_device_port, pa_object);
>  
> +void pa_device_port_set_available(pa_device_port *p, pa_port_available_t status, pa_core *core)
> +{
> +    uint32_t state;
> +    pa_card *card;
> +    pa_source *source;
> +    pa_sink *sink;
> +
> +    pa_assert(p);
> +
> +    if (p->available == status)
> +        return;
> +
> +    pa_assert(status != PA_PORT_AVAILABLE_UNKNOWN);

Why? I don't think we can guarantee that we will never lose track of
port availability.

> +    p->available = status;
> +    pa_log_debug("Setting port %s to status %s", p->name, status == PA_PORT_AVAILABLE_YES ? "yes" : "no");
> +
> +    /* Post subscriptions to everyone who own us */
> +    PA_IDXSET_FOREACH(card, core->cards, state)
> +        if (p == pa_hashmap_get(card->ports, p->name))
> +            pa_subscription_post(core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_CHANGE, card->index);
> +    if (p->is_output)
> +        PA_IDXSET_FOREACH(sink, core->sinks, state)
> +            if (p == pa_hashmap_get(sink->ports, p->name))
> +                pa_subscription_post(core, PA_SUBSCRIPTION_EVENT_SINK|PA_SUBSCRIPTION_EVENT_CHANGE, sink->index);
> +    if (p->is_input)
> +        PA_IDXSET_FOREACH(source, core->sources, state)
> +            if (p == pa_hashmap_get(source->ports, p->name))
> +                pa_subscription_post(core, PA_SUBSCRIPTION_EVENT_SOURCE|PA_SUBSCRIPTION_EVENT_CHANGE, source->index);

I'd like ports to have their own subscription class.

> +
> +    pa_hook_fire(&core->hooks[PA_CORE_HOOK_PORT_AVAILABLE_CHANGED], p);
> +}
> +
>  static void device_port_free(pa_object *o) {
>      pa_device_port *p = PA_DEVICE_PORT(o);
>      pa_assert(p);
> diff --git a/src/pulsecore/device-port.h b/src/pulsecore/device-port.h
> index f01d736..a4894e1 100644
> --- a/src/pulsecore/device-port.h
> +++ b/src/pulsecore/device-port.h
> @@ -61,4 +61,9 @@ pa_device_port *pa_device_port_new(const char *name, const char *description, si
>  
>  void pa_device_port_hashmap_free(pa_hashmap *h);
>  
> +#include <pulsecore/core.h>
> +
> +/* The port's available status has changed */
> +void pa_device_port_set_available(pa_device_port *p, pa_port_available_t available, pa_core *core);

I don't like that core parameter. The port should know by itself which
core it belongs to.

> +
>  #endif