On Wed, Sep 10, 2014 at 07:31:17PM +0200, Alexandre Ratchov wrote:
> audioctl output is full of useless, misleading and/or unreliable
> fields. Let's keep the usable ones only. The plan is to remove them
> from the kernel as well.
>
> OK?
I've been asked in private to explain the reason I think these
fields are useless/misleading, see comments inlined below.
But the interesting (and difficult) question is why we have these
fields at all. Probably for SunOS compatibility, for shell-scripts
written on SunOS back in 1997 to work on OpenBSD?
> Index: audioctl.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/audioctl/audioctl.c,v
> retrieving revision 1.23
> diff -u -p -u -p -r1.23 audioctl.c
> --- audioctl.c 13 Nov 2013 18:50:05 -0000 1.23
> +++ audioctl.c 10 Sep 2014 12:15:06 -0000
> @@ -80,17 +80,10 @@ struct field {
> u_int oldval;
> } fields[] = {
> { "name", &adev.name, STRING, READONLY },
> - { "version", &adev.version, STRING, READONLY },
version is not reliable as not set by all devices, and nobody knows
the version of what this is (driver version? chipset version? API
version?).
> - { "config", &adev.config, STRING, READONLY },
same here, certain devices return the driver/device name (ex.
azalia0), others parts of the chip name (already shown in dmesg).
Neither useful, nor reliable.
> { "encodings", encbuf, STRING, READONLY },
> { "properties", &properties, PROPS, READONLY },
> - { "full_duplex", &fullduplex, UINT, 0 },
> - { "fullduplex", &fullduplex, UINT, 0 },
since ~2009 we don't support non-full-duplex mode; i.e. devices
opened in read-write mode are necessarily used in full-duplex, so
the information is available in the "mode" field.
> - { "blocksize", &info.blocksize, UINT, 0 },
>
the block size of what? the play or record one? (hint: they are not
the same). And note that we already have a play.block_size and
record.block_size fields which are useful.
> { "hiwat", &info.hiwat, UINT, 0 },
> { "lowat", &info.lowat, UINT, 0 },
> - { "output_muted", &info.output_muted, UCHAR, 0 },
this is an alias for a mixerctl control (outputs.master.mute in
most cases). It may not exist on certain devices, so it's not
reliable.
> - { "monitor_gain", &info.monitor_gain, UINT, 0 },
same here, except that it works on very few devices.
> { "mode", &info.mode, P_R, READONLY },
> { "play.rate", &info.play.sample_rate, UINT, 0 },
> { "play.sample_rate", &info.play.sample_rate, UINT, ALIAS },
> @@ -99,19 +92,9 @@ struct field {
> { "play.bps", &info.play.bps, UINT, 0 },
> { "play.msb", &info.play.msb, UINT, 0 },
> { "play.encoding", &info.play.encoding, ENC, 0 },
> - { "play.gain", &info.play.gain, UINT, 0 },
> - { "play.balance", &info.play.balance, UCHAR, 0 },
> - { "play.port", &info.play.port, XINT, 0 },
> - { "play.avail_ports", &info.play.avail_ports, XINT, 0 },
ditto, mixerctl controls (when available)
> - { "play.seek", &info.play.seek, UINT, READONLY },
this is not used.
> { "play.samples", &info.play.samples, UINT, READONLY },
> - { "play.eof", &info.play.eof, UINT, READONLY },
not used
> { "play.pause", &info.play.pause, UCHAR, 0 },
> - { "play.error", &info.play.error, UCHAR, READONLY },
alias for "(play.errors > 0) ? 1 : 0"
> - { "play.waiting", &info.play.waiting, UCHAR, READONLY },
not used, always 0
> - { "play.open", &info.play.open, UCHAR, READONLY },
this is 1 if the "mode" field contains "play" and 0 otherwise; so
this is a redundant, let's drop it.
> { "play.active", &info.play.active, UCHAR, READONLY },
> - { "play.buffer_size", &info.play.buffer_size, UINT, 0 },
misleading. Always contains 65536 which is the size of DMA-able
memory block. This is not the audio ring buffer size as the name
would suggest (which is smaller and must be multiple of the frame
size).
> - { "record.gain", &info.record.gain, UINT, 0 },
> - { "record.balance", &info.record.balance, UCHAR, 0 },
> - { "record.port", &info.record.port, XINT, 0 },
> - { "record.avail_ports", &info.record.avail_ports,XINT, 0 },
> - { "record.seek", &info.record.seek, UINT, READONLY },
> - { "record.eof", &info.record.eof, UINT, READONLY },
> - { "record.error", &info.record.error, UCHAR, READONLY },
> - { "record.waiting", &info.record.waiting, UCHAR, READONLY },
> - { "record.open", &info.record.open, UCHAR, READONLY },
> - { "record.buffer_size", &info.record.buffer_size,UINT, 0 },
same as for the "play" fields
