[alsa-devel] [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device

[Tanu Kaskinen]

On Fri, 2015-03-20 at 15:18 +0100, Takashi Iwai wrote:
> At Fri, 20 Mar 2015 13:49:24 +0000,
> Jie, Yang wrote:
> > 
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai at suse.de]
> > > Sent: Friday, March 20, 2015 9:21 PM
> > > To: Jie, Yang
> > > Cc: perex at perex.cz; broonie at kernel.org; alsa-devel at alsa-project.org;
> > > Girdwood, Liam R; Liam Girdwood; Tanu Kaskinen
> > > (tanu.kaskinen at linux.intel.com)
> > > Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack
> > > input device
> > > 
> > > At Fri, 20 Mar 2015 12:50:33 +0000,
> > > Jie, Yang wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Takashi Iwai [mailto:tiwai at suse.de]
> > > > > Sent: Friday, March 20, 2015 8:27 PM
> > > > > To: Jie, Yang
> > > > > Cc: perex at perex.cz; broonie at kernel.org; alsa-devel at alsa-project.org;
> > > > > Girdwood, Liam R; Liam Girdwood
> > > > > Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for
> > > > > every jack input device
> > > > >
> > > > > At Fri, 20 Mar 2015 12:22:10 +0000,
> > > > > Jie, Yang wrote:
> > > > > >
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int snd_jack_new_kctl(struct snd_card *card, struct
> > > > > > > > +snd_jack *jack, int type) {
> > > > > > > > +	struct snd_kcontrol *kctl;
> > > > > > > > +	struct snd_jack_kctl *jack_kctl;
> > > > > > > > +	int i, err, index, state = 0 /* use 0 for default state ??
> > > > > > > > +*/;
> > > > > > > > +
> > > > > > > > +	INIT_LIST_HEAD(&jack->kctl_list);
> > > > > > > > +	for (i = 0; i < fls(SND_JACK_BTN_0); i++) {
> > > > > > > > +		int testbit = 1 << i;
> > > > > > > > +		if (type & testbit) {
> > > > > > >
> > > > > > > With this implementation, you'll get multiple boolean kctls for
> > > > > > > a headset.  I thought we agreed with creating a single boolean for
> > > headset?
> > > > > > [Keyon] We agreed with creating multiple kctls for combo jack, e.g.
> > > > > headset.
> > > > > > Furthermore, e.g., imagine that type = SND_JACK_HEADSET  |
> > > > > > SND_JACK_BTN_0, we will create 3 kctls for the jack, when BTN_0 is
> > > > > > pressed, we will report to the 3rd kctl.
> > > > >
> > > > > Hm, I don't remember that I agreed with multiple kctls...
> > > > >
> > > > > The multiple kctls have a significant drawback (multiple event calls
> > > > > for a single
> > > > > headset) and its behavior is incompatible with the current code
> > > > > (both the name change and the behavior change).  That is, your patch
> > > > > will very likely break the existing applications.
> > > > [Keyon] I am not very clear with the existing applications that using
> > > > these kctl events(seems Android use input subsystem event? Which we
> > > > didn't Change here. If I understand correctly, Pulseaudio uses jack
> > > > switch controls, via the name, then we can use different name for
> > > > headphone and mic here.)
> > > 
> > > PA uses jack kctls.
> > > 
> > > If you rename, how would you guarantee that the existing application will
> > > work as expected?  PA doesn't have the definition of "Headset Speaker Jack"
> > > or such.
> > > 
> > > And, no, we have no option "fix PA".  Other way round: we are not allowed
> > > to break the current PA (or any user-space) behavior in general.
> > > 
> > > > we will generate 2 event calls(one headphone, one mic) when Headset
> > > > plug in/out, applications will receive these 2 events,  and they can
> > > > do anything, e.g. decide to switch on/off speaker/headphone.
> > > 
> > > Won't this break any user-space stuff?
> > > 
> > > > BTW, I haven't implemented the generating of combo jack kctls' name
> > > > yet, currently, they looked like below:
> > > > numid=12,iface=CARD,name='Headset Jack'
> > > > numid=13,iface=CARD,name='Headset Jack',index=1
> > > > numid=14,iface=CARD,name='Headset Jack',index=2
> > > >
> > > > once we have come to agreement, we can modify it in
> > > > snd_jack_new_kctl(), e.g., "Headset Jack Mic" and "Headset Jack
> > > Speakers".
> > > 
> > > .... and how the existing user-space works without changing its code?
> > > 
> > > 
> > > Keyon, the most important point at this moment is to keep the compatibility.
> > > HD-audio is no new driver.  It's a driver that has been present over a decade
> > > with (literally) thousands of variants.
> > > Please keep this in mind, and reconsider whether your patch will retain the
> > > compatibility, especially with PulseAudio.
> > [Keyon] understood. Then we should follow the HD-audio style, So, what do 
> > you suggest here? Should we create only one single Boolean kctls for headset,
> > and report true when status in headphone bit it true? Then we need a tricky
> > exception mapping here?
> > 
> > Sorry, I am a little confusing here, because Mark suggest to create multiple controls
> > for multiple bits jack, and you also agreed with that.  :(
> 
> Just prepare two exceptions for SND_JACK_HEADSET and
> SND_JACK_VIDEOUT.  These are already defined as single types in
> sound/jack.h.  The code can't be so tricky if you write properly :)

Can someone clarify what is the current plan? Will there be changes to
the control naming or behaviour for existing hardware?

PulseAudio's jack handling seems somewhat messy to me, so it may be
difficult to understand what kind of changes will break things and what
won't. I think these are the jack controls that PulseAudio currently
recognizes that are most important in this discussion:

 * Headset Mic Jack
 * Headphone Mic Jack
 * Mic Jack
 * Headphone Jack

"Headset Mic Jack" indicates the availability of a headset mic. In
PulseAudio it doesn't imply anything about the headset speaker
availability, even though logically, if there's a headset mic plugged
in, surely the headset speakers are available too.

"Headphone Mic Jack" indicates the availability of either headphones or
a microphone. There's no information about which of those is plugged in,
just that one of those devices is plugged in. This control is *not* for
headsets, according to the commit that added the support for that
control to PulseAudio[1].

"Mic Jack" indicates the availability of a standalone microphone. I
don't know if the kernel currently exposes both "Headset Mick Jack" and
"Mic Jack" if there's one physical jack that is able to distinguish
between the two device types, but I suppose it would be good to have
both controls in that case, so that applications know what kind of
device has been plugged in.

"Headphone Jack" indicates the availability of headphones. PulseAudio
doesn't currently have support for any kind of "Headset Speaker Jack"
control, so I guess the kernel currently uses "Headphone Jack" also with
physical jacks that support headsets.

One thing that is unclear for me is that how are those jacks represented
that support any of headsets/headphones/microphones, but don't provide
information about which device type has been plugged in. I know David
has made a UI for Ubuntu for selecting the device type once something
has been plugged in to such jack, but I don't remember how the UI can
know that the jack supports all of those three device types.

[1] http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=7369a53ab5f606e87a3cd1cd4eebd40226bab090

-- 
Tanu