Changeset 70594 in vbox

Timestamp:

Jan 15, 2018 4:35:07 PM (7 years ago)

Author:

vboxsync

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

120302

Message:

Audio/HDA: Fixes for Windows guest surround setups.

Location:

trunk/src/VBox/Devices/Audio

Files:

• HDAStream.cpp (modified) (10 diffs)
• HDAStream.h (modified) (1 diff)

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

@@ -217 +217 @@
     }
 
+    if (   !pStream->u32CBL
+        || !pStream->u16LVI
+        || !pStream->u64BDLBase
+        || !pStream->u16FIFOS)
+    {
+        return VINF_SUCCESS;
+    }
+
     /* Set the stream's frame size. */
     pStream->State.cbFrameSize = pCfg->Props.cChannels * (pCfg->Props.cBits / 8 /* To bytes */);
+    LogFunc(("[SD%RU8] cChannels=%RU8, cBits=%RU8 -> cbFrameSize=%RU32\n",
+             pStream->u8SD, pCfg->Props.cChannels, pCfg->Props.cBits, pStream->State.cbFrameSize));
+    Assert(pStream->State.cbFrameSize); /* Frame size must not be 0. */
 
     /*
@@ -235 +246 @@
              pStream->State.Cfg.Props.uHz, rc));
 
-    if (   pStream->u32CBL
-        && pStream->u16LVI)
+    /* Make sure that mandatory parameters are set up correctly. */
+    AssertStmt(pStream->u32CBL %  pStream->State.cbFrameSize == 0, rc = VERR_INVALID_PARAMETER);
+    AssertStmt(pStream->u16LVI >= 1,                               rc = VERR_INVALID_PARAMETER);
+
+    if (RT_SUCCESS(rc))
     {
         /* Make sure that the chosen Hz rate dividable by the stream's rate. */
@@ -263 +277 @@
             RT_ZERO(BDLE);
 
-            Assert(pStream->u64BDLBase);
-
             int rc2 = hdaBDLEFetch(pThis, &BDLE, pStream->u64BDLBase, 0 /* Entry */);
             AssertRC(rc2);
 
-            Assert(BDLE.Desc.u32BufSize % pStream->State.cbFrameSize == 0);
+            /* Note: Do *not* check if this BDLE aligns to the stream's frame size.
+             *       It can happen that this isn't the case on some guests, e.g.
+             *       on Windows with a 5.1 speaker setup.
+             *
+             *       The only thing which counts is that the stream's CBL value
+             *       properly aligns to the stream's frame size.
+             */
 
             /* If no custom set position adjustment is set, apply some
@@ -340 +358 @@
         pStream->State.cbTransferProcessed        = 0;
         pStream->State.cTransferPendingInterrupts = 0;
+        pStream->State.cbDMALeft                  = 0;
 
         const uint64_t cTicksPerHz = TMTimerGetFreq(pThis->pTimer) / pThis->u16TimerHz;
@@ -366 +385 @@
 #endif
     }
+
+    if (RT_FAILURE(rc))
+        LogRel(("HDA: Initializing stream #%RU8 failed with %Rrc\n", rc));
 
     return rc;
@@ -537 +559 @@
     AssertPtrReturnVoid(pStream);
 
-    Assert(u32LPIB % pStream->State.cbFrameSize == 0);
-
     Log3Func(("[SD%RU8] LPIB=%RU32 (DMA Position Buffer Enabled: %RTbool)\n",
               pStream->u8SD, u32LPIB, pStream->pHDAState->fDMAPosition));
@@ -937 +957 @@
             if (RT_SUCCESS(rc))
             {
-                uint32_t cbDMAWritten = 0;
-                uint32_t cbDMALeft    = cbDMA;
-
-                if (cbDMALeft > RTCircBufFree(pCircBuf))
-                {
-                    LogRel2(("HDA: FIFO overflow for stream #%RU8 (%RU32 bytes outstanding)\n",
-                             pStream->u8SD, cbDMALeft - RTCircBufFree(pCircBuf)));
-
-                    /* Try to read as much as possible. */
-                    cbDMALeft = (uint32_t)RTCircBufFree(pCircBuf);
-
-                    rc = VERR_BUFFER_OVERFLOW;
-                }
+                uint32_t cbDMARead = pStream->State.cbDMALeft ? pStream->State.cbFrameSize - pStream->State.cbDMALeft : 0;
+                uint32_t cbDMALeft = RT_MIN(cbDMA, RTCircBufFree(pCircBuf));
 
 #ifndef VBOX_WITH_HDA_AUDIO_INTERLEAVING_STREAMS_SUPPORT
                 /**
                  * The following code extracts the required audio stream (channel) data
-                 * of non-interleaved  *and* interleaved audio streams.
+                 * of non-interleaved *and* interleaved audio streams.
                  *
                  * We by default only support 2 channels with 16-bit samples (HDA_FRAME_SIZE),
@@ -962 +971 @@
                  * So take this into account by just handling the first channel in such a stream ("A")
                  * and just discard the other channel's data.
+                 *
+                 * @todo Optimize this stuff -- copying only one frame a time is expensive.
                  */
-                while (cbDMALeft)
+                while (cbDMALeft >= pStream->State.cbFrameSize)
                 {
                     void *pvBuf; size_t cbBuf;
                     RTCircBufAcquireWriteBlock(pCircBuf, HDA_FRAME_SIZE, &pvBuf, &cbBuf);
 
+                    AssertBreak(cbDMARead <= sizeof(abChunk));
+
                     if (cbBuf)
-                        memcpy(pvBuf, abChunk + cbDMAWritten, cbBuf);
+                        memcpy(pvBuf, abChunk + cbDMARead, cbBuf);
 
                     RTCircBufReleaseWriteBlock(pCircBuf, cbBuf);
 
-                    cbDMALeft    -= (uint32_t)pStream->State.cbFrameSize;
-                    cbDMAWritten += (uint32_t)pStream->State.cbFrameSize;
+                    Assert(cbDMALeft >= pStream->State.cbFrameSize);
+                    cbDMALeft -= pStream->State.cbFrameSize;
+                    cbDMARead += pStream->State.cbFrameSize;
                 }
+
+                pStream->State.cbDMALeft = cbDMALeft;
+                Assert(pStream->State.cbDMALeft < pStream->State.cbFrameSize);
+
+                const size_t cbFree = RTCircBufFree(pCircBuf);
+                if (!cbFree)
+                    LogRel2(("HDA: FIFO of stream #%RU8 full, discarding audio data\n", pStream->u8SD));
 #else
                 /** @todo This needs making use of HDAStreamMap + HDAStreamChannel. */
@@ -992 +1013 @@
         if (cbDMA)
         {
-            Assert(cbDMA % pStream->State.cbFrameSize == 0);
-
             /* We always increment the position of DMA buffer counter because we're always reading
              * into an intermediate buffer. */
@@ -1069 +1088 @@
 
     /* Sanity. */
-    Assert(cbProcessed % pStream->State.cbFrameSize == 0);
     Assert(cbProcessed == cbToProcess);
     Assert(cbLeft      == 0);

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

@@ -171 +171 @@
     RTLISTANCHORR3          lstDMAHandlers;
 #endif
+    /** How much DMA data from a previous transfer is left to be processed (in bytes).
+     *  This can happen if the stream's frame size is bigger (e.g. 16 bytes) than
+     *  the current DMA FIFO can hold (e.g. 10 bytes). Mostly needed for more complex
+     *  stuff like interleaved surround streams. */
+    uint16_t                cbDMALeft;
     /** Unused, padding. */
-    uint8_t                 Padding3[3];
+    uint8_t                 Padding3;
 } HDASTREAMSTATE, *PHDASTREAMSTATE;