Changeset 89779 in vbox for trunk/src/VBox/Devices/Audio

Timestamp:

Jun 18, 2021 2:02:38 PM (4 years ago)

Author:

vboxsync

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

145232

Message:

Audio: Size the mixer buffer for sinks according to the scheulding hint. This should typically reduce the buffer size from the fixed size of 100ms we've used so far, helping to reduce latency build up. Also, 100ms might not have been enough for some configurations. bugref:9890

Location:

trunk/src/VBox/Devices/Audio

Files:

• AudioMixer.cpp (modified) (5 diffs)
• AudioMixer.h (modified) (1 diff)
• DevHda.cpp (modified) (1 diff)
• DevIchAc97.cpp (modified) (1 diff)
• DevSB16.cpp (modified) (1 diff)

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

@@ -945 +945 @@
  *
  * @returns VBox status code.
- * @param   pSink   The sink to set audio format for.
- * @param   pProps  The properties of the new audio format (guest side).
- */
-int AudioMixerSinkSetFormat(PAUDMIXSINK pSink, PCPDMAUDIOPCMPROPS pProps)
+ * @param   pSink               The sink to set audio format for.
+ * @param   pProps              The properties of the new audio format (guest side).
+ * @param   cMsSchedulingHint   Scheduling hint for mixer buffer sizing.
+ */
+int AudioMixerSinkSetFormat(PAUDMIXSINK pSink, PCPDMAUDIOPCMPROPS pProps, uint32_t cMsSchedulingHint)
 {
     AssertPtrReturn(pSink, VERR_INVALID_POINTER);
@@ -955 +956 @@
     AssertReturn(AudioHlpPcmPropsAreValid(pProps), VERR_INVALID_PARAMETER);
 
+    /*
+     * Calculate the mixer buffer size so we can force a recreation if it changes.
+     *
+     * This used to be fixed at 100ms, however that's usually too generous and can
+     * in theory be too small.  Generally, we size the buffer at 3 DMA periods as
+     * that seems reasonable.  Now, since the we don't quite trust the scheduling
+     * hint we're getting, make sure we're got a minimum of 30ms buffer space, but
+     * no more than 500ms.
+     */
+    if (cMsSchedulingHint <= 10)
+        cMsSchedulingHint = 30;
+    else
+    {
+        cMsSchedulingHint *= 3;
+        if (cMsSchedulingHint > 500)
+            cMsSchedulingHint = 500;
+    }
+    uint32_t const cBufferFrames = PDMAudioPropsMilliToFrames(pProps, cMsSchedulingHint);
+     /** @todo configuration override on the buffer size? */
+
     int rc = RTCritSectEnter(&pSink->CritSect);
     AssertRCReturn(rc, rc);
@@ -960 +981 @@
     /*
      * Do nothing unless the format actually changed.
-     */
-    if (!PDMAudioPropsAreEqual(&pSink->PCMProps, pProps))
+     * The buffer size must not match exactly, within +/- 2% is okay.
+     */
+    uint32_t cOldBufferFrames;
+    if (   !PDMAudioPropsAreEqual(&pSink->PCMProps, pProps)
+        || (   cBufferFrames != (cOldBufferFrames = AudioMixBufSize(&pSink->MixBuf))
+            && (uint32_t)RT_ABS((int32_t)(cBufferFrames - cOldBufferFrames)) > cBufferFrames / 50) )
     {
 #ifdef LOG_ENABLED
@@ -967 +992 @@
 #endif
         if (PDMAudioPropsHz(&pSink->PCMProps) != 0)
-            LogFlowFunc(("[%s] Old format: %s\n", pSink->pszName, PDMAudioPropsToString(&pSink->PCMProps, szTmp, sizeof(szTmp)) ));
+            LogFlowFunc(("[%s] Old format: %s; buffer: %u frames\n", pSink->pszName,
+                         PDMAudioPropsToString(&pSink->PCMProps, szTmp, sizeof(szTmp)), AudioMixBufSize(&pSink->MixBuf) ));
         pSink->PCMProps = *pProps;
-        LogFlowFunc(("[%s] New format: %s\n", pSink->pszName, PDMAudioPropsToString(&pSink->PCMProps, szTmp, sizeof(szTmp)) ));
+        LogFlowFunc(("[%s] New format: %s; buffer: %u frames\n", pSink->pszName,
+                     PDMAudioPropsToString(&pSink->PCMProps, szTmp, sizeof(szTmp)), cBufferFrames ));
 
         /*
@@ -976 +1003 @@
         AudioMixBufTerm(&pSink->MixBuf);
 
-        /** @todo r=bird: Make sure we've got more room here than what's expected to
-         *        be moved in one guest DMA period. */
-        rc = AudioMixBufInit(&pSink->MixBuf, pSink->pszName, &pSink->PCMProps,
-                             PDMAudioPropsMilliToFrames(&pSink->PCMProps, 100 /*ms*/)); /** @todo Make this configurable? */
+        rc = AudioMixBufInit(&pSink->MixBuf, pSink->pszName, &pSink->PCMProps, cBufferFrames);
         if (RT_SUCCESS(rc))
         {

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

@@ -298 +298 @@
 bool        AudioMixerSinkIsActive(PAUDMIXSINK pSink);
 void        AudioMixerSinkReset(PAUDMIXSINK pSink);
-int         AudioMixerSinkSetFormat(PAUDMIXSINK pSink, PCPDMAUDIOPCMPROPS pPCMProps);
+int         AudioMixerSinkSetFormat(PAUDMIXSINK pSink, PCPDMAUDIOPCMPROPS pPCMProps, uint32_t cMsSchedulingHint);
 int         AudioMixerSinkSetVolume(PAUDMIXSINK pSink, PCPDMAUDIOVOLUME pVol);
 int         AudioMixerSinkUpdate(PAUDMIXSINK pSink, uint32_t cbDmaUsed, uint32_t cbDmaPeriod);

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

@@ -2348 +2348 @@
     LogFunc(("Sink=%s, Stream=%s\n", pMixSink->pszName, pCfg->szName));
 
-    if (!AudioHlpStreamCfgIsValid(pCfg))
-        return VERR_INVALID_PARAMETER;
-
-    int rc = AudioMixerSinkSetFormat(pMixSink, &pCfg->Props);
-    if (RT_SUCCESS(rc))
-    {
-        PHDADRIVER pDrv;
-        RTListForEach(&pThisCC->lstDrv, pDrv, HDADRIVER, Node)
-        {
-            /* We ignore failures here because one non-working driver shouldn't
-               be allowed to spoil it for everyone else. */
-            int rc2 = hdaR3MixerAddDrvStream(pDevIns, pMixSink, pCfg, pDrv);
-            if (RT_FAILURE(rc2))
-                LogFunc(("Attaching stream failed with %Rrc (ignored)\n", rc2));
-        }
-    }
+    int rc;
+    if (AudioHlpStreamCfgIsValid(pCfg))
+    {
+        rc = AudioMixerSinkSetFormat(pMixSink, &pCfg->Props, pCfg->Device.cMsSchedulingHint);
+        if (RT_SUCCESS(rc))
+        {
+            PHDADRIVER pDrv;
+            RTListForEach(&pThisCC->lstDrv, pDrv, HDADRIVER, Node)
+            {
+                /* We ignore failures here because one non-working driver shouldn't
+                   be allowed to spoil it for everyone else. */
+                int rc2 = hdaR3MixerAddDrvStream(pDevIns, pMixSink, pCfg, pDrv);
+                if (RT_FAILURE(rc2))
+                    LogFunc(("Attaching stream failed with %Rrc (ignored)\n", rc2));
+            }
+        }
+    }
+    else
+        rc = VERR_INVALID_PARAMETER;
     return rc;
 }

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

@@ -1839 +1839 @@
     if (AudioHlpStreamCfgIsValid(pCfg))
     {
-        rc = AudioMixerSinkSetFormat(pMixSink, &pCfg->Props);
+        rc = AudioMixerSinkSetFormat(pMixSink, &pCfg->Props, pCfg->Device.cMsSchedulingHint);
         if (RT_SUCCESS(rc))
         {

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

@@ -1795 +1795 @@
     AssertPtrReturn(pMixSink, VERR_INVALID_POINTER);
 
-    if (!AudioHlpStreamCfgIsValid(pCfg))
-        return VERR_INVALID_PARAMETER;
-
-    int rc = AudioMixerSinkSetFormat(pMixSink, &pCfg->Props);
-    if (RT_FAILURE(rc))
-        return rc;
-
-    PSB16DRIVER pDrv;
-    RTListForEach(&pThis->lstDrv, pDrv, SB16DRIVER, Node)
-    {
-        int rc2 = sb16AddDrvStream(pDevIns, pMixSink, pCfg, pDrv);
-        if (RT_FAILURE(rc2))
-            LogFunc(("Attaching stream failed with %Rrc\n", rc2));
-
-        /* Do not pass failure to rc here, as there might be drivers which aren't
-         * configured / ready yet. */
-    }
+    int rc;
+    if (AudioHlpStreamCfgIsValid(pCfg))
+    {
+        rc = AudioMixerSinkSetFormat(pMixSink, &pCfg->Props, pCfg->Device.cMsSchedulingHint);
+        if (RT_SUCCESS(rc))
+        {
+            PSB16DRIVER pDrv;
+            RTListForEach(&pThis->lstDrv, pDrv, SB16DRIVER, Node)
+            {
+                int rc2 = sb16AddDrvStream(pDevIns, pMixSink, pCfg, pDrv);
+                if (RT_FAILURE(rc2))
+                    LogFunc(("Attaching stream failed with %Rrc\n", rc2));
+
+                /* Do not pass failure to rc here, as there might be drivers which aren't
+                 * configured / ready yet. */
+            }
+        }
+    }
+    else
+        rc = VERR_INVALID_PARAMETER;
 
     LogFlowFuncLeaveRC(rc);