- channel.h

  * New definition CHN_F_HAS_VCHAN.
- channel.c
  * Use CHN_F_HAS_VCHAN to mark channel with vchan capability instead
    of relying on SLIST_EMPTY(&channel->children) == true for better
    clarification and future possible usages of children (like
    'slave' channel).
  * Various fixes, including blocksize / format bps allignment,
    better 24bit seeking (mplayer, others).
  * Improve format chain building, it's now possible to record something
    to a format non-native to the soundcard through various feeder format
    converters or to higher sampling rate. This also gains another feature,
    like doing vchan mixing on non s16le soundcard such as sb8.
- sound.c
  * Increase robustness within various function that handle vchan
    creation / termination (these function need a total rewrite, but
    that would cause other major rewrite within various places too!).
    As far as its robustness can be guaranteed, leave it as is.
  * Optimize channel ordering, prefer *real* hardware playback
    channels over virtual channels. cat /dev/sndstat should look
    better.
  * Increase sndstat verbosity to include bufsoft/bufhard allocation.
- vchan.c
  * Fix LOR 119.
    - http://sources.zabbadoz.net/freebsd/lor.html#119
  * Reorder / increase robustness of vchan_create() / destroy().
    Enforce destroy_dev() during destroy operation, fix possible
    panic / dangling character device.
    - http://lists.freebsd.org/pipermail/freebsd-current/2005-May/050308.html
  * Tolerate a little bit more during mixing process, this should help
    non s16le soundcards.

Note: Recoring in a non-native rate/format may result in overruns. A friendly
      application is wavrec from audio/wavplay. The problem is under
      investigation.

Submitted by:	Ariff Abdullah <skywizard@MyBSD.org.my>

4 files changed, 350 insertions, 231 deletions

diff --git a/sys/dev/sound/pcm/channel.c b/sys/dev/sound/pcm/channel.c
index fa56199..eb5a89e 100644
--- a/sys/dev/sound/pcm/channel.c
+++ b/sys/dev/sound/pcm/channel.c
@@ -107,7 +107,9 @@ chn_polltrigger(struct pcm_channel *c)
 			return (sndbuf_getblocks(bs) > sndbuf_getprevblocks(bs))? 1 : 0;
 	} else {
 		amt = (c->direction == PCMDIR_PLAY)? sndbuf_getfree(bs) : sndbuf_getready(bs);
+#if 0
 		lim = (c->flags & CHN_F_HAS_SIZE)? sndbuf_getblksz(bs) : 1;
+#endif
 		lim = 1;
 		return (amt >= lim)? 1 : 0;
 	}
@@ -227,11 +229,13 @@ chn_wrfeed(struct pcm_channel *c)
 	unsigned int ret, amt;
 
 	CHN_LOCKASSERT(c);
-/*    	DEB(
+#if 0
+    	DEB(
 	if (c->flags & CHN_F_CLOSING) {
 		sndbuf_dump(b, "b", 0x02);
 		sndbuf_dump(bs, "bs", 0x02);
-	}) */
+	})
+#endif
 
 	if (c->flags & CHN_F_MAPPED)
 		sndbuf_acquire(bs, NULL, sndbuf_getfree(bs));
@@ -379,6 +383,11 @@ chn_rddump(struct pcm_channel *c, unsigned int cnt)
     	struct snd_dbuf *b = c->bufhard;
 
 	CHN_LOCKASSERT(c);
+#if 0
+	static uint32_t kk = 0;
+	printf("%u: dumping %d bytes\n", ++kk, cnt);
+#endif
+	c->xruns++;
 	sndbuf_setxrun(b, sndbuf_getxrun(b) + cnt);
 	return sndbuf_dispose(b, NULL, cnt);
 }
@@ -401,11 +410,16 @@ chn_rdfeed(struct pcm_channel *c)
 		sndbuf_dump(bs, "bs", 0x02);
 	})
 
+#if 0
 	amt = sndbuf_getready(b);
 	if (sndbuf_getfree(bs) < amt) {
 		c->xruns++;
 		amt = sndbuf_getfree(bs);
 	}
+#endif
+	amt = sndbuf_getfree(bs);
+	if (amt < sndbuf_getready(b))
+		c->xruns++;
 	ret = (amt > 0)? sndbuf_feed(b, bs, c, c->feeder, amt) : 0;
 
 	amt = sndbuf_getready(b);
@@ -555,10 +569,12 @@ chn_start(struct pcm_channel *c, int force)
 		 * fed at the first irq.
 		 */
 		if (c->direction == PCMDIR_PLAY) {
+			/*
+			 * Reduce pops during playback startup.
+			 */
+			sndbuf_fillsilence(b);
 			if (SLIST_EMPTY(&c->children))
 				chn_wrfeed(c);
-			else
-				sndbuf_fillsilence(b);
 		}
 		sndbuf_setrun(b, 1);
 		c->xruns = 0;
@@ -755,11 +771,15 @@ chn_reset(struct pcm_channel *c, u_int32_t fmt)
 
 	r = CHANNEL_RESET(c->methods, c->devinfo);
 	if (fmt != 0) {
+#if 0
 		hwspd = DSP_DEFAULT_SPEED;
 		/* only do this on a record channel until feederbuilder works */
 		if (c->direction == PCMDIR_REC)
 			RANGE(hwspd, chn_getcaps(c)->minspeed, chn_getcaps(c)->maxspeed);
 		c->speed = hwspd;
+#endif
+		hwspd = chn_getcaps(c)->minspeed;
+		c->speed = hwspd;
 
 		if (r == 0)
 			r = chn_setformat(c, fmt);
@@ -955,13 +975,8 @@ chn_tryspeed(struct pcm_channel *c, int speed)
 		if (r)
 			goto out;
 
-		if (!(c->feederflags & (1 << FEEDER_RATE))) {
-			if (c->direction == PCMDIR_PLAY &&
-					!(c->flags & CHN_F_VIRTUAL))
-				r = CHANNEL_SETFORMAT(c->methods, c->devinfo,
-							c->feeder->desc->out);
+		if (!(c->feederflags & (1 << FEEDER_RATE)))
 			goto out;
-		}
 
 		r = EINVAL;
 		f = chn_findfeeder(c, FEEDER_RATE);
@@ -978,18 +993,12 @@ chn_tryspeed(struct pcm_channel *c, int speed)
 		x = (c->direction == PCMDIR_REC)? bs : b;
 		r = FEEDER_SET(f, FEEDRATE_DST, sndbuf_getspd(x));
 		DEB(printf("feeder_set(FEEDRATE_DST, %d) = %d\n", sndbuf_getspd(x), r));
-		if (c->direction == PCMDIR_PLAY && !(c->flags & CHN_F_VIRTUAL)
-				&& !((c->format & AFMT_S16_LE) &&
-						(c->format & AFMT_STEREO))) {
-			uint32_t fmt;
-
-			fmt = chn_fmtchain(c, chn_getcaps(c)->fmtlist);
-			if (fmt != 0)
-				r = CHANNEL_SETFORMAT(c->methods, c->devinfo, fmt);
-			else
-				r = EINVAL;
-		}
 out:
+		if (!r)
+			r = CHANNEL_SETFORMAT(c->methods, c->devinfo,
+							sndbuf_getfmt(b));
+		if (!r)
+			sndbuf_setfmt(bs, c->format);
 		DEB(printf("setspeed done, r = %d\n", r));
 		return r;
 	} else
@@ -1095,6 +1104,10 @@ chn_setblocksize(struct pcm_channel *c, int blkcnt, int blksz)
 	}
 
 	reqblksz = blksz;
+	if (reqblksz < sndbuf_getbps(bs))
+		reqblksz = sndbuf_getbps(bs);
+	if (reqblksz % sndbuf_getbps(bs))
+		reqblksz -= reqblksz % sndbuf_getbps(bs);
 
 	/* adjust for different hw format/speed */
 	irqhz = (sndbuf_getbps(bs) * sndbuf_getspd(bs)) / blksz;
@@ -1158,6 +1171,24 @@ out1:
 	    blksz, maxsize, blkcnt));
 out:
 	c->flags &= ~CHN_F_SETBLOCKSIZE;
+#if 0
+	if (1) {
+		static uint32_t kk = 0;
+		printf("%u: b %d/%d/%d : (%d)%d/0x%0x | bs %d/%d/%d : (%d)%d/0x%0x\n", ++kk,
+			sndbuf_getsize(b), sndbuf_getblksz(b), sndbuf_getblkcnt(b),
+			sndbuf_getbps(b),
+			sndbuf_getspd(b), sndbuf_getfmt(b),
+			sndbuf_getsize(bs), sndbuf_getblksz(bs), sndbuf_getblkcnt(bs),
+			sndbuf_getbps(bs),
+			sndbuf_getspd(bs), sndbuf_getfmt(bs));
+		if (sndbuf_getsize(b) % sndbuf_getbps(b) ||
+				sndbuf_getblksz(b) % sndbuf_getbps(b) ||
+				sndbuf_getsize(bs) % sndbuf_getbps(bs) ||
+				sndbuf_getblksz(b) % sndbuf_getbps(b)) {
+			printf("%u: bps/blksz alignment screwed!\n", kk);
+		}
+	}
+#endif
 	return ret;
 }
 
@@ -1228,7 +1259,7 @@ chn_buildfeeder(struct pcm_channel *c)
 {
 	struct feeder_class *fc;
 	struct pcm_feederdesc desc;
-	u_int32_t tmp[2], type, flags, hwfmt;
+	u_int32_t tmp[2], type, flags, hwfmt, *fmtlist;
 	int err;
 
 	CHN_LOCKASSERT(c);
@@ -1249,8 +1280,13 @@ chn_buildfeeder(struct pcm_channel *c)
 		}
 		c->feeder->desc->out = c->format;
 	} else {
-		desc.type = FEEDER_MIXER;
-		desc.in = 0;
+		if (c->flags & CHN_F_HAS_VCHAN) {
+			desc.type = FEEDER_MIXER;
+			desc.in = 0;
+		} else {
+			DEB(printf("can't decide which feeder type to use!\n"));
+			return EOPNOTSUPP;
+		}
 		desc.out = c->format;
 		desc.flags = 0;
 		fc = feeder_getclass(&desc);
@@ -1268,6 +1304,7 @@ chn_buildfeeder(struct pcm_channel *c)
 		}
 	}
 	flags = c->feederflags;
+	fmtlist = chn_getcaps(c)->fmtlist;
 
 	DEB(printf("feederflags %x\n", flags));
 
@@ -1286,7 +1323,9 @@ chn_buildfeeder(struct pcm_channel *c)
 				return EOPNOTSUPP;
 			}
 
-			if (c->feeder->desc->out != fc->desc->in) {
+			if ((type == FEEDER_RATE &&
+					!fmtvalid(fc->desc->in, fmtlist))
+					|| c->feeder->desc->out != fc->desc->in) {
  				DEB(printf("build fmtchain from 0x%x to 0x%x: ", c->feeder->desc->out, fc->desc->in));
 				tmp[0] = fc->desc->in;
 				tmp[1] = 0;
@@ -1308,28 +1347,23 @@ chn_buildfeeder(struct pcm_channel *c)
 		}
 	}
 
-	if (fmtvalid(c->feeder->desc->out, chn_getcaps(c)->fmtlist)) {
+	if (fmtvalid(c->feeder->desc->out, fmtlist)
+			&& !(c->direction == PCMDIR_REC &&
+				c->format != c->feeder->desc->out))
 		hwfmt = c->feeder->desc->out;
-	} else {
+	else {
 		if (c->direction == PCMDIR_REC) {
 			tmp[0] = c->format;
 			tmp[1] = 0;
 			hwfmt = chn_fmtchain(c, tmp);
-		} else {
-#if 0
-			u_int32_t *x = chn_getcaps(c)->fmtlist;
-			printf("acceptable formats for %s:\n", c->name);
-			while (*x) {
-				printf("[0x%8x] ", *x);
-				x++;
-			}
-#endif
-			hwfmt = chn_fmtchain(c, chn_getcaps(c)->fmtlist);
-		}
+		} else
+			hwfmt = chn_fmtchain(c, fmtlist);
 	}
 
-	if (hwfmt == 0)
+	if (hwfmt == 0 || !fmtvalid(hwfmt, fmtlist)) {
+		DEB(printf("Invalid hardware format: 0x%x\n", hwfmt));
 		return ENODEV;
+	}
 
 	sndbuf_setfmt(c->bufhard, hwfmt);
 
diff --git a/sys/dev/sound/pcm/channel.h b/sys/dev/sound/pcm/channel.h
index 23cd719..b854976 100644
--- a/sys/dev/sound/pcm/channel.h
+++ b/sys/dev/sound/pcm/channel.h
@@ -137,10 +137,13 @@ int fmtvalid(u_int32_t fmt, u_int32_t *fmtlist);
 #define CHN_F_DEAD		0x00020000
 #define CHN_F_BADSETTING	0x00040000
 #define CHN_F_SETBLOCKSIZE	0x00080000
+#define CHN_F_HAS_VCHAN		0x00100000
 
 #define	CHN_F_VIRTUAL		0x10000000  /* not backed by hardware */
 
-#define CHN_F_RESET		(CHN_F_BUSY | CHN_F_DEAD | CHN_F_VIRTUAL)
+#define CHN_F_RESET		(CHN_F_BUSY | CHN_F_DEAD | \
+					CHN_F_HAS_VCHAN | CHN_F_VIRTUAL)
+					
 
 #define CHN_N_RATE		0x00000001
 #define CHN_N_FORMAT		0x00000002
diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c
index 6f823ed..dbeda1d 100644
--- a/sys/dev/sound/pcm/sound.c
+++ b/sys/dev/sound/pcm/sound.c
@@ -189,7 +189,8 @@ pcm_chnalloc(struct snddev_info *d, int direction, pid_t pid, int chnum)
 			SLIST_FOREACH(sce, &d->channels, link) {
 				c = sce->channel;
 				CHN_LOCK(c);
-				if (!SLIST_EMPTY(&c->children)) {
+				if ((c->flags & CHN_F_HAS_VCHAN) &&
+						!SLIST_EMPTY(&c->children)) {
 					err = vchan_create(c);
 					CHN_UNLOCK(c);
 					if (!err)
@@ -246,45 +247,64 @@ pcm_inprog(struct snddev_info *d, int delta)
 static void
 pcm_setmaxautovchans(struct snddev_info *d, int num)
 {
-	struct pcm_channel *c;
+	struct pcm_channel *c, *ch;
     	struct snddev_channel *sce;
 	int err, done;
 
+	/*
+	 * XXX WOAH... NEED SUPER CLEANUP!!!
+	 * Robust, yet confusing. Understanding these will
+	 * cause your brain spinning like a Doki Doki Dynamo.
+	 */
 	if (num > 0 && d->vchancount == 0) {
 		SLIST_FOREACH(sce, &d->channels, link) {
 			c = sce->channel;
 			CHN_LOCK(c);
-			if ((c->direction == PCMDIR_PLAY) && !(c->flags & CHN_F_BUSY)) {
+			if ((c->direction == PCMDIR_PLAY) &&
+					!(c->flags & CHN_F_BUSY) &&
+					SLIST_EMPTY(&c->children)) {
 				c->flags |= CHN_F_BUSY;
 				err = vchan_create(c);
 				if (err) {
 					c->flags &= ~CHN_F_BUSY;
-					CHN_UNLOCK(c);
 					device_printf(d->dev, "vchan_create(%s) == %d\n", c->name, err);
-				} else
-					CHN_UNLOCK(c);
+				}
+				CHN_UNLOCK(c);
 				return;
 			}
 			CHN_UNLOCK(c);
 		}
+		return;
 	}
 	if (num == 0 && d->vchancount > 0) {
-		done = 0;
-		while (!done) {
-			done = 1;
+		/*
+		 * XXX Keep retrying...
+		 */
+		for (done = 0; done < 1024; done++) {
+			ch = NULL;
 			SLIST_FOREACH(sce, &d->channels, link) {
 				c = sce->channel;
-				if ((c->flags & CHN_F_VIRTUAL) && !(c->flags & CHN_F_BUSY)) {
-					done = 0;
-					snd_mtxlock(d->lock);
-					err = vchan_destroy(c);
-					snd_mtxunlock(d->lock);
-					if (err)
-						device_printf(d->dev, "vchan_destroy(%s) == %d\n", c->name, err);
-					break;		/* restart */
+				CHN_LOCK(c);
+				if (c->direction == PCMDIR_PLAY &&
+						!(c->flags & CHN_F_BUSY) &&
+						(c->flags & CHN_F_VIRTUAL)) {
+					ch = c;
+					break;
 				}
+				CHN_UNLOCK(c);
 			}
+			if (ch != NULL) {
+				CHN_UNLOCK(ch);
+				snd_mtxlock(d->lock);
+				err = vchan_destroy(ch);
+				if (err)
+					device_printf(d->dev, "vchan_destroy(%s) == %d\n",
+								ch->name, err);
+				snd_mtxunlock(d->lock);
+			} else
+				return;
 		}
+		return;
 	}
 }
 
@@ -327,7 +347,11 @@ sysctl_hw_snd_maxautovchans(SYSCTL_HANDLER_ARGS)
 				d = devclass_get_softc(pcm_devclass, i);
 				if (!d)
 					continue;
-				pcm_setmaxautovchans(d, v);
+				if (d->flags & SD_F_AUTOVCHAN) {
+					if (pcm_inprog(d, 1) == 1)
+						pcm_setmaxautovchans(d, v);
+					pcm_inprog(d, -1);
+				}
 			}
 		}
 		snd_maxautovchans = v;
@@ -449,11 +473,37 @@ pcm_chn_add(struct snddev_info *d, struct pcm_channel *ch)
 	if (SLIST_EMPTY(&d->channels)) {
 		SLIST_INSERT_HEAD(&d->channels, sce, link);
 	} else {
+		/*
+		 * Micro optimization, channel ordering:
+		 * hw,hw,hw,vch,vch,vch,rec
+		 */
 		after = NULL;
-		SLIST_FOREACH(tmp, &d->channels, link) {
-			after = tmp;
+		if (ch->flags & CHN_F_VIRTUAL) {
+			/* virtual channel to the end */
+			SLIST_FOREACH(tmp, &d->channels, link) {
+				if (tmp->channel->direction == PCMDIR_REC)
+					break;
+				after = tmp;
+			}
+		} else {
+			if (ch->direction == PCMDIR_REC) {
+				SLIST_FOREACH(tmp, &d->channels, link) {
+					after = tmp;
+				}
+			} else {
+				SLIST_FOREACH(tmp, &d->channels, link) {
+					if (tmp->channel->direction == PCMDIR_REC)
+						break;
+					if (!(tmp->channel->flags & CHN_F_VIRTUAL))
+						after = tmp;
+				}
+			}
+		}
+		if (after == NULL) {
+			SLIST_INSERT_HEAD(&d->channels, sce, link);
+		} else {
+			SLIST_INSERT_AFTER(after, sce, link);
 		}
-		SLIST_INSERT_AFTER(after, sce, link);
 	}
 	snd_mtxunlock(d->lock);
 	sce->dsp_devt= make_dev(&dsp_cdevsw,
@@ -506,10 +556,10 @@ pcm_chn_remove(struct snddev_info *d, struct pcm_channel *ch)
 gotit:
 	SLIST_REMOVE(&d->channels, sce, snddev_channel, link);
 
-    	if (ch->direction == PCMDIR_REC)
-		d->reccount--;
-	else if (ch->flags & CHN_F_VIRTUAL)
+	if (ch->flags & CHN_F_VIRTUAL)
 		d->vchancount--;
+	else if (ch->direction == PCMDIR_REC)
+		d->reccount--;
 	else
 		d->playcount--;
 
@@ -691,10 +741,10 @@ pcm_register(device_t dev, void *devinfo, int numplay, int numrec)
 	SYSCTL_ADD_INT(snd_sysctl_tree(dev), SYSCTL_CHILDREN(snd_sysctl_tree_top(dev)),
             OID_AUTO, "buffersize", CTLFLAG_RD, &d->bufsz, 0, "");
 #endif
-	if (numplay > 0)
+	if (numplay > 0) {
 		vchan_initsys(dev);
-	if (numplay == 1)
 		d->flags |= SD_F_AUTOVCHAN;
+	}
 
 	sndstat_register(dev, d->status, sndstat_prepare_pcm);
     	return 0;
@@ -739,9 +789,12 @@ pcm_unregister(device_t dev)
 	}
 
 	SLIST_FOREACH(sce, &d->channels, link) {
-		destroy_dev(sce->dsp_devt);
-		destroy_dev(sce->dspW_devt);
-		destroy_dev(sce->audio_devt);
+		if (sce->dsp_devt)
+			destroy_dev(sce->dsp_devt);
+		if (sce->dspW_devt)
+			destroy_dev(sce->dspW_devt);
+		if (sce->audio_devt)
+			destroy_dev(sce->audio_devt);
 		if (sce->dspr_devt)
 			destroy_dev(sce->dspr_devt);
 	}
@@ -827,11 +880,19 @@ sndstat_prepare_pcm(struct sbuf *s, device_t dev, int verbose)
 			if (c->bufhard != NULL && c->bufsoft != NULL) {
 				sbuf_printf(s, "interrupts %d, ", c->interrupts);
 				if (c->direction == PCMDIR_REC)
-					sbuf_printf(s, "overruns %d, hfree %d, sfree %d",
-						c->xruns, sndbuf_getfree(c->bufhard), sndbuf_getfree(c->bufsoft));
+					sbuf_printf(s, "overruns %d, hfree %d, sfree %d [b:%d/%d/%d|bs:%d/%d/%d]",
+						c->xruns, sndbuf_getfree(c->bufhard), sndbuf_getfree(c->bufsoft),
+						sndbuf_getsize(c->bufhard), sndbuf_getblksz(c->bufhard),
+						sndbuf_getblkcnt(c->bufhard),
+						sndbuf_getsize(c->bufsoft), sndbuf_getblksz(c->bufsoft),
+						sndbuf_getblkcnt(c->bufsoft));
 				else
-					sbuf_printf(s, "underruns %d, ready %d",
-						c->xruns, sndbuf_getready(c->bufsoft));
+					sbuf_printf(s, "underruns %d, ready %d [b:%d/%d/%d|bs:%d/%d/%d]",
+						c->xruns, sndbuf_getready(c->bufsoft),
+						sndbuf_getsize(c->bufhard), sndbuf_getblksz(c->bufhard),
+						sndbuf_getblkcnt(c->bufhard),
+						sndbuf_getsize(c->bufsoft), sndbuf_getblksz(c->bufsoft),
+						sndbuf_getblkcnt(c->bufsoft));
 				sbuf_printf(s, "\n\t");
 			}
 
@@ -869,26 +930,31 @@ sysctl_hw_snd_vchans(SYSCTL_HANDLER_ARGS)
 	struct snddev_info *d;
     	struct snddev_channel *sce;
 	struct pcm_channel *c;
-	int err, newcnt, cnt, busy;
-	int x;
+	int err, newcnt, cnt;
 
+	/*
+	 * XXX WOAH... NEED SUPER CLEANUP!!!
+	 * Robust, yet confusing. Understanding these will
+	 * cause your brain spinning like a Doki Doki Dynamo.
+	 */
 	d = oidp->oid_arg1;
 
-	x = pcm_inprog(d, 1);
-	if (x != 1) {
+	if (!(d->flags & SD_F_AUTOVCHAN)) {
 		pcm_inprog(d, -1);
-		return EINPROGRESS;
+		return EINVAL;
 	}
 
-	busy = 0;
 	cnt = 0;
 	SLIST_FOREACH(sce, &d->channels, link) {
 		c = sce->channel;
 		CHN_LOCK(c);
 		if ((c->direction == PCMDIR_PLAY) && (c->flags & CHN_F_VIRTUAL)) {
 			cnt++;
-			if (c->flags & CHN_F_BUSY)
-				busy++;
+			if (req->newptr != NULL && c->flags & CHN_F_BUSY) {
+				/* Better safe than sorry */
+				CHN_UNLOCK(c);
+				return EBUSY;
+			}
 		}
 		CHN_UNLOCK(c);
 	}
@@ -899,9 +965,12 @@ sysctl_hw_snd_vchans(SYSCTL_HANDLER_ARGS)
 
 	if (err == 0 && req->newptr != NULL) {
 
-		if (newcnt < 0 || newcnt > SND_MAXVCHANS) {
-			pcm_inprog(d, -1);
+		if (newcnt < 0 || newcnt > SND_MAXVCHANS)
 			return E2BIG;
+
+		if (pcm_inprog(d, 1) != 1) {
+			pcm_inprog(d, -1);
+			return EINPROGRESS;
 		}
 
 		if (newcnt > cnt) {
@@ -940,22 +1009,15 @@ addok:
 				if (err == 0)
 					cnt++;
 			}
-			if (SLIST_EMPTY(&c->children))
-				c->flags &= ~CHN_F_BUSY;
 			CHN_UNLOCK(c);
 		} else if (newcnt < cnt) {
-			if (busy > newcnt) {
-				printf("cnt %d, newcnt %d, busy %d\n", cnt, newcnt, busy);
-				pcm_inprog(d, -1);
-				return EBUSY;
-			}
-
 			snd_mtxlock(d->lock);
 			while (err == 0 && newcnt < cnt) {
 				SLIST_FOREACH(sce, &d->channels, link) {
 					c = sce->channel;
 					CHN_LOCK(c);
-					if ((c->flags & (CHN_F_BUSY | CHN_F_VIRTUAL)) == CHN_F_VIRTUAL)
+					if (c->direction == PCMDIR_PLAY &&
+							(c->flags & (CHN_F_BUSY | CHN_F_VIRTUAL)) == CHN_F_VIRTUAL)
 						goto remok;
 
 					CHN_UNLOCK(c);
@@ -971,8 +1033,8 @@ remok:
 			}
 			snd_mtxunlock(d->lock);
 		}
+		pcm_inprog(d, -1);
 	}
-	pcm_inprog(d, -1);
 	return err;
 }
 #endif
diff --git a/sys/dev/sound/pcm/vchan.c b/sys/dev/sound/pcm/vchan.c
index dfc0705..5c6addc 100644
--- a/sys/dev/sound/pcm/vchan.c
+++ b/sys/dev/sound/pcm/vchan.c
@@ -82,13 +82,21 @@ feed_vchan_s16(struct pcm_feeder *f, struct pcm_channel *c, u_int8_t *b, u_int32
 	struct snd_dbuf *src = source;
 	struct pcmchan_children *cce;
 	struct pcm_channel *ch;
+	uint32_t sz;
 	int16_t *tmp, *dst;
-	unsigned int cnt;
+	unsigned int cnt, rcnt = 0;
 
+	#if 0
 	if (sndbuf_getsize(src) < count)
 		panic("feed_vchan_s16(%s): tmp buffer size %d < count %d, flags = 0x%x",
 		    c->name, sndbuf_getsize(src), count, c->flags);
+	#endif
+	sz = sndbuf_getsize(src);
+	if (sz < count)
+		count = sz;
 	count &= ~1;
+	if (count < 2)
+		return 0;
 	bzero(b, count);
 
 	/*
@@ -107,12 +115,14 @@ feed_vchan_s16(struct pcm_feeder *f, struct pcm_channel *c, u_int8_t *b, u_int32
 			if (ch->flags & CHN_F_MAPPED)
 				sndbuf_acquire(ch->bufsoft, NULL, sndbuf_getfree(ch->bufsoft));
 			cnt = FEEDER_FEED(ch->feeder, ch, (u_int8_t *)tmp, count, ch->bufsoft);
-			vchan_mix_s16(dst, tmp, cnt / 2);
+			vchan_mix_s16(dst, tmp, cnt >> 1);
+			if (cnt > rcnt)
+				rcnt = cnt;
 		}
    		CHN_UNLOCK(ch);
 	}
 
-	return count;
+	return rcnt & ~1;
 }
 
 static struct pcm_feederdesc feeder_vchan_s16_desc[] = {
@@ -135,6 +145,8 @@ vchan_init(kobj_t obj, void *devinfo, struct snd_dbuf *b, struct pcm_channel *c,
 
 	KASSERT(dir == PCMDIR_PLAY, ("vchan_init: bad direction"));
 	ch = malloc(sizeof(*ch), M_DEVBUF, M_WAITOK | M_ZERO);
+	if (!ch)
+		return NULL;
 	ch->parent = parent;
 	ch->channel = c;
 	ch->fmt = AFMT_U8;
@@ -171,6 +183,7 @@ vchan_setformat(kobj_t obj, void *data, u_int32_t format)
    	CHN_UNLOCK(channel);
 	chn_notify(parent, CHN_N_FORMAT);
    	CHN_LOCK(channel);
+	sndbuf_setfmt(channel->bufsoft, format);
 	return 0;
 }
 
@@ -192,12 +205,14 @@ static int
 vchan_setblocksize(kobj_t obj, void *data, u_int32_t blocksize)
 {
 	struct vchinfo *ch = data;
+	struct pcm_channel *channel = ch->channel;
 	struct pcm_channel *parent = ch->parent;
 	/* struct pcm_channel *channel = ch->channel; */
 	int prate, crate;
 
 	ch->blksz = blocksize;
    	/* CHN_UNLOCK(channel); */
+	sndbuf_setblksz(channel->bufhard, blocksize);
 	chn_notify(parent, CHN_N_BLOCKSIZE);
    	CHN_LOCK(parent);
    	/* CHN_LOCK(channel); */
@@ -264,23 +279,14 @@ sysctl_hw_snd_vchanrate(SYSCTL_HANDLER_ARGS)
 {
 	struct snddev_info *d;
     	struct snddev_channel *sce;
-	struct pcm_channel *c, *fake;
+	struct pcm_channel *c, *ch = NULL, *fake;
 	struct pcmchan_caps *caps;
 	int err = 0;
-	int errcnt = 0;
-	int found = 0;
 	int newspd = 0;
-	int success = 0;
 
 	d = oidp->oid_arg1;
-	if (pcm_inprog(d, 1) != 1) {
-		pcm_inprog(d, -1);
-		return EINPROGRESS;
-	}
-	if (d->vchancount < 1) {
-		pcm_inprog(d, -1);
+	if (!(d->flags & SD_F_AUTOVCHAN) || d->vchancount < 1)
 		return EINVAL;
-	}
 	SLIST_FOREACH(sce, &d->channels, link) {
 		c = sce->channel;
 		CHN_LOCK(c);
@@ -289,75 +295,52 @@ sysctl_hw_snd_vchanrate(SYSCTL_HANDLER_ARGS)
 				if (req->newptr != NULL &&
 						(c->flags & CHN_F_BUSY)) {
 					CHN_UNLOCK(c);
-					pcm_inprog(d, -1);
 					return EBUSY;
 				}
-			} else if (!SLIST_EMPTY(&c->children)) {
-				if (found++ == 0)
-					newspd = c->speed;
+				if (ch == NULL)
+					ch = c->parentchannel;
 			}
 		}
 		CHN_UNLOCK(c);
 	}
-	if (found < 1) {
-		pcm_inprog(d, -1);
-		return EINVAL;
+	if (ch != NULL) {
+		CHN_LOCK(ch);
+		newspd = ch->speed;
+		CHN_UNLOCK(ch);
 	}
 	err = sysctl_handle_int(oidp, &newspd, sizeof(newspd), req);
 	if (err == 0 && req->newptr != NULL) {
-		if (newspd < 1) {
+		if (ch == NULL || newspd < 1 ||
+				newspd < feeder_rate_ratemin ||
+				newspd > feeder_rate_ratemax)
+			return EINVAL;
+		if (pcm_inprog(d, 1) != 1) {
+			pcm_inprog(d, -1);
+			return EINPROGRESS;
+		}
+		CHN_LOCK(ch);
+		caps = chn_getcaps(ch);
+		if (caps == NULL || newspd < caps->minspeed ||
+				newspd > caps->maxspeed) {
+			CHN_UNLOCK(ch);
 			pcm_inprog(d, -1);
 			return EINVAL;
 		}
-		SLIST_FOREACH(sce, &d->channels, link) {
-			c = sce->channel;
-			CHN_LOCK(c);
-			if (c->direction == PCMDIR_PLAY) {
-				if (c->flags & CHN_F_VIRTUAL) {
-					if (c->flags & CHN_F_BUSY) {
-						CHN_UNLOCK(c);
-						pcm_inprog(d, -1);
-						return EBUSY;
-					}
-				} else if (!SLIST_EMPTY(&c->children)) {
-					caps = chn_getcaps(c);
-					if (caps != NULL) {
-						if (newspd < caps->minspeed ||
-								newspd > caps->maxspeed ||
-								newspd < feeder_rate_ratemin ||
-								newspd > feeder_rate_ratemax) {
-							errcnt++;
-						} else {
-							if (newspd != c->speed) {
-								err = chn_setspeed(c, newspd);
-								if (err != 0)
-									errcnt++;
-								else
-									success++;
-							} else
-								success++;
-						}
-					} else
-						errcnt++;
+		if (newspd != ch->speed) {
+			err = chn_setspeed(ch, newspd);
+			CHN_UNLOCK(ch);
+			if (err == 0) {
+				fake = pcm_getfakechan(d);
+				if (fake != NULL) {
+					CHN_LOCK(fake);
+					fake->speed = newspd;
+					CHN_UNLOCK(fake);
 				}
 			}
-			CHN_UNLOCK(c);
-		}
-		/*
-		 * Save new value to fake channel.
-		 */
-		if (success > 0) {
-			fake = pcm_getfakechan(d);
-			if (fake != NULL) {
-				CHN_LOCK(fake);
-				fake->speed = newspd;
-				CHN_UNLOCK(fake);
-			}
-		}
+		} else
+			CHN_UNLOCK(ch);
+		pcm_inprog(d, -1);
 	}
-	pcm_inprog(d, -1);
-	if (errcnt > 0)
-		err = EINVAL;
 	return err;
 }
 #endif
@@ -369,10 +352,15 @@ vchan_create(struct pcm_channel *parent)
 {
     	struct snddev_info *d = parent->parentsnddev;
 	struct pcmchan_children *pce;
-	struct pcm_channel *child;
-	int err, first;
+	struct pcm_channel *child, *fake;
+	struct pcmchan_caps *parent_caps;
+	int err, first, speed = 0;
 
-   	CHN_UNLOCK(parent);
+	if (!(parent->flags & CHN_F_BUSY))
+		return EBUSY;
+
+
+	CHN_UNLOCK(parent);
 
 	pce = malloc(sizeof(*pce), M_DEVBUF, M_WAITOK | M_ZERO);
 	if (!pce) {
@@ -387,16 +375,7 @@ vchan_create(struct pcm_channel *parent)
    		CHN_LOCK(parent);
 		return ENODEV;
 	}
-
-   	CHN_LOCK(parent);
-	if (!(parent->flags & CHN_F_BUSY))
-		return EBUSY;
-
-	first = SLIST_EMPTY(&parent->children);
-	/* add us to our parent channel's children */
 	pce->channel = child;
-	SLIST_INSERT_HEAD(&parent->children, pce, link);
-	CHN_UNLOCK(parent);
 
 	/* add us to our grandparent's channel list */
 	/*
@@ -406,44 +385,55 @@ vchan_create(struct pcm_channel *parent)
 	if (err) {
 		pcm_chn_destroy(child);
 		free(pce, M_DEVBUF);
+		CHN_LOCK(parent);
+		return err;
 	}
 
    	CHN_LOCK(parent);
-	/* XXX gross ugly hack, murder death kill */
-	if (first && !err) {
-		struct pcm_channel *fake;
-		struct pcmchan_caps *parent_caps;
-		int speed = 0;
-
-		err = chn_reset(parent, AFMT_STEREO | AFMT_S16_LE);
-		if (err)
-			printf("chn_reset: %d\n", err);
-
-		fake = pcm_getfakechan(d);
-		if (fake != NULL) {
-			/*
-			 * Trying to avoid querying kernel hint, previous
-			 * value already saved here.
-			 */
-			CHN_LOCK(fake);
-			speed = fake->speed;
-			CHN_UNLOCK(fake);
-		}
-		/*
-		 * This is very sad. Few soundcards advertised as being
-		 * able to do (insanely) higher/lower speed, but in
-		 * reality, they simply can't. At least, we give user chance
-		 * to set sane value via kernel hints file or sysctl.
-		 */
-		if (speed < 1 && resource_int_value(device_get_name(parent->dev),
-				device_get_unit(parent->dev),
-				"vchanrate", &speed) != 0) {
-			speed = VCHAN_DEFAULT_SPEED;
-		}
+	/* add us to our parent channel's children */
+	first = SLIST_EMPTY(&parent->children);
+	SLIST_INSERT_HEAD(&parent->children, pce, link);
+	parent->flags |= CHN_F_HAS_VCHAN;
 
+	if (first) {
 		parent_caps = chn_getcaps(parent);
+		if (parent_caps == NULL)
+			err = EINVAL;
+
+		if (!err)
+			err = chn_reset(parent, AFMT_STEREO | AFMT_S16_LE);
+
+		if (!err) {
+			fake = pcm_getfakechan(d);
+			if (fake != NULL) {
+				/*
+				 * Avoid querying kernel hint, use saved value
+				 * from fake channel.
+				 */
+				CHN_UNLOCK(parent);
+				CHN_LOCK(fake);
+				speed = fake->speed;
+				CHN_UNLOCK(fake);
+				CHN_LOCK(parent);
+			}
+
+			/*
+			 * This is very sad. Few soundcards advertised as being
+			 * able to do (insanely) higher/lower speed, but in
+			 * reality, they simply can't. At least, we give user chance
+			 * to set sane value via kernel hints or sysctl.
+			 */
+			if (speed < 1) {
+				int r;
+				CHN_UNLOCK(parent);
+				r = resource_int_value(device_get_name(parent->dev),
+							device_get_unit(parent->dev),
+								"vchanrate", &speed);
+				CHN_LOCK(parent);
+				if (r != 0)
+					speed = VCHAN_DEFAULT_SPEED;
+			}
 
-		if (parent_caps != NULL) {
 			/*
 			 * Limit speed based on driver caps.
 			 * This is supposed to help fixed rate, non-VRA
@@ -453,35 +443,45 @@ vchan_create(struct pcm_channel *parent)
 				speed = parent_caps->minspeed;
 			if (speed > parent_caps->maxspeed)
 				speed = parent_caps->maxspeed;
-		}
 
-		/*
-		 * We still need to limit the speed between
-		 * feeder_rate_ratemin <-> feeder_rate_ratemax. This is
-		 * just an escape goat if all of the above failed
-		 * miserably.
-		 */
-		if (speed < feeder_rate_ratemin)
-			speed = feeder_rate_ratemin;
-		if (speed > feeder_rate_ratemax)
-			speed = feeder_rate_ratemax;
-
-		err = chn_setspeed(parent, speed);
-		if (err)
-			printf("chn_setspeed: %d\n", err);
-		else {
-			if (fake != NULL) {
+			/*
+			 * We still need to limit the speed between
+			 * feeder_rate_ratemin <-> feeder_rate_ratemax. This is
+			 * just an escape goat if all of the above failed
+			 * miserably.
+			 */
+			if (speed < feeder_rate_ratemin)
+				speed = feeder_rate_ratemin;
+			if (speed > feeder_rate_ratemax)
+				speed = feeder_rate_ratemax;
+
+			err = chn_setspeed(parent, speed);
+
+			if (!err && fake != NULL) {
 				/*
 				 * Save new value to fake channel.
 				 */
+				CHN_UNLOCK(parent);
 				CHN_LOCK(fake);
 				fake->speed = speed;
 				CHN_UNLOCK(fake);
+				CHN_LOCK(parent);
 			}
 		}
+		
+		if (err) {
+			SLIST_REMOVE(&parent->children, pce, pcmchan_children, link);
+			parent->flags &= ~CHN_F_HAS_VCHAN;
+			CHN_UNLOCK(parent);
+			free(pce, M_DEVBUF);
+			pcm_chn_remove(d, child);
+			pcm_chn_destroy(child);
+			CHN_LOCK(parent);
+			return err;
+		}
 	}
 
-	return err;
+	return 0;
 }
 
 int
@@ -490,6 +490,7 @@ vchan_destroy(struct pcm_channel *c)
 	struct pcm_channel *parent = c->parentchannel;
     	struct snddev_info *d = parent->parentsnddev;
 	struct pcmchan_children *pce;
+	struct snddev_channel *sce;
 	int err, last;
 
 	CHN_LOCK(parent);
@@ -510,23 +511,44 @@ vchan_destroy(struct pcm_channel *c)
 	CHN_UNLOCK(parent);
 	return EINVAL;
 gotch:
+	SLIST_FOREACH(sce, &d->channels, link) {
+		if (sce->channel == c) {
+			if (sce->dsp_devt)
+				destroy_dev(sce->dsp_devt);
+			if (sce->dspW_devt)
+				destroy_dev(sce->dspW_devt);
+			if (sce->audio_devt)
+				destroy_dev(sce->audio_devt);
+			if (sce->dspr_devt)
+				destroy_dev(sce->dspr_devt);
+			break;
+		}
+	}
 	SLIST_REMOVE(&parent->children, pce, pcmchan_children, link);
 	free(pce, M_DEVBUF);
 
 	last = SLIST_EMPTY(&parent->children);
-	if (last)
+	if (last) {
 		parent->flags &= ~CHN_F_BUSY;
+		parent->flags &= ~CHN_F_HAS_VCHAN;
+	}
 
 	/* remove us from our grandparent's channel list */
 	err = pcm_chn_remove(d, c);
-	if (err) {
-		CHN_UNLOCK(parent);
-		return err;
-	}
 
 	CHN_UNLOCK(parent);
 	/* destroy ourselves */
-	err = pcm_chn_destroy(c);
+	if (!err)
+		err = pcm_chn_destroy(c);
+
+#if 0
+	if (!err && last) {
+		CHN_LOCK(parent);
+		chn_reset(parent, chn_getcaps(parent)->fmtlist[0]);
+		chn_setspeed(parent, chn_getcaps(parent)->minspeed);
+		CHN_UNLOCK(parent);
+	}
+#endif
 
 	return err;
 }
@@ -548,5 +570,3 @@ vchan_initsys(device_t dev)
 
 	return 0;
 }
-
-