I have been using the sound subsystem on an ibm 701C at user level and
reading the code for a bit and have some bug fixes and suggestions for
improvements.  I have a patch file for changes against sbdsp.c and
audio.c included at the end of this mail.  I also fixed sox,
rplay/rplayd, radio/broadcast to work -- no big deal.  I'll probably
just ship the changes back to the maintainers.

Bugs first:
The sbdsp. code sbdsp_recalc_blocksize() is broken and stupid.  The
"straight rate code" says:
 	if (sc->sc_orate > 8000 || sc->sc_irate > 8000)
while the "converted rate code" says:
	if (sc->sc_otc > SB_8K || sc->sc_itc < SB_8K) All the tests
should be ">".  But the big problem is that this code should not be
there at all.  What it does is that once the sample rate goes > 8K, it
forces the blksize to 2048.  Well, the sound just sucks.  At 8k the
blksize is 160, and presumably for 16K, you want 320, etc, etc.  That
is the calculation done in audio is just fine and the results sound
fine.  Maybe some of the sbdsp_recalc_blocksize() test is still valid
-- checking to make sure the block is < 4k.  But I doubt the rest has
value.

So you ask.  Why hasn't anyone seen this problem -- should be real
obvious.  The next bug is in audio.c if you look at audiosetinfo(),
you'll see that if you change the channels or precision, the code
recalc's the block size.  But if you change the sampling rate it does
not -- bug.  So with regard to the previous bug, if you just change the
sample rate on /dev/audio the block is not recalc'd and all sounds well,
but if you change samples and precision, the the recalc code kicks in
and mungs the sound quality.  So if you put in the previous bug fix then
you can put in this bug fix and all will be well.  (PS.  If you are
using /dev/sound vs /dev/audio, what happens is that the first time thru
after changing sample the sound is fine [because it has not recalc'ed]
but on subsequent plays to /dev/sound it calc's the blocksize (at open)
base on the higher rate and the sound is awefull -- again all this is
w/o the first bug fix.

The next problem is more subtle and maybe a just misfeature of
/dev/sound.  What I discovered is that if you exit a program that
plays sound and start a program to record record, nothing appeares to
happen.  What goes wrong is that the sc_mode had been AUMODE_PLAY.
When you switch to AUMODE_RECORD on /dev/sound this new "mode" gets
or'ed into the sc_mode to give you both play|record -- which spells
big trouble.  And a whole bunch of code (cb_pause) is now active to
handle this "feature".  (With /dev/audio the sc_mode is never or'ed it
is set.)  This could be fixed by making /dev/sound behave like
/dev/audio on open.  Alternatively, I set it so that on device close
the old mode bit is cleared out.  Slightly different -- maybe better
maybe not.  But this issue needs some fix.

Next misfeature (maybe) If you switch between /dev/sound and
/dev/audio you get suprised.  When you go back to /dev/sound, all the
settings are back to the /dev/audio ones.  I would argue that iff
/dev/sound is supposed to retain its values across open/close it
should not be munged by some "independent" device, /dev/audio.  (You
should save audio state when you close /dev/sound restore when you
open.)

Mixer Bugsf: In sbdsb_attach speaker volume is inited to max/2 but the
gain array (the software copy) sets everything to max

Misfeature:
On the SBPro bass and treble are just two different values of
a single cut off filter.  So you can not set them indepently.
What's worse is that setting the input source which is in the
same byte; resets both values.  This should be fixed.

Now on to a discussion: I thought this stuff was really confusing in
the manual page.  I had to read audio.c, sbdsp.c and finally write
some code to figure out what was going on.  When you use it, you see
string names used vs #define numbers, you see that most
"audio_mixer_name_t" "msgid"'s are always 0, (so why bother having the
field), you see in the audio_mixer_value, the audio_mixer_name_t names
are always "volume" (so again why bother.)  But more importantly, this
elaborate distinction of values and sets and masks for
mixer_dev_info_t items is tedious for the SBPro but is broken with
regards to the SB16.  Here, input and output mixing is not an
enumeration but a bit mask.  Also treble/bass are not "enumerations"
but 4 bit fields.  So basically I would throw out the existing
mixer_devinfo and return array of key/value pairs.  Much simpler and
more general.  Now I confess that I have not looked at the Sun code to
see if this bizarre interface is inherited from them or is a bsd
thing.


Lastly, it might be nice to:
	add support for:
		a few sun calls that rplay/rplayd and other programs that use
		the Sun audio model use like:
			ioctl I_FLUSH FLUSHW
			ioctl I_SETSIG
			SIGPOLL
	add support for the AudioDrive ESx688.
		This is a SBPro superset.  It can do 16bit/44k/stereo.
		Linux has support.  I've asked Hunnu where he got the
		product info.
	add support for SB16 - The spec is in the SBK.
		SBK's are on the net (I have one) but I
		don't have a SB16.
	

--------------------------------------------------------
Index: src/sys11B/dev/audio.c
diff -c src/sys11B/dev/audio.c:1.1.1.4 src/sys11B/dev/audio.c:1.2
*** src/sys11B/dev/audio.c:1.1.1.4	Mon May 13 07:36:04 1996
--- src/sys11B/dev/audio.c	Thu Jun 20 15:50:23 1996
***************
*** 693,704 ****
  	}
  	
  	hw->close(sc->hw_hdl);
! 	
  	if (flags&FREAD)
  		sc->sc_open &= ~AUOPEN_READ;
  	if (flags&FWRITE)
  		sc->sc_open &= ~AUOPEN_WRITE;
- 
  	splx(s);
  	DPRINTF(("audio_close: done\n"));
  
--- 693,715 ----
  	}
  	
  	hw->close(sc->hw_hdl);
! #define	new	
! #ifdef	new
! 	if ((flags&(FREAD|FWRITE)) == (FREAD|FWRITE))
! 		/* You are much too clever!
! 		   We'll leave everything alone */;
! 	else if (flags&FREAD) {
! 		sc->sc_mode &= ~AUMODE_RECORD;
! 		sc->pr.cb_pause = 0;
! 	} else if (flags&FWRITE) {
! 		sc->sc_mode &= ~AUMODE_PLAY;
! 		sc->rr.cb_pause = 0;
! 	}
! #endif
  	if (flags&FREAD)
  		sc->sc_open &= ~AUOPEN_READ;
  	if (flags&FWRITE)
  		sc->sc_open &= ~AUOPEN_WRITE;
  	splx(s);
  	DPRINTF(("audio_close: done\n"));
  
***************
*** 1460,1465 ****
--- 1471,1483 ----
  		if (error)
  			return(error);
  		sc->sc_50ms = 50 * hw->get_out_sr(sc->hw_hdl) / 1000;
+ #ifdef	new
+ 		sc->sc_blksize = audio_blocksize = audio_calc_blksize(sc);
+ 		audio_alloc_auzero(sc, sc->sc_blksize);
+ 		bps = hw->get_precision(sc->hw_hdl) / NBBY;
+ 		sc->sc_smpl_in_blk = sc->sc_blksize / bps;
+ 		audio_initbufs(sc);
+ #endif
  	}
  	if (r->sample_rate != ~0) {
  		if (!cleared)
***************
*** 1469,1474 ****
--- 1487,1500 ----
  		if (error)
  			return(error);
  		sc->sc_50ms = 50 * hw->get_in_sr(sc->hw_hdl) / 1000;
+ #ifdef	new
+ 		sc->sc_blksize = audio_blocksize = audio_calc_blksize(sc);
+ 		audio_alloc_auzero(sc, sc->sc_blksize);
+ 		bps = hw->get_precision(sc->hw_hdl) / NBBY;
+ 		sc->sc_smpl_in_blk = sc->sc_blksize / bps;
+ 		audio_initbufs(sc);
+ #endif
+ #undef	new
  	}
  	if (p->encoding != ~0) {
  		if (!cleared)
Index: src/sys11B/dev/isa/sbdsp.c
diff -c src/sys11B/dev/isa/sbdsp.c:1.1.1.4 src/sys11B/dev/isa/sbdsp.c:1.2
*** src/sys11B/dev/isa/sbdsp.c:1.1.1.4	Mon May 13 07:36:25 1996
--- src/sys11B/dev/isa/sbdsp.c	Thu Jun 20 15:49:41 1996
***************
*** 644,649 ****
--- 644,657 ----
  
  	/* Higher speeds need bigger blocks to avoid popping and silence gaps. */
  	if (blk < NBPG/4 || blk > NBPG/2) {
+ #define	new
+ #ifdef	new
+ 	/*
+ 	 * The check below seems pretty braindead.  For 8k sample blk = 160
+ 	 * and is left alone.  For 16k which has blk = 320 [or for 8k+epsilon]
+ 	 * the code kicks in and yields blk = 2048 !!!!!!!!!!!
+ 	 */
+ #else
  		if (ISSB16CLASS(sc)) {
  			if (sc->sc_orate > 8000 || sc->sc_irate > 8000)
  				blk = NBPG/2;
***************
*** 651,656 ****
--- 659,666 ----
  			if (sc->sc_otc > SB_8K || sc->sc_itc < SB_8K)
  				blk = NBPG/2;
  		}
+ #endif
+ #undef	new
  	}
  	/* don't try to DMA too much at once, though. */
  	if (blk > NBPG)