Changeset 87159 in vbox

Timestamp:

Jan 4, 2021 11:38:37 AM (4 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

142072

Message:

Audio: Enable/disable streams using PDMIAUDIOCONNECTOR::pfnStreamControl() instead of PDMIAUDIOCONNECTOR::pfnEnable(). bugref:9882

Location:

trunk

Files:

• include/VBox/vmm/pdmaudioifs.h (modified) (1 diff)
• src/VBox/Devices/Audio/AudioMixer.cpp (modified) (2 diffs)
• src/VBox/Devices/Audio/DrvAudio.cpp (modified) (10 diffs)

trunk/include/VBox/vmm/pdmaudioifs.h

@@ -1392 +1392 @@
      * Associated input streams then return silence when read from those.
      *
+     * Warning: Use this function carefully, as this could violate / run against the (global) VM settings. See #9882.
+     *
      * @returns VBox status code.
      * @param   pInterface      Pointer to the interface structure containing the called function pointer.

trunk/src/VBox/Devices/Audio/AudioMixer.cpp

@@ -1532 +1532 @@
     int rc;
 
-    if (pSink->In.pStreamRecSource) /* First, disable old recording source, if any set. */
-    {
-        const PPDMIAUDIOCONNECTOR pConn = pSink->In.pStreamRecSource->pConn;
-        AssertPtr(pConn);
-        rc = pConn->pfnEnable(pConn, PDMAUDIODIR_IN, false /* Disable */);
+    /*
+     * Warning: Do *not* use pfnConn->pfnEnable() for enabling/disabling streams here, as this will unconditionally (re-)enable
+     *          streams, which would violate / run against the (global) VM settings. See #9882.
+     */
+
+    /* Get pointers of current recording source to make code easier to read below. */
+    PAUDMIXSTREAM       pCurRecSrc       = pSink->In.pStreamRecSource; /* Can be NULL. */
+    PPDMIAUDIOCONNECTOR pCurRecSrcConn   = NULL;
+    PPDMAUDIOSTREAM     pCurRecSrcStream = NULL;
+
+    if (pCurRecSrc) /* First, disable old recording source, if any is set. */
+    {
+        pCurRecSrcConn   = pSink->In.pStreamRecSource->pConn;
+        AssertPtrReturn(pCurRecSrcConn, VERR_INVALID_POINTER);
+        pCurRecSrcStream = pCurRecSrc->pStream;
+        AssertPtrReturn(pCurRecSrcStream, VERR_INVALID_POINTER);
+
+        rc = pCurRecSrcConn->pfnStreamControl(pCurRecSrcConn, pCurRecSrcStream, PDMAUDIOSTREAMCMD_DISABLE);
     }
     else
@@ -1548 +1561 @@
             AssertMsg(pStream->pStream->enmDir == PDMAUDIODIR_IN, ("Specified stream is not an input stream\n"));
             AssertPtr(pStream->pConn);
-            rc = pStream->pConn->pfnEnable(pStream->pConn, PDMAUDIODIR_IN, true /* Enable */);
+            rc = pStream->pConn->pfnStreamControl(pStream->pConn, pStream->pStream, PDMAUDIOSTREAMCMD_ENABLE);
             if (RT_SUCCESS(rc))
-                pSink->In.pStreamRecSource = pStream;
-            else if (pSink->In.pStreamRecSource) /* Stay with the current recording source (if any) and re-enable it. */
             {
-                const PPDMIAUDIOCONNECTOR pConn = pSink->In.pStreamRecSource->pConn;
-                AssertPtr(pConn);
-                rc = pConn->pfnEnable(pConn, PDMAUDIODIR_IN, true /* Enable */);
+                pCurRecSrc = pStream;
             }
+            else if (pCurRecSrc) /* Stay with the current recording source (if any) and re-enable it. */
+            {
+                rc = pCurRecSrcConn->pfnStreamControl(pCurRecSrcConn, pCurRecSrcStream, PDMAUDIOSTREAMCMD_ENABLE);
+            }
         }
         else
-            pSink->In.pStreamRecSource = NULL; /* Unsetting, see audioMixerSinkRemoveStreamInternal. */
-    }
+            pCurRecSrc = NULL; /* Unsetting, see audioMixerSinkRemoveStreamInternal. */
+    }
+
+    /* Invalidate pointers. */
+    pSink->In.pStreamRecSource = pCurRecSrc;
 
     LogFunc(("[%s] Recording source is now '%s', rc=%Rrc\n",

trunk/src/VBox/Devices/Audio/DrvAudio.cpp

@@ -448 +448 @@
         return VINF_SUCCESS;
 
-    LogRel2(("Audio: %s stream '%s'\n", DrvAudioHlpStreamCmdToStr(enmStreamCmd), pStream->szName));
-
     int rc = VINF_SUCCESS;
 
+    /*
+     * Whether to propagate commands down to the backend.
+     *
+     * This is needed for critical operations like recording audio if audio input is disabled on a per-driver level.
+     *
+     * Note that not all commands will be covered by this, such as operations like stopping, draining and droppping,
+     * which are considered uncritical and sometimes even are required for certain backends (like DirectSound on Windows).
+     *
+     * The actual stream state will be untouched to not make the state machine handling more complicated than
+     * it already is.
+     *
+     * See #9882.
+     */
+    const bool fEnabled =    (   pStream->enmDir == PDMAUDIODIR_IN
+                              && pThis->In.fEnabled)
+                          || (   pStream->enmDir == PDMAUDIODIR_OUT
+                              && pThis->Out.fEnabled);
+
+    LogRel2(("Audio: %s stream '%s' in backend (%s is %s)\n", DrvAudioHlpStreamCmdToStr(enmStreamCmd), pStream->szName,
+                                                              DrvAudioHlpAudDirToStr(pStream->enmDir),
+                                                              fEnabled ? "enabled" : "disabled"));
     switch (enmStreamCmd)
     {
         case PDMAUDIOSTREAMCMD_ENABLE:
         {
-            rc = pThis->pHostDrvAudio->pfnStreamControl(pThis->pHostDrvAudio, pStream->pvBackend, PDMAUDIOSTREAMCMD_ENABLE);
+            if (fEnabled)
+                rc = pThis->pHostDrvAudio->pfnStreamControl(pThis->pHostDrvAudio, pStream->pvBackend, PDMAUDIOSTREAMCMD_ENABLE);
             break;
         }
@@ -468 +488 @@
         case PDMAUDIOSTREAMCMD_PAUSE:
         {
-            rc = pThis->pHostDrvAudio->pfnStreamControl(pThis->pHostDrvAudio, pStream->pvBackend, PDMAUDIOSTREAMCMD_PAUSE);
+            if (fEnabled) /* Needed, as resume below also is being checked for. */
+                rc = pThis->pHostDrvAudio->pfnStreamControl(pThis->pHostDrvAudio, pStream->pvBackend, PDMAUDIOSTREAMCMD_PAUSE);
             break;
         }
@@ -474 +495 @@
         case PDMAUDIOSTREAMCMD_RESUME:
         {
-            rc = pThis->pHostDrvAudio->pfnStreamControl(pThis->pHostDrvAudio, pStream->pvBackend, PDMAUDIOSTREAMCMD_RESUME);
+            if (fEnabled)
+                rc = pThis->pHostDrvAudio->pfnStreamControl(pThis->pHostDrvAudio, pStream->pvBackend, PDMAUDIOSTREAMCMD_RESUME);
             break;
         }
@@ -939 +961 @@
     do
     {
-        if (   !pThis->Out.fEnabled
-            || !DrvAudioHlpStreamStatusIsReady(pStream->fStatus))
+        if (!DrvAudioHlpStreamStatusIsReady(pStream->fStatus))
         {
             rc = VERR_AUDIO_STREAM_NOT_READY;
+            break;
+        }
+
+        /* If output is disabled on a per-driver level, send data to the bit bucket instead. See #9882. */
+        if (!pThis->Out.fEnabled)
+        {
+            fToBitBucket = true;
             break;
         }
@@ -2390 +2418 @@
     do
     {
-        if (   !pThis->In.fEnabled
-            || !DrvAudioHlpStreamStatusCanRead(pStream->fStatus))
-        {
-            rc = VERR_AUDIO_STREAM_NOT_READY;
-            break;
-        }
-
-        /*
-         * Read from the parent buffer (that is, the guest buffer) which
-         * should have the audio data in the format the guest needs.
-         */
         uint32_t cfReadTotal = 0;
 
         const uint32_t cfBuf = AUDIOMIXBUF_B2F(&pStream->Guest.MixBuf, cbBuf);
 
-        uint32_t cfToRead = RT_MIN(cfBuf, AudioMixBufLive(&pStream->Guest.MixBuf));
-        while (cfToRead)
-        {
-            uint32_t cfRead;
-            rc = AudioMixBufAcquireReadBlock(&pStream->Guest.MixBuf,
-                                             (uint8_t *)pvBuf + AUDIOMIXBUF_F2B(&pStream->Guest.MixBuf, cfReadTotal),
-                                             AUDIOMIXBUF_F2B(&pStream->Guest.MixBuf, cfToRead), &cfRead);
-            if (RT_FAILURE(rc))
+        if (pThis->In.fEnabled) /* Input for this audio driver enabled? See #9822. */
+        {
+            if (!DrvAudioHlpStreamStatusCanRead(pStream->fStatus))
+            {
+                rc = VERR_AUDIO_STREAM_NOT_READY;
                 break;
+            }
+
+            /*
+             * Read from the parent buffer (that is, the guest buffer) which
+             * should have the audio data in the format the guest needs.
+             */
+            uint32_t cfToRead = RT_MIN(cfBuf, AudioMixBufLive(&pStream->Guest.MixBuf));
+            while (cfToRead)
+            {
+                uint32_t cfRead;
+                rc = AudioMixBufAcquireReadBlock(&pStream->Guest.MixBuf,
+                                                 (uint8_t *)pvBuf + AUDIOMIXBUF_F2B(&pStream->Guest.MixBuf, cfReadTotal),
+                                                 AUDIOMIXBUF_F2B(&pStream->Guest.MixBuf, cfToRead), &cfRead);
+                if (RT_FAILURE(rc))
+                    break;
 
 #ifdef VBOX_WITH_STATISTICS
-            const uint32_t cbRead = AUDIOMIXBUF_F2B(&pStream->Guest.MixBuf, cfRead);
-
-            STAM_COUNTER_ADD(&pThis->Stats.TotalBytesRead,       cbRead);
-
-            STAM_COUNTER_ADD(&pStream->In.Stats.TotalFramesRead, cfRead);
-            STAM_COUNTER_INC(&pStream->In.Stats.TotalTimesRead);
+                const uint32_t cbRead = AUDIOMIXBUF_F2B(&pStream->Guest.MixBuf, cfRead);
+
+                STAM_COUNTER_ADD(&pThis->Stats.TotalBytesRead,       cbRead);
+
+                STAM_COUNTER_ADD(&pStream->In.Stats.TotalFramesRead, cfRead);
+                STAM_COUNTER_INC(&pStream->In.Stats.TotalTimesRead);
 #endif
-            Assert(cfToRead >= cfRead);
-            cfToRead -= cfRead;
-
-            cfReadTotal += cfRead;
-
-            AudioMixBufReleaseReadBlock(&pStream->Guest.MixBuf, cfRead);
-        }
-
-        if (cfReadTotal)
-        {
-            if (pThis->In.Cfg.Dbg.fEnabled)
-                DrvAudioHlpFileWrite(pStream->In.Dbg.pFileStreamRead,
-                                     pvBuf, AUDIOMIXBUF_F2B(&pStream->Guest.MixBuf, cfReadTotal), 0 /* fFlags */);
-
-            AudioMixBufFinish(&pStream->Guest.MixBuf, cfReadTotal);
+                Assert(cfToRead >= cfRead);
+                cfToRead -= cfRead;
+
+                cfReadTotal += cfRead;
+
+                AudioMixBufReleaseReadBlock(&pStream->Guest.MixBuf, cfRead);
+            }
+
+            if (cfReadTotal)
+            {
+                if (pThis->In.Cfg.Dbg.fEnabled)
+                    DrvAudioHlpFileWrite(pStream->In.Dbg.pFileStreamRead,
+                                         pvBuf, AUDIOMIXBUF_F2B(&pStream->Guest.MixBuf, cfReadTotal), 0 /* fFlags */);
+
+                AudioMixBufFinish(&pStream->Guest.MixBuf, cfReadTotal);
+            }
         }
 
@@ -2719 +2749 @@
                 fEnable ? "Enabling" : "Disabling", enmDir == PDMAUDIODIR_IN ? "input" : "output", pThis->szName));
 
+        /* Update the status first, as this will be checked for in drvAudioStreamControlInternalBackend() below. */
+        *pfEnabled = fEnable;
+
         PPDMAUDIOSTREAM pStream;
         RTListForEach(&pThis->lstStreams, pStream, PDMAUDIOSTREAM, Node)
@@ -2725 +2758 @@
                 continue;
 
-            int rc2 = drvAudioStreamControlInternal(pThis, pStream,
-                                                    fEnable ? PDMAUDIOSTREAMCMD_ENABLE : PDMAUDIOSTREAMCMD_DISABLE);
+            /* Note: Only enable / disable the backend, do *not* change the stream's internal status.
+             *       Callers (device emulation, mixer, ...) from outside will not see any status or behavior change,
+             *       to not confuse the rest of the state machine.
+             *
+             *       When disabling:
+             *          - playing back audo data would go to /dev/null
+             *          - recording audio data would return silence instead
+             *
+             * See #9882.
+             */
+            int rc2 = drvAudioStreamControlInternalBackend(pThis, pStream,
+                                                           fEnable ? PDMAUDIOSTREAMCMD_ENABLE : PDMAUDIOSTREAMCMD_DISABLE);
             if (RT_FAILURE(rc2))
             {
@@ -2743 +2786 @@
             /* Keep going. */
         }
-
-        *pfEnabled = fEnable;
     }
 
@@ -2864 +2905 @@
     uint32_t cbReadable = 0;
 
+    /* All input streams for this driver disabled? See #9882. */
+    const bool fDisabled = !pThis->In.fEnabled;
+
     if (   pThis->pHostDrvAudio
-        && DrvAudioHlpStreamStatusCanRead(pStream->fStatus))
+        && (   DrvAudioHlpStreamStatusCanRead(pStream->fStatus)
+            || fDisabled)
+       )
     {
         const uint32_t cfReadable = AudioMixBufLive(&pStream->Guest.MixBuf);
@@ -2881 +2927 @@
              * Reading the actual data from a stream then will return silence then.
              */
-            if (!DrvAudioHlpStreamStatusCanRead(
-                pThis->pHostDrvAudio->pfnStreamGetStatus(pThis->pHostDrvAudio, pStream->pvBackend)))
+            if (  !DrvAudioHlpStreamStatusCanRead(
+                      pThis->pHostDrvAudio->pfnStreamGetStatus(pThis->pHostDrvAudio, pStream->pvBackend)
+                || fDisabled))
             {
                 cbReadable = DrvAudioHlpNanoToBytes(RTTimeNanoTS() - pStream->tsLastReadWrittenNs,
                                                     &pStream->Host.Cfg.Props);
-                Log3Func(("[%s] Backend stream not ready, returning silence\n", pStream->szName));
+                Log3Func(("[%s] Backend stream not ready or driver has disabled audio input, returning silence\n", pStream->szName));
             }
         }