I was asked by Colin to give this a quick review, so...
On 2011-05-19 18:44, Jorge Eduardo Candelaria wrote:
> From: Margarita Olaya Cabrera<magi at slimlogic.co.uk>
>
> This patch adds support to module-alsa-card so that we can read jack insertion
> and removal events using the input device subsystem. i.e. we can detect
> headphone insertions and removals.
>
> This patch adds a new module parameter called jack_id that contains the ID of
> the jack input device associated with this sound card. If we receive a valid
> jack_id during module init then we start a reader thread that will read the
> jack input device and fire a hook on every removal and insertion event.
>
> Jack support development was kindly sponsored by Wolfson Microelectronics PLC
>
> Signed-off-by: Margarita Olaya Cabrera<magi at slimlogic.co.uk>
> Signed-off-by: Jorge Eduardo Candelaria<jedu at slimlogic.co.uk>
> ---
> src/modules/alsa/alsa-jack.h | 42 ++++++++++++
> src/modules/alsa/module-alsa-card.c | 120 +++++++++++++++++++++++++++++++++++
> src/pulsecore/core.h | 2 +
> 3 files changed, 164 insertions(+), 0 deletions(-)
> create mode 100644 src/modules/alsa/alsa-jack.h
>
> diff --git a/src/modules/alsa/alsa-jack.h b/src/modules/alsa/alsa-jack.h
> new file mode 100644
> index 0000000..4fc67c6
> --- /dev/null
> +++ b/src/modules/alsa/alsa-jack.h
Along the lines of Tanu, if this is not alsa-specific, let's take this
header and move into pulsecore.
> @@ -0,0 +1,42 @@
> +#ifndef foopulsejackdetecthfoo
> +#define foopulsejackdetecthfoo
> +
> +/***
> + This file is part of PulseAudio.
> +
> + Copyright 2011 Wolfson Microelectronics PLC
> + Author Margarita Olaya<magi at slimlogic.co.uk>
> +
> + PulseAudio is free software; you can redistribute it and/or modify
> + it under the terms of the GNU Lesser General Public License as published
> + by the Free Software Foundation; either version 2.1 of the License,
> + or (at your option) any later version.
> +
> + PulseAudio is distributed in the hope that it will be useful, but
> + WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public License
> + along with PulseAudio; if not, write to the Free Software
> + Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> + USA.
> +***/
> +
> +#include<inttypes.h>
> +
> +typedef enum pa_jack_event {
> + PA_JACK_HEADPHONES,
> + PA_JACK_HEADSET,
I'm a little hesitant to whether we should have one "headset" or whether
that should be emulated by one "headphone" and one "microphone", as that
is what a headset is.
> + PA_JACK_MICROPHONE,
> + PA_JACK_LINEOUT,
add PA_JACK_VIDEOOUT for HDMI/Displayport (which was just added to ALSA).
> + PA_JACK_UNKNOWN,
> + PA_JACK_MAX
> +} pa_jack_event_t;
> +
> +typedef struct pa_jack_detect {
> + pa_jack_event_t event;
> + char *card;
Agree with Tanu about pa_card pointer here.
> +} pa_jack_detect_t;
> +
> +#endif
> diff --git a/src/modules/alsa/module-alsa-card.c b/src/modules/alsa/module-alsa-card.c
> index e60aa5e..75202fb 100644
> --- a/src/modules/alsa/module-alsa-card.c
> +++ b/src/modules/alsa/module-alsa-card.c
> @@ -29,6 +29,9 @@
> #include<pulsecore/core-util.h>
> #include<pulsecore/modargs.h>
> #include<pulsecore/queue.h>
> +#include<pulsecore/thread.h>
> +
> +#include<linux/input.h>
>
> #include<modules/reserve-wrap.h>
>
> @@ -39,6 +42,7 @@
> #include "alsa-util.h"
> #include "alsa-sink.h"
> #include "alsa-source.h"
> +#include "alsa-jack.h"
> #include "module-alsa-card-symdef.h"
>
> PA_MODULE_AUTHOR("Lennart Poettering");
> @@ -55,6 +59,7 @@ PA_MODULE_USAGE(
> "source_properties=<properties for the source> "
> "namereg_fail=<pa_namereg_register() fail parameter value> "
> "device_id=<ALSA card index> "
> + "jack_id=<Jack device index> "
Eh, a card can have more than one jack, and is further rather connected
to a port rather than a card, so this (and thus the rest of the
implementation) doesn't seem right to me.
--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
