Subject: Proposed changes to MI audio port mapping
To: None <tech-kern@netbsd.org>
From: Frederick Bruckman <fredb@immanent.net>
List: tech-kern
Date: 02/11/2003 13:08:43
While investigating a fix for PR's kern/10221 and kern/17159
(basically, audioctl -w play.gain doesn't work), I discovered
more than a few problems:
1) The play master was being chosen from the wrong class of mixer
control: AudioCmonitor, rather than AudioCoutputs. The master is never
"monitor.master", but rather, "outputs.master" on all devices
surveyed. The play selector is also wrong. "eap" is the only device
that seems to have an output selector, but it's "outputs.select", not
"monitor.outputs".
2) For the recording master, many of the devices don't have a
"record.record" mixercontrol, as documented in audio(9), but some have
"record.volume".
3) For the recording selector:
3a) au_portof() doesn't check the class, so it arbitrarily matches
the first port in the mixer list which matches mi.label.name. Trying
to alter the audio interface's "record.gain" when, say "cd" is
selected, alters "inputs.cd" -- which has no effect on recording on
any of the devices I investigated (esm, eso, ess) -- rather than the
intended "record.cd" (but see below).
3b) No hardware driver actually sets "mixer_class" to the index of the
mixer, as documented in audio(9). Rather, the values are all arbitrary
integers.
3c) When you do correct the above, and manage to select an input for
recording, it generally disables that input's gain control!
For (1) and (2), I propose to break out some of functionality of
au_check_ports() into the calling loop, so as to be flexible about
permitting variation. I would use a new function, au_setup_ports(),
to do part of what au_check_ports() used to do.
2) For the long term, I would judge both variations wrong, and
suggest "record.master", though "record.volume" and "record.record"
would only be deprecated (i.e. still work for a while).
3a) I would add a class argument to au_portof(), so that
au_setup_ports() can get the mixer of the required class.
3b) This is easy to fix: just use mi.mixer_class of the mixer of type
AUDIO_MIXER_CLASS to identify the class, rather than mi.index.
3c) This is the hardest part to fix. The eso "record" mixer, for
example, is actually two mixers: an input mixer (active when
"mixerout" is the selected device), and an output mixer
(record.record). Selecting a device *disables* the input mixerctl for
that device. To enable it, you have to select mixerout, which actually
mixes the various input selections. Many other devices have a
"mixerout" selection, inherited from the chip driver, but with no
corresponding inputs, so *it* *does* *nothing*. Other devices have
only the master (usually "record.volume").
To do the right thing for all cases above, I would add state to the
struct audio_softc (actually, to each of struct au_mixer_ports) to
remember the port selected. For the dual case, we actually select
"mixerout", but let "record.gain" adjust the selected ports mixerctl.
For the simpler case, "record.gain" adjusts the record master. There's
also a mechanism to clear the state, and go with the record master,
even with a dual mixer. [This if for users who notice that the audio
device behaves differently after certain operations. I don't expect
applications to need to do it.] The state is also cleared
automatically if mixerctl frobbing is detected. It sounds complicated
(it is!) but it should all be transparent to the application.
I would also update the audio(4) and audio(9) man pages, and
eventually switch all the hardware drivers to "record.master".
I've never done anything this ambitious before, so I would greatly
appreciate it if someone would review the following. Thanks!
Frederick
Index: audiovar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/audiovar.h,v
retrieving revision 1.27
diff -u -r1.27 audiovar.h
--- audiovar.h 2002/03/18 00:42:36 1.27
+++ audiovar.h 2003/02/02 20:50:05
@@ -113,14 +113,18 @@
#define AUDIO_N_PORTS 4
struct au_mixer_ports {
- int index;
- int master;
- int nports;
- u_char isenum;
- u_int allports;
- u_int aumask[AUDIO_N_PORTS];
- u_int misel [AUDIO_N_PORTS];
- u_int miport[AUDIO_N_PORTS];
+ int index; /* index of port-selector mixerctl */
+ int master; /* index of master mixerctl */
+ int nports; /* number of selectable ports */
+ u_char isenum; /* selector is enum type */
+ u_int allports; /* all aumasks or'd */
+ u_int aumask[AUDIO_N_PORTS]; /* exposed value of "ports" */
+ u_int misel [AUDIO_N_PORTS]; /* ord of port, for selector */
+ u_int miport[AUDIO_N_PORTS]; /* index of port's mixerctl */
+ u_char isdual; /* has working mixerout */
+ int mixerout; /* ord of mixerout, for dual case */
+ int cur_port; /* the port that gain actually controls when
+ mixerout is selected, for dual case */
};
#if NAURATECONV > 0
Index: audio.c
===================================================================
RCS file: /cvsroot/src/sys/dev/audio.c,v
retrieving revision 1.173
diff -u -r1.173 audio.c
--- audio.c 2003/01/31 02:15:57 1.173
+++ audio.c 2003/02/02 20:50:12
@@ -166,9 +166,8 @@
{ AudioNline, AUDIO_LINE_OUT },
{ 0 }
};
-void au_check_ports(struct audio_softc *, struct au_mixer_ports *,
- mixer_devinfo_t *, int, char *, char *,
- struct portname *);
+void au_setup_ports(struct audio_softc *, struct au_mixer_ports *,
+ mixer_devinfo_t *, struct portname *);
int au_set_gain(struct audio_softc *, struct au_mixer_ports *,
int, int);
void au_get_gain(struct audio_softc *, struct au_mixer_ports *,
@@ -180,7 +179,7 @@
int *, int *r);
int au_set_lr_value(struct audio_softc *, mixer_ctrl_t *,
int, int);
-int au_portof(struct audio_softc *, char *);
+int au_portof(struct audio_softc *, char *, int);
dev_type_open(audioopen);
dev_type_close(audioclose);
@@ -224,7 +223,7 @@
void *hdlp = sa->hdl;
int error;
mixer_devinfo_t mi;
- int iclass, oclass, props;
+ int iclass, mclass, oclass, props;
#ifdef DIAGNOSTIC
if (hwp == 0 ||
@@ -298,40 +297,72 @@
audio_calcwater(sc);
sc->sc_input_fragment_length = 0;
- iclass = oclass = -1;
+ iclass = mclass = oclass = -1;
sc->sc_inports.index = -1;
sc->sc_inports.master = -1;
sc->sc_inports.nports = 0;
sc->sc_inports.isenum = 0;
sc->sc_inports.allports = 0;
+ sc->sc_inports.isdual = 0;
+ sc->sc_inports.mixerout = -1;
+ sc->sc_inports.cur_port = -1;
sc->sc_outports.index = -1;
sc->sc_outports.master = -1;
sc->sc_outports.nports = 0;
sc->sc_outports.isenum = 0;
sc->sc_outports.allports = 0;
+ sc->sc_outports.isdual = 0;
+ sc->sc_outports.mixerout = -1;
+ sc->sc_outports.cur_port = -1;
sc->sc_monitor_port = -1;
for(mi.index = 0; ; mi.index++) {
if (hwp->query_devinfo(hdlp, &mi) != 0)
break;
- if (mi.type == AUDIO_MIXER_CLASS &&
- strcmp(mi.label.name, AudioCrecord) == 0)
- iclass = mi.index;
- if (mi.type == AUDIO_MIXER_CLASS &&
- strcmp(mi.label.name, AudioCmonitor) == 0)
- oclass = mi.index;
+ if (mi.type == AUDIO_MIXER_CLASS) {
+ if (strcmp(mi.label.name, AudioCrecord) == 0)
+ iclass = mi.mixer_class;
+ if (strcmp(mi.label.name, AudioCmonitor) == 0)
+ mclass = mi.mixer_class;
+ if (strcmp(mi.label.name, AudioCoutputs) == 0)
+ oclass = mi.mixer_class;
+ }
}
for(mi.index = 0; ; mi.index++) {
if (hwp->query_devinfo(hdlp, &mi) != 0)
break;
if (mi.type == AUDIO_MIXER_CLASS)
continue;
- au_check_ports(sc, &sc->sc_inports, &mi, iclass,
- AudioNsource, AudioNrecord, itable);
- au_check_ports(sc, &sc->sc_outports, &mi, oclass,
- AudioNoutput, AudioNmaster, otable);
- if (mi.mixer_class == oclass &&
- (strcmp(mi.label.name, AudioNmonitor) == 0))
- sc->sc_monitor_port = mi.index;
+ if (mi.mixer_class == iclass) {
+ if (strcmp(mi.label.name, AudioNmaster) == 0)
+ sc->sc_inports.master = mi.index;
+#if 1 /* Deprecated. Use AudioNmaster. */
+ if (strcmp(mi.label.name, AudioNrecord) == 0)
+ sc->sc_inports.master = mi.index;
+ if (strcmp(mi.label.name, AudioNvolume) == 0)
+ sc->sc_inports.master = mi.index;
+#endif
+ if (strcmp(mi.label.name, AudioNsource) == 0) {
+ if (mi.type == AUDIO_MIXER_ENUM) {
+ int i;
+ for(i = 0; i < mi.un.e.num_mem; i++)
+ if (strcmp(mi.un.e.member[i].label.name,
+ AudioNmixerout) == 0)
+ sc->sc_inports.mixerout =
+ mi.un.e.member[i].ord;
+ }
+ au_setup_ports(sc, &sc->sc_inports, &mi,
+ itable);
+ }
+ } else if (mi.mixer_class == mclass) {
+ if (strcmp(mi.label.name, AudioNmonitor) == 0)
+ sc->sc_monitor_port = mi.index;
+ } else if (mi.mixer_class == oclass) {
+ if (strcmp(mi.label.name, AudioNmaster) == 0)
+ sc->sc_outports.master = mi.index;
+ if (strcmp(mi.label.name, AudioNselect) == 0)
+ au_setup_ports(sc, &sc->sc_outports, &mi,
+ otable);
+ }
}
DPRINTF(("audio_attach: inputs ports=0x%x, input master=%d, "
"output ports=0x%x, output master=%d\n",
@@ -396,62 +427,53 @@
}
int
-au_portof(struct audio_softc *sc, char *name)
+au_portof(struct audio_softc *sc, char *name, int class)
{
mixer_devinfo_t mi;
for(mi.index = 0;
sc->hw_if->query_devinfo(sc->hw_hdl, &mi) == 0;
mi.index++)
- if (strcmp(mi.label.name, name) == 0)
+ if (mi.mixer_class == class && strcmp(mi.label.name, name) == 0)
return mi.index;
return -1;
}
void
-au_check_ports(struct audio_softc *sc, struct au_mixer_ports *ports,
- mixer_devinfo_t *mi, int cls, char *name, char *mname,
- struct portname *tbl)
+au_setup_ports(struct audio_softc *sc, struct au_mixer_ports *ports,
+ mixer_devinfo_t *mi, struct portname *tbl)
{
int i, j;
- if (mi->mixer_class != cls)
- return;
- if (strcmp(mi->label.name, mname) == 0) {
- ports->master = mi->index;
- return;
- }
- if (strcmp(mi->label.name, name) != 0)
- return;
+ ports->index = mi->index;
if (mi->type == AUDIO_MIXER_ENUM) {
- ports->index = mi->index;
- for(i = 0; tbl[i].name; i++) {
- for(j = 0; j < mi->un.e.num_mem; j++) {
- if (strcmp(mi->un.e.member[j].label.name,
- tbl[i].name) == 0) {
- ports->aumask[ports->nports] = tbl[i].mask;
- ports->misel [ports->nports] = mi->un.e.member[j].ord;
- ports->miport[ports->nports++] =
- au_portof(sc, mi->un.e.member[j].label.name);
- ports->allports |= tbl[i].mask;
- }
- }
- }
- ports->isenum = 1;
+ ports->isenum = 1;
+ for(i = 0; tbl[i].name; i++)
+ for(j = 0; j < mi->un.e.num_mem; j++)
+ if (strcmp(mi->un.e.member[j].label.name,
+ tbl[i].name) == 0) {
+ ports->allports |= tbl[i].mask;
+ ports->aumask[ports->nports] = tbl[i].mask;
+ ports->misel[ports->nports] =
+ mi->un.e.member[j].ord;
+ ports->miport[ports->nports] =
+ au_portof(sc, tbl[i].name, mi->mixer_class);
+ if (ports->mixerout != -1 &&
+ ports->miport[ports->nports++] != -1)
+ ports->isdual = 1;
+ }
} else if (mi->type == AUDIO_MIXER_SET) {
- ports->index = mi->index;
- for(i = 0; tbl[i].name; i++) {
- for(j = 0; j < mi->un.s.num_mem; j++) {
- if (strcmp(mi->un.s.member[j].label.name,
- tbl[i].name) == 0) {
- ports->aumask[ports->nports] = tbl[i].mask;
- ports->misel [ports->nports] = mi->un.s.member[j].mask;
- ports->miport[ports->nports++] =
- au_portof(sc, mi->un.s.member[j].label.name);
- ports->allports |= tbl[i].mask;
- }
- }
- }
+ for(i = 0; tbl[i].name; i++)
+ for(j = 0; j < mi->un.s.num_mem; j++)
+ if (strcmp(mi->un.s.member[j].label.name,
+ tbl[i].name) == 0) {
+ ports->allports |= tbl[i].mask;
+ ports->aumask[ports->nports] = tbl[i].mask;
+ ports->misel[ports->nports] =
+ mi->un.s.member[j].mask;
+ ports->miport[ports->nports++] =
+ au_portof(sc, tbl[i].name, mi->mixer_class);
+ }
}
}
@@ -2575,15 +2597,22 @@
error = sc->hw_if->get_port(sc->hw_hdl, &ct);
if (error)
return error;
- for(i = 0; i < ports->nports; i++) {
- if (ports->misel[i] == ct.un.ord) {
- ct.dev = ports->miport[i];
- if (ct.dev == -1 ||
- au_set_lr_value(sc, &ct, l, r))
- goto usemaster;
- else
- break;
- }
+ if (ports->isdual) {
+ if (ports->cur_port == -1)
+ ct.dev = ports->master;
+ else
+ ct.dev = ports->miport[ports->cur_port];
+ error = au_set_lr_value(sc, &ct, l, r);
+ } else {
+ for(i = 0; i < ports->nports; i++)
+ if (ports->misel[i] == ct.un.ord) {
+ ct.dev = ports->miport[i];
+ if (ct.dev == -1 ||
+ au_set_lr_value(sc, &ct, l, r))
+ goto usemaster;
+ else
+ break;
+ }
}
} else {
ct.type = AUDIO_MIXER_SET;
@@ -2651,16 +2680,23 @@
if (sc->hw_if->get_port(sc->hw_hdl, &ct))
goto bad;
ct.type = AUDIO_MIXER_VALUE;
- for(i = 0; i < ports->nports; i++) {
- if (ports->misel[i] == ct.un.ord) {
- ct.dev = ports->miport[i];
- if (ct.dev == -1 ||
- au_get_lr_value(sc, &ct,
- &lgain, &rgain))
- goto usemaster;
- else
- break;
- }
+ if (ports->isdual) {
+ if (ports->cur_port == -1)
+ ct.dev = ports->master;
+ else
+ ct.dev = ports->miport[ports->cur_port];
+ au_get_lr_value(sc, &ct, &lgain, &rgain);
+ } else {
+ for(i = 0; i < ports->nports; i++)
+ if (ports->misel[i] == ct.un.ord) {
+ ct.dev = ports->miport[i];
+ if (ct.dev == -1 ||
+ au_get_lr_value(sc, &ct,
+ &lgain, &rgain))
+ goto usemaster;
+ else
+ break;
+ }
}
} else {
ct.type = AUDIO_MIXER_SET;
@@ -2707,11 +2743,22 @@
au_set_port(struct audio_softc *sc, struct au_mixer_ports *ports, u_int port)
{
mixer_ctrl_t ct;
- int i, error;
+ int i, error, use_mixerout;
- if (port == 0 && ports->allports == 0)
- return 0; /* allow this special case */
-
+ use_mixerout = 1;
+ if (port == 0) {
+ if (ports->allports == 0)
+ return 0; /* Allow this special case. */
+ else if (ports->isdual) {
+ if (ports->cur_port == -1) {
+ return 0;
+ } else {
+ port = ports->aumask[ports->cur_port];
+ ports->cur_port = -1;
+ use_mixerout = 0;
+ }
+ }
+ }
if (ports->index == -1)
return EINVAL;
ct.dev = ports->index;
@@ -2722,7 +2769,12 @@
error = EINVAL;
for(i = 0; i < ports->nports; i++)
if (ports->aumask[i] == port) {
- ct.un.ord = ports->misel[i];
+ if (ports->isdual && use_mixerout) {
+ ct.un.ord = ports->mixerout;
+ ports->cur_port = i;
+ } else {
+ ct.un.ord = ports->misel[i];
+ }
error = sc->hw_if->set_port(sc->hw_hdl, &ct);
break;
}
@@ -2756,9 +2808,16 @@
return 0;
aumask = 0;
if (ports->isenum) {
- for(i = 0; i < ports->nports; i++)
- if (ct.un.ord == ports->misel[i])
- aumask = ports->aumask[i];
+ if (ports->isdual && ports->cur_port != -1) {
+ if (ports->mixerout == ct.un.ord)
+ aumask = ports->aumask[ports->cur_port];
+ else
+ ports->cur_port = -1;
+ }
+ if (aumask == 0)
+ for(i = 0; i < ports->nports; i++)
+ if (ports->misel[i] == ct.un.ord)
+ aumask = ports->aumask[i];
} else {
for(i = 0; i < ports->nports; i++)
if (ct.un.mask & ports->misel[i])