|
Neohapsis is currently accepting applications for employment. For more information, please visit our website www.neohapsis.com or email hr@neohapsis.com |
Re: audio(4): fix for SETINFO ioctl
From: Jacob Meuser (jakemsr
jakemsr.com)
Date: Tue May 02 2006 - 02:47:43 CDT
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
On Sun, Apr 30, 2006 at 06:54:29PM +0200, Alexandre Ratchov wrote:
> the attached patch makes the AUDIO_SETINFO ioctl handle the following
> parameters: play.gain, play.balance, record.gain and record.balance
> in a way consistent with other parameters.
>
> I'll try to explain:
>
> The SETINFO ioctl of the audio(4) driver sets the audio parameters
> (sample rate, encoding ...) but also some mixer controls like record
> source, play/record gain).
>
> The problem is that some mixer controls may not exist because the
> hardware doesn't support them, in this case the AUDIO_SETINFO ioctl()
> returns EINVAL and this is not consistent with the way other settings
> are handled.
the general problem is that audio(4) is currently not checking mixer
class labels. there's a long description of the problem here:
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/audio.c?rev=1.182&content-type=text/x-cvsweb-markup
the corresponding patch, adapted for OpenBSD, is below.
> Indeed for the other settings (monitor, sample rate, ...) the driver
> will set the parameter to a "reasonable" value instead of returning
> EINVAL. For instance, if there is no monitor then the monitor setting
> is ignored; if the sample rate is not supported then another rate is
> used, etc... Things work in this way, because the API doesn't allow
> an user application to query for the supported features (except for
> the properties and the encodings)
>
> To test the patch, for instance use audioctl(1) to set a random
> parameter to an _allowed_ value, example:
>
> audioctl play.rate=44100
>
> shouldn't fail (It always fails if (for instance) record.gain isn't
> supported by the hardware). IMO this only concerns uaudio(4) devices,
> because isa/pci devices seem to support a complete sb(4)-like set of
> controls.
it doesn't fail with this patch, but it's not possible to set the gain
either, on my machine at least. with the patch below, audioctl doesn't
fail and it's possible to set the gain. combining your patch with the
patch below also fails to set the gain.
can you see if this patch, without your patch, works for you?
--
<jakemsr
jakemsr.com>
Index: audio.c
===================================================================
RCS file: /cvs/src/sys/dev/audio.c,v
retrieving revision 1.49
diff -u -r1.49 audio.c
--- audio.c 12 Mar 2006 10:34:50 -0000 1.49
+++ audio.c 12 Mar 2006 10:47:24 -0000

-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 *, const 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);
/* The default audio mode: 8 kHz mono ulaw */

-232,7 +231,7 
void *hdlp = sa->hdl;
int error;
mixer_devinfo_t mi;
- int iclass, oclass;
+ int iclass, mclass, oclass;
printf("\n");

-289,38 +288,70 
audio_init_ringbuffer(&sc->sc_pr);
audio_calcwater(sc);
- iclass = oclass = -1;
+ iclass = mclass = oclass = -1;
sc->sc_inports.index = -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.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 (hwp->query_devinfo(hdlp, &mi) != 0)
+ break;
+ 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, output ports=0x%x\n",
sc->sc_inports.allports, sc->sc_outports.allports));

-388,69 +419,62 
}
int
-au_portof(sc, name)
+au_portof(sc, name, class)
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(sc, ports, mi, cls, name, mname, tbl)
+au_setup_ports(sc, ports, mi, tbl)
struct audio_softc *sc;
struct au_mixer_ports *ports;
mixer_devinfo_t *mi;
- int cls;
- char *name;
- char *mname;
- struct portname *tbl;
+ const 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;
- 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->index = mi->index;
+
+ if (mi->type == AUDIO_MIXER_ENUM) {
+ 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, mi->un.e.member[j].label.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, mi->un.s.member[j].label.name,
+ mi->mixer_class);
+ }
}
}

-2319,15 +2343,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;

-2401,16 +2432,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;

-2458,11 +2496,22 
u_int port;
{
mixer_ctrl_t ct;
- int i, error;
-
- if (port == 0 && ports->allports == 0)
- return 0; /* allow this special case */
+ int i, error, use_mixerout;
+ 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;

-2473,7 +2522,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;
}

-2509,9 +2563,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])
Index: audiovar.h
===================================================================
RCS file: /cvs/src/sys/dev/audiovar.h,v
retrieving revision 1.9
diff -u -r1.9 audiovar.h
--- audiovar.h 26 Aug 2002 16:20:04 -0000 1.9
+++ audiovar.h 12 Mar 2006 10:47:26 -0000

-74,14 +74,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 */
};
/*
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]