[pulseaudio-discuss] [PATCH 1/2] alsa: add jack detection support

[David Henningsson]

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