Bug in buffer_service

Francois Gouget fgouget at codeweavers.com
Fri Jul 16 16:40:55 CDT 2004


When running the ds3d test in interactive mode on Windows I noticed that 
the sounds sometimes played too long. I added some code to check on this 
  and some sounds were indeed playing for 6 or 7 seconds instead of the 
planned 4 seconds (it even once played for 18 seconds).

The reason is a bug in buffer_service() which failed to detected that 
the whole buffer had been played.

There are in fact two bugs:
  * We don't check whether the sound has been fully plyed until it has 
been fully written, i.e. state->written==state->wave_len. Then if offset 
(the position of the last byte to be played) is less than play_pos (the 
byte currently being played), then we have to wait for DirectSound to 
wrap around the buffer. So we set last_pos=play_pos and wait until we 
see play_pos<last_pos.
But if at that time play_pos=64 in a buffer of 32580 bytes, the chances 
of buffer_service() being called right when the play pos is in the right 
  0.2% of the buffer are pretty slim (buffer_service was typically 
called every 4500 bytes). I have seen this bug happen on a regular basis.

  * buffer_service() also requires that play_pos>(written % 
buffer_size). In the case I was seing it was not a problem because 
written % buffer_size was zero. But just we could very well end up with 
written % buffer_size=32570 and then we would have been very lucky to 
catch play_pos just when it is in the right 0.03% of the buffer.

The right way to detect when everything has been played is to keep track 
of how much has been played.


Changelog:

  * dlls/dsound/tests/dsound_test.h
    dlls/dsound/tests/ds3d.c

    Francois Gouget <fgouget at codeweavers.com>
    Keep track of how much has been played and get rid of last_pos. This 
fixes a bug where buffer_service() would not detect that everything had 
been played.
    Check that the actual sound duration was within 10% of the expected 
value.
    Make BUFFER_LEN and TIME_SLICE independent, make sure TIME_SLICE 
does not divide BUFFER_LEN to spice things up a bit.

-- 
Francois Gouget
fgouget at codeweavers.com

-------------- next part --------------
Index: dlls/dsound/tests/dsound_test.h
===================================================================
RCS file: /var/cvs/wine/dlls/dsound/tests/dsound_test.h,v
retrieving revision 1.1
diff -u -r1.1 dsound_test.h
--- dlls/dsound/tests/dsound_test.h	17 Jun 2004 23:03:11 -0000	1.1
+++ dlls/dsound/tests/dsound_test.h	16 Jul 2004 21:24:59 -0000
@@ -47,8 +47,8 @@
 #define NB_FORMATS (sizeof(formats)/sizeof(*formats))
 
 /* The time slice determines how often we will service the buffer */
-#define TIME_SLICE    100
-#define BUFFER_LEN    (4*TIME_SLICE)
+#define TIME_SLICE     31
+#define BUFFER_LEN    400
 
 
 extern char* wave_generate_la(WAVEFORMATEX*,double,DWORD*);
Index: dlls/dsound/tests/ds3d.c
===================================================================
RCS file: /var/cvs/wine/dlls/dsound/tests/ds3d.c,v
retrieving revision 1.1
diff -u -r1.1 ds3d.c
--- dlls/dsound/tests/ds3d.c	17 Jun 2004 23:03:11 -0000	1.1
+++ dlls/dsound/tests/ds3d.c	16 Jul 2004 16:31:02 -0000
@@ -104,9 +104,8 @@
     LPWAVEFORMATEX wfx;
     DWORD buffer_size;
     DWORD written;
+    DWORD played;
     DWORD offset;
-
-    DWORD last_pos;
 } play_state_t;
 
 static int buffer_refill(play_state_t* state, DWORD size)
@@ -166,48 +165,53 @@
 
 static int buffer_service(play_state_t* state)
 {
-    DWORD play_pos,write_pos,buf_free;
+    DWORD last_play_pos,play_pos,buf_free;
     HRESULT rc;
 
-    rc=IDirectSoundBuffer_GetCurrentPosition(state->dsbo,&play_pos,&write_pos);
+    rc=IDirectSoundBuffer_GetCurrentPosition(state->dsbo,&play_pos,NULL);
     ok(rc==DS_OK,"GetCurrentPosition: %lx\n",rc);
     if (rc!=DS_OK) {
         goto STOP;
     }
 
+    /* Update the amount played */
+    last_play_pos=state->played % state->buffer_size;
+    if (play_pos<last_play_pos)
+        state->played+=state->buffer_size-last_play_pos+play_pos;
+    else
+        state->played+=play_pos-last_play_pos;
+
+    if (winetest_debug > 1)
+        trace("buf size=%ld last_play_pos=%ld play_pos=%ld played=%ld / %ld\n",
+              state->buffer_size,last_play_pos,play_pos,state->played,state->wave_len);
+
+    if (state->played>state->wave_len)
+    {
+        /* Everything has been played */
+        goto STOP;
+    }
+
     /* Refill the buffer */
-    if (state->offset<=play_pos) {
+    if (state->offset<=play_pos)
         buf_free=play_pos-state->offset;
-    } else {
+    else
         buf_free=state->buffer_size-state->offset+play_pos;
-    }
+
     if (winetest_debug > 1)
-        trace("buf pos=%ld free=%ld written=%ld / %ld\n",
-              play_pos,buf_free,state->written,state->wave_len);
+        trace("offset=%ld free=%ld written=%ld / %ld\n",
+              state->offset,buf_free,state->written,state->wave_len);
     if (buf_free==0)
         return 1;
 
-    if (state->written<state->wave_len) {
+    if (state->written<state->wave_len)
+    {
         int w=buffer_refill(state,buf_free);
         if (w==-1)
             goto STOP;
         buf_free-=w;
-        if (state->written==state->wave_len) {
-            state->last_pos=(state->offset<play_pos)?play_pos:0;
-            if (winetest_debug > 1)
-                trace("last sound byte at %ld\n",
-                      (state->written % state->buffer_size));
-        }
-    } else {
-        if (state->last_pos!=0 && play_pos<state->last_pos) {
-            /* We wrapped around the end of the buffer */
-            state->last_pos=0;
-        }
-        if (state->last_pos==0 &&
-            play_pos>(state->written % state->buffer_size)) {
-            /* Now everything has been played */
-            goto STOP;
-        }
+        if (state->written==state->wave_len && winetest_debug > 1)
+            trace("last sound byte at %ld\n",
+                  (state->written % state->buffer_size));
     }
 
     if (buf_free>0) {
@@ -341,6 +341,7 @@
         DS3DLISTENER listener_param;
         LPDIRECTSOUND3DBUFFER buffer=NULL;
         DS3DBUFFER buffer_param;
+        DWORD start_time,now;
 
         trace("    Playing %g second 440Hz tone at %ldx%dx%d\n", duration,
               wfx.nSamplesPerSec, wfx.wBitsPerSample,wfx.nChannels);
@@ -397,7 +398,7 @@
             buffer_param.dwSize=sizeof(buffer_param);
             rc=IDirectSound3DBuffer_GetAllParameters(buffer,&buffer_param);
             ok(rc==DS_OK,"IDirectSound3DBuffer_GetAllParameters failed: 0x%lx\n",rc);
-}
+        }
         if (set_volume) {
             if (dsbcaps.dwFlags & DSBCAPS_CTRLVOLUME) {
                 LONG val;
@@ -436,7 +438,7 @@
         state.dsbo=dsbo;
         state.wfx=&wfx;
         state.buffer_size=dsbcaps.dwBufferBytes;
-        state.written=state.offset=0;
+        state.played=state.written=state.offset=0;
         buffer_refill(&state,state.buffer_size);
 
         rc=IDirectSoundBuffer_Play(dsbo,0,0,DSBPLAY_LOOPING);
@@ -472,8 +473,9 @@
             ok(rc==DS_OK,"IDirectSound3dBuffer_SetPosition failed 0x%lx\n",rc);
         }
 
+        start_time=GetTickCount();
         while (buffer_service(&state)) {
-            WaitForSingleObject(GetCurrentProcess(),TIME_SLICE/2);
+            WaitForSingleObject(GetCurrentProcess(),TIME_SLICE);
             if (listener && move_listener) {
                 listener_param.vPosition.x += 0.5;
                 rc=IDirectSound3DListener_SetPosition(listener,listener_param.vPosition.x,listener_param.vPosition.y,listener_param.vPosition.z,DS3D_IMMEDIATE);
@@ -485,6 +487,9 @@
                 ok(rc==DS_OK,"IDirectSound3dBuffer_SetPosition failed 0x%lx\n",rc);
             }
         }
+        /* Check the sound duration was within 10% of the expected value */
+        now=GetTickCount();
+        ok(fabs(1000*duration-now+start_time)<=100*duration,"The sound played for %ld ms instead of %g ms\n",now-start_time,1000*duration);
 
         free(state.wave);
         if (is_primary) {


More information about the wine-patches mailing list