Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 12 Nov 2003 04:14:46 -0500 (EST)
From:      Mathew Kanner <mat@hak.cnd.mcgill.ca>
To:        FreeBSD-gnats-submit@FreeBSD.org
Subject:   kern/59208: sound patch to reduce pops and crackles and fix select on vchans
Message-ID:  <200311120914.hAC9Ek73036355@hak.cnd.mcgill.ca>
Resent-Message-ID: <200311120920.hAC9KFoJ066278@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help

>Number:         59208
>Category:       kern
>Synopsis:       sound patch to reduce pops and crackles and fix select on vchans
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Nov 12 01:20:14 PST 2003
>Closed-Date:
>Last-Modified:
>Originator:     Mathew Kanner
>Release:        FreeBSD 5.1-CURRENT i386
>Organization:
>Environment:
System: FreeBSD tube.mine.nu 5.1-CURRENT-20031021-JPSNAP FreeBSD 5.1-CURRENT-20031021-JPSNAP #0: Tue Oct 21 01:07:08 GMT 2003     root@ushi.jp.freebsd.org:/usr/obj/usr/src/sys/GENERIC  i386

	
>Description:
	[1]While playing with a oss test library with the creative labs opensource 
	soundblaster driver, I noticed that the playback tests had large pops and crackles.
	This was due to the application creating an small low fragment size expecting the
	OS to override it.  The result was such a small buffer underruns were frequent.
	[2] Vchans don't wakeup from select.

	
>How-To-Repeat:
	uh, broad strokes.

	fd=open('/dev/dsp', WRONLY|NONBLOCKING);
	...
	ioctl(setfragsize,(400,32))
	while(1)
	...
		select on fd
		write(fd,buf,fragsize)
	 
	
>Fix:

	Fix [1]: Use a bigger buffer.  I created a sysctl, hw.snd.fragsps, that upper-bound constrains the frags-per-second to a value.  128 works for me.  0 disables.
	Fix [2]: Stroll down the list of children (when present) during a chn_wakeup


--- channel.c.old	Wed Nov 12 02:42:43 2003
+++ channel.c	Wed Nov 12 03:59:31 2003
@@ -41,6 +41,10 @@
 #define DEB(x) x
 */
 
+static int chn_fragsps = 0;
+SYSCTL_INT(_hw_snd, OID_AUTO, fragsps, CTLFLAG_RW,
+	&chn_fragsps, 1, "max fragments per second, 0 to disable");
+
 static int chn_targetirqrate = 32;
 TUNABLE_INT("hw.snd.targetirqrate", &chn_targetirqrate);
 
@@ -59,7 +63,7 @@
 	return err;
 }
 SYSCTL_PROC(_hw_snd, OID_AUTO, targetirqrate, CTLTYPE_INT | CTLFLAG_RW,
-	0, sizeof(int), sysctl_hw_snd_targetirqrate, "I", "");
+	0, sizeof(int), sysctl_hw_snd_targetirqrate, "I", "default fragment targets this IRQ rate");
 static int report_soft_formats = 1;
 SYSCTL_INT(_hw_snd, OID_AUTO, report_soft_formats, CTLFLAG_RW,
 	&report_soft_formats, 1, "report software-emulated formats");
@@ -113,10 +117,17 @@
 chn_wakeup(struct pcm_channel *c)
 {
     	struct snd_dbuf *bs = c->bufsoft;
+        struct pcmchan_children *pce;
 
-	CHN_LOCKASSERT(c);
-	if (SEL_WAITING(sndbuf_getsel(bs)) && chn_polltrigger(c))
-		selwakeup(sndbuf_getsel(bs));
+//	CHN_LOCKASSERT(c);
+	if (SLIST_EMPTY(&c->children)) {
+		if (SEL_WAITING(sndbuf_getsel(bs)) && chn_polltrigger(c))
+			selwakeup(sndbuf_getsel(bs));
+	} else {
+		SLIST_FOREACH(pce, &c->children, link) {
+			chn_wakeup(pce->channel);
+		}
+	}
 	wakeup(bs);
 }
 
@@ -931,7 +942,7 @@
 {
 	struct snd_dbuf *b = c->bufhard;
 	struct snd_dbuf *bs = c->bufsoft;
-	int bufsz, irqhz, tmp, ret;
+	int irqhz, tmp, ret;
 
 	CHN_LOCKASSERT(c);
 	if (!CANCHANGE(c) || (c->flags & CHN_F_MAPPED))
@@ -960,14 +971,23 @@
 			DEB(printf("%s: updating (%d, %d)\n", __func__, blkcnt, blksz));
 		}
 	} else {
+		if ( chn_fragsps != 0 && 
+			sndbuf_getbps(bs) * sndbuf_getspd(bs) / blksz > chn_fragsps) 
+		{
+			blksz = sndbuf_getbps(bs) * sndbuf_getspd(bs) / chn_fragsps;
+			tmp = 32;
+			while (tmp < blksz)
+				tmp <<= 1;
+			blksz = tmp;
+			if (blksz * blkcnt > CHN_2NDBUFMAXSIZE)
+				blkcnt = CHN_2NDBUFMAXSIZE / blksz;
+		}
 		ret = EINVAL;
 		if ((blksz < 16) || (blkcnt < 2) || (blkcnt * blksz > CHN_2NDBUFMAXSIZE))
 			goto out;
 		ret = 0;
 		c->flags |= CHN_F_HAS_SIZE;
 	}
-
-	bufsz = blkcnt * blksz;
 
 	ret = sndbuf_remalloc(bs, blkcnt, blksz);
 	if (ret)
	


>Release-Note:
>Audit-Trail:
>Unformatted:



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200311120914.hAC9Ek73036355>