Bug 31445

I've got a fix for this.  It may not be ideal, but I think it
characterizes the problem well enough that someone who cares can fine
tune it.

The problem is that if a sound is small enough, it never apsses the low
water mark in chn_start to start the playout process.  This patch looks
for small sounds in chn_flush.  If it detects such a sound (a partially
filled soft output buffer when the channel is not triggered), it pads
the output buffer with silence and starts it playing.

A patch follows, relative to /sys/dev/sound/pcm


--- buffer.c.orig	Sun Oct 28 18:31:14 2001
+++ buffer.c	Sun Oct 28 18:33:09 2001
@@ -218,6 +218,28 @@
 }
 
 void
+sndbuf_padsilence(struct snd_dbuf *b, unsigned int length)
+{
+	int i;
+	u_char data, *p;
+	unsigned int len = length;
+
+	if (b->fmt & AFMT_SIGNED)
+		data = 0x00;
+	else
+		data = 0x80;
+
+	i = sndbuf_getfreeptr(b);
+	p = sndbuf_getbuf(b);
+	while (len) {
+		p[i++] = data;
+		len--;
+		if ( i >= b->bufsize ) i = 0;
+	}
+	b->rl += length;
+}
+
+void
 sndbuf_reset(struct snd_dbuf *b)
 {
 	b->hp = 0;
--- buffer.h.orig	Sun Oct 28 18:31:19 2001
+++ buffer.h	Sun Oct 28 18:32:15 2001
@@ -45,6 +45,7 @@
 int sndbuf_remalloc(struct snd_dbuf *b, unsigned int blkcnt, unsigned int blksz);
 void sndbuf_reset(struct snd_dbuf *b);
 void sndbuf_clear(struct snd_dbuf *b, unsigned int length);
+void sndbuf_padsilence(struct snd_dbuf *b, unsigned int length);
 void sndbuf_fillsilence(struct snd_dbuf *b);
 
 u_int32_t sndbuf_getfmt(struct snd_dbuf *b);
--- channel.c.orig	Sun Oct 28 18:31:05 2001
+++ channel.c	Sun Oct 28 18:41:40 2001
@@ -555,8 +555,17 @@
 	CHN_LOCKASSERT(c);
 	KASSERT(c->direction == PCMDIR_PLAY, ("chn_wrupdate on bad channel"));
     	DEB(printf("chn_flush c->flags 0x%08x\n", c->flags));
-	if (!(c->flags & CHN_F_TRIGGERED))
-		return 0;
+	if (!(c->flags & CHN_F_TRIGGERED)) 
+		if ( sndbuf_getready(bs) > 0 && 
+		     sndbuf_getready(bs) < sndbuf_getfree(b) ) {
+			/* this is probably a short sound, pad it out with
+			   silence and start the driver playing it.  */
+			sndbuf_padsilence(bs, sndbuf_getfree(b));
+			chn_start(c, 1);
+			chn_sleep(c, "pcmflu", hz / 10);
+		}
+		else
+			return 0;
 
 	c->flags |= CHN_F_CLOSING;
 	resid = sndbuf_getready(bs) + sndbuf_getready(b);

This may be a resumbission.  I sent it once PGP signed, and that may not
fly.

I've got a fix for this.  It may not be ideal, but I think it
characterizes the problem well enough that someone who cares can fine
tune it.

The problem is that if a sound is small enough, it never apsses the low
water mark in chn_start to start the playout process.  This patch looks
for small sounds in chn_flush.  If it detects such a sound (a partially
filled soft output buffer when the channel is not triggered), it pads
the output buffer with silence and starts it playing.

A patch follows, relative to /sys/dev/sound/pcm


--- buffer.c.orig	Sun Oct 28 18:31:14 2001
+++ buffer.c	Sun Oct 28 18:33:09 2001
@@ -218,6 +218,28 @@
 }
 
 void
+sndbuf_padsilence(struct snd_dbuf *b, unsigned int length)
+{
+	int i;
+	u_char data, *p;
+	unsigned int len = length;
+
+	if (b->fmt & AFMT_SIGNED)
+		data = 0x00;
+	else
+		data = 0x80;
+
+	i = sndbuf_getfreeptr(b);
+	p = sndbuf_getbuf(b);
+	while (len) {
+		p[i++] = data;
+		len--;
+		if ( i >= b->bufsize ) i = 0;
+	}
+	b->rl += length;
+}
+
+void
 sndbuf_reset(struct snd_dbuf *b)
 {
 	b->hp = 0;
--- buffer.h.orig	Sun Oct 28 18:31:19 2001
+++ buffer.h	Sun Oct 28 18:32:15 2001
@@ -45,6 +45,7 @@
 int sndbuf_remalloc(struct snd_dbuf *b, unsigned int blkcnt, unsigned int blksz);
 void sndbuf_reset(struct snd_dbuf *b);
 void sndbuf_clear(struct snd_dbuf *b, unsigned int length);
+void sndbuf_padsilence(struct snd_dbuf *b, unsigned int length);
 void sndbuf_fillsilence(struct snd_dbuf *b);
 
 u_int32_t sndbuf_getfmt(struct snd_dbuf *b);
--- channel.c.orig	Sun Oct 28 18:31:05 2001
+++ channel.c	Sun Oct 28 18:41:40 2001
@@ -555,8 +555,17 @@
 	CHN_LOCKASSERT(c);
 	KASSERT(c->direction == PCMDIR_PLAY, ("chn_wrupdate on bad channel"));
     	DEB(printf("chn_flush c->flags 0x%08x\n", c->flags));
-	if (!(c->flags & CHN_F_TRIGGERED))
-		return 0;
+	if (!(c->flags & CHN_F_TRIGGERED)) 
+		if ( sndbuf_getready(bs) > 0 && 
+		     sndbuf_getready(bs) < sndbuf_getfree(b) ) {
+			/* this is probably a short sound, pad it out with
+			   silence and start the driver playing it.  */
+			sndbuf_padsilence(bs, sndbuf_getfree(b));
+			chn_start(c, 1);
+			chn_sleep(c, "pcmflu", hz / 10);
+		}
+		else
+			return 0;
 
 	c->flags |= CHN_F_CLOSING;
 	resid = sndbuf_getready(bs) + sndbuf_getready(b);

Just a quick follow-up to Ted's post of a patch:  I (hand-)applied the
patch to my -STABLE sources on my laptop (running 4.4-STABLE as of this
morning).  The patch definitely improves the handling of the short
sounds, and avoids the truncation of longer ones (such as gong.au).

I'll try -CURRENT next.

(Audio hardware here is:
pcm0: <ESS Technology Maestro-2E> port 0x1400-0x14ff irq 7 at device 8.0 on pci0
in case that's of interest.)

Cheers,
david
-- 
David H. Wolfskill				david@catwhisker.org
As a computing professional, I believe it would be unethical for me to
advise, recommend, or support the use (save possibly for personal
amusement) of any product that is or depends on any Microsoft product.

OK; works on today's -CURRENT (same hardware), as well.  (For -CURRENT,
I did a "cvs diff" from the -STABLE sources & applied that as a patch
to the -CURRENT sources.  No changes were required.)

Cheers,
david
-- 
David H. Wolfskill				david@catwhisker.org
As a computing professional, I believe it would be unethical for me to
advise, recommend, or support the use (save possibly for personal
amusement) of any product that is or depends on any Microsoft product.

Comment 5 Kris Kennaway 2002-01-06 03:55:35 UTC

Responsible Changed
From-To: freebsd-bugs->sound

Over to sound maintainers

I'm really disappointed to see that this bug, which appeared in
the big audio subsystem change between 4.3 and 4.4, is still here
in 5.4.

I need to deal with short audio files:  words, phrases,
concatenated (but with a CLOSE and OPEN between segments)
to make fairly natural speech.  I'm still running on
an ancient MS-DOS box because this is broken in FreeBSD.

As an example, I have put a set of .wav files speaking
the names of US states and Canadian provinces here:

    <http://www.k1bc.com/testwav/>

If you play those on a non-buggy system, you'll hear the
names of the states.

But on FBSD (4.4 and all later),

    >foreach fil (testwav/*)
    ->      play $fil
    ->      end
    >

you'll hear unintelligible fragments.

But rather than just vent, I'm ready to help write code to fix it.
I don't have the background, though.  I need some architecture
documents on the audio subsystem.  I see it as extremely
complex, to deal with all the sound cards in the world and
also multiple simultaneous sources of audio.

I can't tell where to focus on passing short buffers
through the stack.  Give me some hints.

[The suggested patch of padding the final buffer is not the right
solution.  It completely breaks the rhythm of the speech.]

Thanks,
/Rcc

Comment 7 Alexander Leidinger 2005-09-11 15:35:05 UTC

State Changed
From-To: open->closed

This is supposed to be fixed in -current.

.

Assuming the above-mentioned "-current" referenced by netchild is
in 6.0 RELEASE, I believe this PR was incorrectly
(unthinkingly/disrespectfully?) closed.


I have installed 6.0 on a box here and performed the test described
in my previous comment on this PR.

I added snd_driver_load and sound_load to /boot/loader.conf, and
I downloaded/built sox from /usr/ports.

I repeated the test described in my previous comment.  (The
lunabase.org files do not seem to be available now.)

To make sure that the bug is not in sox, I also tried the same
test using wavplay.

The results are the same from both userland programs.  The short
files are still either truncated or completely omitted.


I request that this PR be re-opened.  And, of course, I
request that the bug be fixed.  I again offer to work on
it, if given some guidance.


/Rcc

Comment 9 Ariff Abdullah 2005-11-12 04:52:49 UTC

On Sat, 12 Nov 2005 01:20:18 GMT
FBSD lists member <7c3b40e4@clements.org> wrote:
>  
>  Assuming the above-mentioned "-current" referenced by netchild is
>  in 6.0 RELEASE, I believe this PR was incorrectly
>  (unthinkingly/disrespectfully?) closed.
>  
You should have known that "-current" does mean "7". "6" is on its way
to become "-stable".
>  
>  I have installed 6.0 on a box here and performed the test described
>  in my previous comment on this PR.
>  
>  I added snd_driver_load and sound_load to /boot/loader.conf, and
>  I downloaded/built sox from /usr/ports.
>  
>  I repeated the test described in my previous comment.  (The
>  lunabase.org files do not seem to be available now.)
>  
>  To make sure that the bug is not in sox, I also tried the same
>  test using wavplay.
>  
>  The results are the same from both userland programs.  The short
>  files are still either truncated or completely omitted.
>  
Would you kindly repeat that test after applying patch from
http://people.freebsd.org/~ariff/ ? Since you're using 'RELEASE', the
appropriate patch is snd_RELENG_6_0_20051108_059.diff. It contains
massive fixes, which unfortunately did not made its way into RELENG_6
by the time of 6.0-RELEASE.

Part of the discussion (simmilar PR) can be found at
http://lists.freebsd.org/pipermail/freebsd-multimedia/2005-November/003025.html

>  
>  I request that this PR be re-opened.  And, of course, I
>  request that the bug be fixed.  I again offer to work on
>  it, if given some guidance.
>  
>  



--
Ariff Abdullah
FreeBSD