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
