[08/16] winecoreaudio: Leave audio unit running, except when waveout device paused.

[Ken Thomases]

Don't stop the device each time it runs out of wavehdrs to play.  This has
several beneficial effects:

First, it eliminates a source of deadlocks.  We must not call Audio Unit
functions while holding our mutex.  There's an implicit mutex in Audio Unit
calls, and it is held while our render callback is called.  Since the
render callback has to lock our mutex, that establishes the order of
locking.  If a thread grabs both the Audio Unit mutex and our mutex, the
Audio Unit mutex must be locked first.  Therefore we must not make Audio
Unit calls while holding our mutex.

This change simplifies the code in several places.  It may also reduce
latency somewhat, by eliminating whatever delay that might be associated
with starting up the Audio Unit when output resumes.

There is one caveat.  The combined operation of changing our state variable
and starting or stopping the Audio Unit is no longer atomic, because only
the former is guarded by our mutex.  Therefore, there's a chance that the
two can get out of sync.  That is addressed by careful order of operations
within wodPause and wodReset.  See the added comment.
---
  dlls/winmm/winecoreaudio/audio.c |   89 +++++++++++++++++++++-----------------
  1 files changed, 50 insertions(+), 39 deletions(-)
-------------- next part --------------
diff --git a/dlls/winmm/winecoreaudio/audio.c b/dlls/winmm/winecoreaudio/audio.c
index 09bd2d3..d11712f 100644
--- a/dlls/winmm/winecoreaudio/audio.c
+++ b/dlls/winmm/winecoreaudio/audio.c
@@ -722,6 +722,17 @@ # endif
         return WAVERR_BADFORMAT; /* FIXME return an error based on the OSStatus */
     }
     wwo->streamDescription = streamFormat;
+
+    ret = AudioOutputUnitStart(wwo->audioUnit);
+    if (ret)
+    {
+        ERR("AudioOutputUnitStart failed: %08x\n", ret);
+        AudioUnitUninitialize(wwo->audioUnit);
+        AudioUnit_CloseAudioUnit(wwo->audioUnit);
+        pthread_mutex_unlock(&wwo->lock);
+        return MMSYSERR_ERROR; /* FIXME return an error based on the OSStatus */
+    }
+
     wwo->state = WINE_WS_STOPPED;
                 
     wwo->wFlags = HIWORD(dwFlags & CALLBACK_TYPEMASK);
@@ -916,14 +927,7 @@ static void wodHelper_PlayPtrNext(WINE_W
         wwo->lpPlayPtr = wwo->lpPlayPtr->lpNext;
 
         if (!wwo->lpPlayPtr)
-        {
-            OSStatus status;
             wwo->state = WINE_WS_STOPPED;
-            status = AudioOutputUnitStop(wwo->audioUnit);
-            if (status && wwo->err_on)
-                fprintf(stderr, "err:winecoreaudio:wodHelper_PlayPtrNext AudioOutputUnitStop return %c%c%c%c\n",
-                        (char) (status >> 24), (char) (status >> 16), (char) (status >> 8), (char) status);
-        }
         else
             wodHelper_CheckForLoopBegin(wwo);
     }
@@ -1023,12 +1027,12 @@ static  DWORD  wodHelper_Reset(WINE_WAVE
         wwo->state = WINE_WS_PAUSED;
     }
 
-    status = AudioOutputUnitStop(wwo->audioUnit);
-
     pthread_mutex_unlock(&wwo->lock);
 
+    status = AudioOutputUnitStart(wwo->audioUnit);
+
     if (status) {
-        ERR( "AudioOutputUnitStop return %c%c%c%c\n",
+        ERR( "AudioOutputUnitStart return %c%c%c%c\n",
              (char) (status >> 24), (char) (status >> 16), (char) (status >> 8), (char) status);
         return MMSYSERR_ERROR; /* FIXME return an error based on the OSStatus */
     }
@@ -1084,13 +1088,7 @@ static DWORD wodWrite(WORD wDevID, LPWAV
         wwo->lpPlayPtr = lpWaveHdr;
 
         if (wwo->state == WINE_WS_STOPPED)
-        {
-            OSStatus status = AudioOutputUnitStart(wwo->audioUnit);
-            if (status) {
-                ERR("AudioOutputUnitStart return %c%c%c%c\n", (char) (status >> 24), (char) (status >> 16), (char) (status >> 8), (char) status);
-            }
-            else wwo->state = WINE_WS_PLAYING;
-        }
+            wwo->state = WINE_WS_PLAYING;
 
         wodHelper_CheckForLoopBegin(wwo);
 
@@ -1115,21 +1113,26 @@ static DWORD wodPause(WORD wDevID)
         WARN("bad device ID !\n");
         return MMSYSERR_BADDEVICEID;
     }
-    
+
+    /* The order of the following operations is important since we can't hold
+     * the mutex while we make an Audio Unit call.  Stop the Audio Unit before
+     * setting the PAUSED state.  In wodRestart, the order is reversed.  This
+     * guarantees that we can't get into a situation where the state is
+     * PLAYING or STOPPED but the Audio Unit isn't running.  Although we can
+     * be in PAUSED state with the Audio Unit still running, that's harmless
+     * because the render callback will just produce silence.
+     */
+    status = AudioOutputUnitStop(WOutDev[wDevID].audioUnit);
+    if (status) {
+        WARN("AudioOutputUnitStop return %c%c%c%c\n",
+             (char) (status >> 24), (char) (status >> 16), (char) (status >> 8), (char) status);
+    }
+
     pthread_mutex_lock(&WOutDev[wDevID].lock);
     if (WOutDev[wDevID].state == WINE_WS_PLAYING || WOutDev[wDevID].state == WINE_WS_STOPPED)
-    {
         WOutDev[wDevID].state = WINE_WS_PAUSED;
-        status = AudioOutputUnitStop(WOutDev[wDevID].audioUnit);
-        if (status) {
-            ERR( "AudioOutputUnitStop return %c%c%c%c\n",
-                 (char) (status >> 24), (char) (status >> 16), (char) (status >> 8), (char) status);
-            pthread_mutex_unlock(&WOutDev[wDevID].lock);
-            return MMSYSERR_ERROR; /* FIXME return an error based on the OSStatus */
-        }
-    }
     pthread_mutex_unlock(&WOutDev[wDevID].lock);
-    
+
     return MMSYSERR_NOERROR;
 }
 
@@ -1138,6 +1141,8 @@ static DWORD wodPause(WORD wDevID)
 */
 static DWORD wodRestart(WORD wDevID)
 {
+    OSStatus status;
+
     TRACE("(%u);\n", wDevID);
     
     if (wDevID >= MAX_WAVEOUTDRV )
@@ -1145,26 +1150,32 @@ static DWORD wodRestart(WORD wDevID)
         WARN("bad device ID !\n");
         return MMSYSERR_BADDEVICEID;
     }
-    
+
+    /* The order of the following operations is important since we can't hold
+     * the mutex while we make an Audio Unit call.  Set the PLAYING/STOPPED
+     * state before starting the Audio Unit.  In wodPause, the order is
+     * reversed.  This guarantees that we can't get into a situation where
+     * the state is PLAYING or STOPPED but the Audio Unit isn't running.
+     * Although we can be in PAUSED state with the Audio Unit still running,
+     * that's harmless because the render callback will just produce silence.
+     */
     pthread_mutex_lock(&WOutDev[wDevID].lock);
     if (WOutDev[wDevID].state == WINE_WS_PAUSED)
     {
         if (WOutDev[wDevID].lpPlayPtr)
-        {
-            OSStatus status = AudioOutputUnitStart(WOutDev[wDevID].audioUnit);
-            if (status) {
-                ERR("AudioOutputUnitStart return %c%c%c%c\n",
-                    (char) (status >> 24), (char) (status >> 16), (char) (status >> 8), (char) status);
-                pthread_mutex_unlock(&WOutDev[wDevID].lock);
-                return MMSYSERR_ERROR; /* FIXME return an error based on the OSStatus */
-            }
             WOutDev[wDevID].state = WINE_WS_PLAYING;
-        }
         else
             WOutDev[wDevID].state = WINE_WS_STOPPED;
     }
     pthread_mutex_unlock(&WOutDev[wDevID].lock);
-    
+
+    status = AudioOutputUnitStart(WOutDev[wDevID].audioUnit);
+    if (status) {
+        ERR("AudioOutputUnitStart return %c%c%c%c\n",
+            (char) (status >> 24), (char) (status >> 16), (char) (status >> 8), (char) status);
+        return MMSYSERR_ERROR; /* FIXME return an error based on the OSStatus */
+    }
+
     return MMSYSERR_NOERROR;
 }