DSOUND: buffer underrun & sound chunks left out (RESEND)

[Ove Kaaven]

man, 2003-03-17 kl. 10:06 skrev Jerry Ji:
> This patch fix two problem:
> 1. the improper play position return by DSOUND_PrimaryGetPosition - 
> beffer underrun,

I guess this should be considered a bug in wineoss (or maybe your kernel
drivers), but it's not a dsound bug and should not be fixed by reducing
the readout precision of drivers that actually work properly. I don't
think your buffer.c patch is very correct either, it would cause
spurious noise under some circumstances.

> 2. and the notify mechanics we used - sound data skipped.
> 
> Please refer to the comment for the first problem.
> 
> DSOUND_CheckEvent(IDirectSoundBufferImpl *dsb, int len) original use 
> current play position and the count of data TO BE played to check if a 
> notify should be generated. That's to say, an application may be 
> notified before the data at the desired offset really played. Since the 
> application got the notify that the data in it's demand region is 
> useless (played), it might call IDirectSoundBufferImpl_Lock to lock this 
> region, which contains useful data, IDirectSoundBufferImpl_Lock will 
> handle the conflict by cancelling the region by default.

Here's my fix for the problem in WineX, and it's supposed to be able to
handle the OFFSETSTOP case. I think it should apply to Wine without too
much trouble.

Log:
Ove Kaaven <ovek at transgaming.com>
Somewhat more reliable dsound position notifications.

Index: mixer.c
===================================================================
RCS file: /cvsroot/winex/wine/dlls/dsound/mixer.c,v
retrieving revision 1.8
retrieving revision 1.10
diff -u -r1.8 -r1.10
--- mixer.c	1 Oct 2002 13:51:32 -0000	1.8
+++ mixer.c	14 Feb 2003 22:04:28 -0000	1.10
@@ -345,17 +345,6 @@
 
 	if (len == 0) {
 		/* This should only happen if we aren't looping and temp < 4 */
-
-		/* We skip the remainder, so check for possible events */
-		DSOUND_CheckEvent(dsb, dsb->buflen - dsb->buf_mixpos);
-		/* Stop */
-		dsb->state = STATE_STOPPED;
-		dsb->playpos = 0;
-		dsb->last_playpos = 0;
-		dsb->buf_mixpos = 0;
-		dsb->leadin = FALSE;
-		/* Check for DSBPN_OFFSETSTOP */
-		DSOUND_CheckEvent(dsb, 0);
 		return 0;
 	}
 
@@ -397,9 +386,6 @@
 	}
 	/* free(buf); */
 
-	if (dsb->dsbd.dwFlags & DSBCAPS_CTRLPOSITIONNOTIFY)
-		DSOUND_CheckEvent(dsb, ilen);
-
 	if (dsb->leadin && (dsb->startpos > dsb->buf_mixpos) && (dsb->startpos <= dsb->buf_mixpos + ilen)) {
 		/* HACK... leadin should be reset when the PLAY position reaches the startpos,
 		 * not the MIX position... but if the sound buffer is bigger than our prebuffering
@@ -411,14 +397,7 @@
 	dsb->buf_mixpos += ilen;
 
 	if (dsb->buf_mixpos >= dsb->buflen) {
-		if (!(dsb->playflags & DSBPLAY_LOOPING)) {
-			dsb->state = STATE_STOPPED;
-			dsb->playpos = 0;
-			dsb->last_playpos = 0;
-			dsb->buf_mixpos = 0;
-			dsb->leadin = FALSE;
-			DSOUND_CheckEvent(dsb, 0);		/* For DSBPN_OFFSETSTOP */
-		} else {
+		if (dsb->playflags & DSBPLAY_LOOPING) {
 			/* wrap */
 			while (dsb->buf_mixpos >= dsb->buflen)
 				dsb->buf_mixpos -= dsb->buflen;
@@ -573,6 +552,10 @@
 	DWORD adv_done =
 		((dsb->dsound->mixpos < writepos) ? dsb->dsound->buflen : 0) +
 		dsb->dsound->mixpos - writepos;
+	DWORD played =
+		((buf_writepos < dsb->playpos) ? dsb->buflen : 0) +
+		buf_writepos - dsb->playpos;
+	DWORD buf_left = dsb->buflen - buf_writepos;
 	int still_behind;
 
 	TRACE("buf_writepos=%ld, primary_writepos=%ld\n", buf_writepos, writepos);
@@ -581,6 +564,12 @@
 	      mixlen);
 	TRACE("looping=%ld, startpos=%ld, leadin=%ld\n", dsb->playflags, dsb->startpos, dsb->leadin);
 
+	/* check for notification positions */
+	if (dsb->dsbd.dwFlags & DSBCAPS_CTRLPOSITIONNOTIFY &&
+	    dsb->state != STATE_STARTING) {
+		DSOUND_CheckEvent(dsb, played);
+	}
+
 	/* save write position for non-GETCURRENTPOSITION2... */
 	dsb->playpos = buf_writepos;
 
@@ -653,7 +642,8 @@
 		/* smaller than a fragment, wait until it gets larger
 		 * before we take the mixing overhead */
 		TRACE("mixlen not worth it, deferring mixing\n");
-		return 0;
+		still_behind = 1;
+		goto post_mix;
 	}
 
 	/* ok, we know how much to mix, let's go */
@@ -675,6 +665,19 @@
 	}
 	TRACE("new primary_mixpos=%ld, primary_advbase=%ld\n", dsb->primary_mixpos, dsb->dsound->mixpos);
 	TRACE("mixed data len=%ld, still_behind=%d\n", mixlen-len, still_behind);
+
+post_mix:
+	/* check if buffer should be considered complete */
+	if (buf_left < dsb->writelead &&
+	    !(dsb->playflags & DSBPLAY_LOOPING)) {
+		dsb->state = STATE_STOPPED;
+		dsb->playpos = 0;
+		dsb->last_playpos = 0;
+		dsb->buf_mixpos = 0;
+		dsb->leadin = FALSE;
+		DSOUND_CheckEvent(dsb, buf_left);
+	}
+
 	/* return how far we think the primary buffer can
 	 * advance its underrun detector...*/
 	if (still_behind) return 0;
@@ -707,6 +710,7 @@
 			if (dsb->state == STATE_STOPPING) {
 				DSOUND_MixCancel(dsb, writepos, TRUE);
 				dsb->state = STATE_STOPPED;
+				DSOUND_CheckEvent(dsb, 0);
 			} else {
 				if ((dsb->state == STATE_STARTING) || recover) {
 					dsb->primary_mixpos = writepos;