diff options
author | Daniel Baumann <daniel.baumann@progress-linux.org> | 2024-04-16 22:55:46 +0000 |
---|---|---|
committer | Daniel Baumann <daniel.baumann@progress-linux.org> | 2024-04-16 22:55:46 +0000 |
commit | 96647a898d62d699808316238dfb933d960413f2 (patch) | |
tree | 0138491ada40b7b3fcb80d4b219fa7922ae8f512 /src/VBox/Devices/Audio | |
parent | Adding debian version 7.0.14-dfsg-4. (diff) | |
download | virtualbox-96647a898d62d699808316238dfb933d960413f2.tar.xz virtualbox-96647a898d62d699808316238dfb933d960413f2.zip |
Merging upstream version 7.0.16-dfsg.
Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>
Diffstat (limited to 'src/VBox/Devices/Audio')
-rw-r--r-- | src/VBox/Devices/Audio/AudioMixer.cpp | 11 | ||||
-rw-r--r-- | src/VBox/Devices/Audio/DevHdaStream.cpp | 60 | ||||
-rw-r--r-- | src/VBox/Devices/Audio/DevIchAc97.cpp | 11 | ||||
-rw-r--r-- | src/VBox/Devices/Audio/DevSB16.cpp | 159 |
4 files changed, 179 insertions, 62 deletions
diff --git a/src/VBox/Devices/Audio/AudioMixer.cpp b/src/VBox/Devices/Audio/AudioMixer.cpp index 90d86063..899154eb 100644 --- a/src/VBox/Devices/Audio/AudioMixer.cpp +++ b/src/VBox/Devices/Audio/AudioMixer.cpp @@ -2011,6 +2011,9 @@ uint64_t AudioMixerSinkTransferFromCircBuf(PAUDMIXSINK pSink, PRTCIRCBUF pCircBu Assert(pSink->enmDir == PDMAUDIODIR_OUT); RT_NOREF(idStream); + int rc = RTCritSectEnter(&pSink->CritSect); + AssertRCReturn(rc, rc); + /* * Figure how much that we can push down. */ @@ -2022,7 +2025,8 @@ uint64_t AudioMixerSinkTransferFromCircBuf(PAUDMIXSINK pSink, PRTCIRCBUF pCircBu Log3Func(("idStream=%u: cbSinkWritable=%#RX32 cbCircBufReadable=%#RX32 -> cbToTransfer=%#RX32 @%#RX64\n", idStream, cbSinkWritable, cbCircBufReadable, cbToTransfer, offStream)); - AssertMsg(!(pSink->fStatus & AUDMIXSINK_STS_DRAINING) || cbCircBufReadable == pSink->cbDmaLeftToDrain, + /* Note: There now can be more data in the DMA buffer than initially announced. See @bugref{10354}. */ + AssertMsg(!(pSink->fStatus & AUDMIXSINK_STS_DRAINING) || cbCircBufReadable >= pSink->cbDmaLeftToDrain, ("cbCircBufReadable=%#x cbDmaLeftToDrain=%#x\n", cbCircBufReadable, pSink->cbDmaLeftToDrain)); /* @@ -2080,6 +2084,11 @@ uint64_t AudioMixerSinkTransferFromCircBuf(PAUDMIXSINK pSink, PRTCIRCBUF pCircBu else Assert(cbToTransfer2 == 0); + Log3Func(("idStream=%u: cbCircBufUsed=%RX32 left\n", idStream, (uint32_t)RTCircBufUsed(pCircBuf))); + + RTCritSectLeave(&pSink->CritSect); + + LogFlowFuncLeave(); return offStream; } diff --git a/src/VBox/Devices/Audio/DevHdaStream.cpp b/src/VBox/Devices/Audio/DevHdaStream.cpp index aa6b0535..9bb27a4a 100644 --- a/src/VBox/Devices/Audio/DevHdaStream.cpp +++ b/src/VBox/Devices/Audio/DevHdaStream.cpp @@ -797,18 +797,6 @@ int hdaR3StreamSetUp(PPDMDEVINS pDevIns, PHDASTATE pThis, PHDASTREAM pStreamShar * Set up internal ring buffer. */ - /* (Re-)Allocate the stream's internal DMA buffer, - * based on the timing *and* PCM properties we just got above. */ - if (pStreamR3->State.pCircBuf) - { - RTCircBufDestroy(pStreamR3->State.pCircBuf); - pStreamR3->State.pCircBuf = NULL; - pStreamR3->State.StatDmaBufSize = 0; - pStreamR3->State.StatDmaBufUsed = 0; - } - pStreamShared->State.offWrite = 0; - pStreamShared->State.offRead = 0; - /* * The default internal ring buffer size must be: * @@ -857,16 +845,37 @@ int hdaR3StreamSetUp(PPDMDEVINS pDevIns, PHDASTATE pThis, PHDASTREAM pStreamShar rc = VERR_INVALID_PARAMETER); if (RT_SUCCESS(rc)) { - rc = RTCircBufCreate(&pStreamR3->State.pCircBuf, cbCircBuf); - if (RT_SUCCESS(rc)) + /** + * Note: Only re-create the DMA buffer if the size actually has changed. + * + * Otherwise do *not* reset the stream's circular buffer here, as the audio mixer still relies on + * previously announced DMA data (via AudioMixerSinkDrainAndStop()) and processes it asynchronously. + * Resetting the buffer here will cause a race condition. See @bugref{10354}. */ + if (pStreamR3->State.StatDmaBufSize != cbCircBuf) { - pStreamR3->State.StatDmaBufSize = cbCircBuf; + /* (Re-)Allocate the stream's internal DMA buffer, + * based on the timing *and* PCM properties we just got above. */ + if (pStreamR3->State.pCircBuf) + { + RTCircBufDestroy(pStreamR3->State.pCircBuf); + pStreamR3->State.pCircBuf = NULL; + pStreamR3->State.StatDmaBufSize = 0; + pStreamR3->State.StatDmaBufUsed = 0; + } + pStreamShared->State.offWrite = 0; + pStreamShared->State.offRead = 0; - /* - * Forward the timer frequency hint to TM as well for better accuracy on - * systems w/o preemption timers (also good for 'info timers'). - */ - PDMDevHlpTimerSetFrequencyHint(pDevIns, pStreamShared->hTimer, uTransferHz); + rc = RTCircBufCreate(&pStreamR3->State.pCircBuf, cbCircBuf); + if (RT_SUCCESS(rc)) + { + pStreamR3->State.StatDmaBufSize = cbCircBuf; + + /* + * Forward the timer frequency hint to TM as well for better accuracy on + * systems w/o preemption timers (also good for 'info timers'). + */ + PDMDevHlpTimerSetFrequencyHint(pDevIns, pStreamShared->hTimer, uTransferHz); + } } } @@ -1003,10 +1012,9 @@ void hdaR3StreamReset(PHDASTATE pThis, PHDASTATER3 pThisCC, PHDASTREAM pStreamSh pStreamShared->State.idxScheduleLoop = 0; pStreamShared->State.fInputPreBuffered = false; - if (pStreamR3->State.pCircBuf) - RTCircBufReset(pStreamR3->State.pCircBuf); - pStreamShared->State.offWrite = 0; - pStreamShared->State.offRead = 0; + /* Note: Do *not* reset the stream's circular buffer here, as the audio mixer still relies on + * previously announced DMA data (via AudioMixerSinkDrainAndStop()) and processes it asynchronously. + * Resetting the buffer here will cause a race condition. See @bugref{10354}. */ /* Report that we're done resetting this stream. */ HDA_STREAM_REG(pThis, CTL, uSD) = 0; @@ -1041,6 +1049,8 @@ int hdaR3StreamEnable(PHDASTATE pThis, PHDASTREAM pStreamShared, PHDASTREAMR3 pS PAUDMIXSINK const pSink = pStreamR3->pMixSink ? pStreamR3->pMixSink->pMixSink : NULL; if (pSink) { + AudioMixerSinkLock(pSink); + if (fEnable) { if (pStreamR3->State.pAioRegSink != pSink) @@ -1060,6 +1070,8 @@ int hdaR3StreamEnable(PHDASTATE pThis, PHDASTREAM pStreamShared, PHDASTREAMR3 pS else rc = AudioMixerSinkDrainAndStop(pSink, pStreamR3->State.pCircBuf ? (uint32_t)RTCircBufUsed(pStreamR3->State.pCircBuf) : 0); + + AudioMixerSinkUnlock(pSink); } if ( RT_SUCCESS(rc) && fEnable diff --git a/src/VBox/Devices/Audio/DevIchAc97.cpp b/src/VBox/Devices/Audio/DevIchAc97.cpp index 97743372..969e12db 100644 --- a/src/VBox/Devices/Audio/DevIchAc97.cpp +++ b/src/VBox/Devices/Audio/DevIchAc97.cpp @@ -1730,8 +1730,9 @@ static void ichac97R3StreamReset(PAC97STATE pThis, PAC97STREAM pStream, PAC97STR LogFunc(("[SD%RU8]\n", pStream->u8SD)); - if (pStreamCC->State.pCircBuf) - RTCircBufReset(pStreamCC->State.pCircBuf); + /* Note: Do *not* reset the stream's circular buffer here, as the audio mixer still relies on + * previously announced DMA data (via AudioMixerSinkDrainAndStop()) and processes it asynchronously. + * Resetting the buffer here will cause a race condition. See @bugref{10354}. */ pStream->Regs.bdbar = 0; pStream->Regs.civ = 0; @@ -2198,7 +2199,11 @@ static int ichac97R3StreamSetUp(PPDMDEVINS pDevIns, PAC97STATE pThis, PAC97STATE */ if ( pStreamCC->State.pCircBuf && RTCircBufSize(pStreamCC->State.pCircBuf) == cbCircBuf) - RTCircBufReset(pStreamCC->State.pCircBuf); + { + /* Note: Do *not* reset the stream's circular buffer here, as the audio mixer still relies on + * previously announced DMA data (via AudioMixerSinkDrainAndStop()) and processes it asynchronously. + * Resetting the buffer here will cause a race condition. See @bugref{10354}. */ + } else { if (pStreamCC->State.pCircBuf) diff --git a/src/VBox/Devices/Audio/DevSB16.cpp b/src/VBox/Devices/Audio/DevSB16.cpp index a405f311..7df081e7 100644 --- a/src/VBox/Devices/Audio/DevSB16.cpp +++ b/src/VBox/Devices/Audio/DevSB16.cpp @@ -110,6 +110,8 @@ typedef struct SB16STATE *PSB16STATE; */ typedef struct SB16STREAMSTATE { + /** Critical section for this stream. */ + RTCRITSECT CritSect; /** Flag indicating whether this stream is in enabled state or not. */ bool fEnabled; /** Set if we've registered the asynchronous update job. */ @@ -335,6 +337,8 @@ static int sb16StreamEnable(PSB16STATE pThis, PSB16STREAM pStream, bool fEnable static void sb16StreamReset(PSB16STATE pThis, PSB16STREAM pStream); static int sb16StreamOpen(PPDMDEVINS pDevIns, PSB16STATE pThis, PSB16STREAM pStream); static void sb16StreamClose(PPDMDEVINS pDevIns, PSB16STATE pThis, PSB16STREAM pStream); +DECLINLINE(void) sb16StreamLock(PSB16STREAM pStream); +DECLINLINE(void) sb16StreamUnlock(PSB16STREAM pStream); DECLINLINE(PAUDMIXSINK) sb16StreamIndexToSink(PSB16STATE pThis, uint8_t uIdx); static void sb16StreamTransferScheduleNext(PSB16STATE pThis, PSB16STREAM pStream, uint32_t cSamples); static int sb16StreamDoDmaOutput(PSB16STATE pThis, PSB16STREAM pStream, int uDmaChan, uint32_t offDma, uint32_t cbDma, uint32_t cbToRead, uint32_t *pcbRead); @@ -1923,6 +1927,20 @@ static void sb16RemoveDrv(PPDMDEVINS pDevIns, PSB16STATE pThis, PSB16DRIVER pDrv * Stream handling * *********************************************************************************************************************************/ +/** + * Reads DMA output data into the internal DMA buffer. + * + * @returns VBox status code. + * @param pThis The SB16 state. + * @param pStream The SB16 stream to read DMA output data from. + * @param uDmaChan DMA channel to read from. + * @param offDma DMA offset (in bytes) to start reading from. + * @param cbDma DMA area size in bytes. + * @param cbToRead How much bytes to read in total. + * @param pcbRead Where to return the DMA bytes read. + * + * @thread EMT + */ static int sb16StreamDoDmaOutput(PSB16STATE pThis, PSB16STREAM pStream, int uDmaChan, uint32_t offDma, uint32_t cbDma, uint32_t cbToRead, uint32_t *pcbRead) { @@ -1930,6 +1948,8 @@ static int sb16StreamDoDmaOutput(PSB16STATE pThis, PSB16STREAM pStream, int uDma //Assert(cbToRead <= cbFree); /** @todo Add statistics for overflows. */ cbToRead = RT_MIN(cbToRead, cbFree); + sb16StreamLock(pStream); + uint32_t cbReadTotal = 0; while (cbToRead) { @@ -1948,6 +1968,8 @@ static int sb16StreamDoDmaOutput(PSB16STATE pThis, PSB16STREAM pStream, int uDma if (cbReadTotal > 0) break; *pcbRead = 0; + + sb16StreamUnlock(pStream); return rc; } @@ -1956,8 +1978,12 @@ static int sb16StreamDoDmaOutput(PSB16STATE pThis, PSB16STREAM pStream, int uDma else AudioHlpFileWrite(pStream->Dbg.Runtime.pFileDMA, pv, cbRead); + Log3Func(("Writing %#RX32 bytes to DMA buffer\n", cbRead)); + RTCircBufReleaseWriteBlock(pStream->State.pCircBuf, cbRead); + Log3Func(("Now %#RX32 bytes in buffer\n", RTCircBufUsed(pStream->State.pCircBuf))); + Assert(cbToRead >= cbRead); pStream->State.offWrite += cbRead; offDma = (offDma + cbRead) % cbDma; @@ -1970,6 +1996,9 @@ static int sb16StreamDoDmaOutput(PSB16STATE pThis, PSB16STREAM pStream, int uDma /* Update buffer stats. */ pStream->State.StatDmaBufUsed = (uint32_t)RTCircBufUsed(pStream->State.pCircBuf); + Log3Func(("[SD%RU8] cbCircBufUsed=%#RX64\n", pStream->uIdx, RTCircBufUsed(pStream->State.pCircBuf))); + + sb16StreamUnlock(pStream); return VINF_SUCCESS; } @@ -2011,8 +2040,19 @@ static int sb16StreamEnable(PSB16STATE pThis, PSB16STREAM pStream, bool fEnable, } else { - rc = AudioMixerSinkDrainAndStop(pSink, pStream->State.pCircBuf ? (uint32_t)RTCircBufUsed(pStream->State.pCircBuf) : 0); + /* We need to lock the stream here in order to synchronize the amount of bytes we have to drain + * here with the mixer's async I/O thread. See @bugref{10354}. */ + sb16StreamLock(pStream); + + uint32_t const cbToDrain = pStream->State.pCircBuf ? (uint32_t)RTCircBufUsed(pStream->State.pCircBuf) : 0; + Log3Func(("cbToDrain=%#RX32\n", cbToDrain)); + + sb16StreamUnlock(pStream); + + rc = AudioMixerSinkDrainAndStop(pSink, cbToDrain); AssertRCReturn(rc, rc); + + } pStream->State.fEnabled = fEnable; @@ -2068,6 +2108,9 @@ static int sb16StreamCreate(PSB16STATE pThis, PSB16STREAM pStream, uint8_t uIdx) { LogFlowFuncEnter(); + int rc = RTCritSectInit(&pStream->State.CritSect); + AssertRCReturn(rc, rc); + pStream->Dbg.Runtime.fEnabled = pThis->Dbg.fEnabled; if (RT_LIKELY(!pStream->Dbg.Runtime.fEnabled)) @@ -2111,6 +2154,9 @@ static int sb16StreamDestroy(PPDMDEVINS pDevIns, PSB16STATE pThis, PSB16STREAM p pStream->State.fRegisteredAsyncUpdateJob = false; } + int rc2 = RTCritSectDelete(&pStream->State.CritSect); + AssertRC(rc2); + if (pStream->State.pCircBuf) { RTCircBufDestroy(pStream->State.pCircBuf); @@ -2177,6 +2223,28 @@ static void sb16StreamReset(PSB16STATE pThis, PSB16STREAM pStream) } /** + * Locks a SB16 stream for serialized access. + * + * @param pStream The SB16 stream to lock. + */ +DECLINLINE(void) sb16StreamLock(PSB16STREAM pStream) +{ + int rc2 = RTCritSectEnter(&pStream->State.CritSect); + AssertRC(rc2); +} + +/** + * Unlocks a formerly locked SB16 stream. + * + * @param pStream The SB16 stream to unlock. + */ +DECLINLINE(void) sb16StreamUnlock(PSB16STREAM pStream) +{ + int rc2 = RTCritSectLeave(&pStream->State.CritSect); + AssertRC(rc2); +} + +/** * Opens a SB16 stream with its current mixer settings. * * @returns VBox status code. @@ -2209,47 +2277,59 @@ static int sb16StreamOpen(PPDMDEVINS pDevIns, PSB16STATE pThis, PSB16STREAM pStr PDMAudioPropsChannels(&pStream->Cfg.Props), pStream->Cfg.Props.fSigned ? "S" : "U", PDMAudioPropsSampleBits(&pStream->Cfg.Props))); - /* (Re-)create the stream's internal ring buffer. */ - if (pStream->State.pCircBuf) - { - RTCircBufDestroy(pStream->State.pCircBuf); - pStream->State.pCircBuf = NULL; - } - /** @todo r=bird: two DMA periods is probably too little. */ const uint32_t cbCircBuf = PDMAudioPropsMilliToBytes(&pStream->Cfg.Props, (RT_MS_1SEC / pStream->uTimerHz) * 2 /* Use double buffering here */); + int rc = VINF_SUCCESS; - int rc = RTCircBufCreate(&pStream->State.pCircBuf, cbCircBuf); - AssertRCReturn(rc, rc); - pStream->State.StatDmaBufSize = (uint32_t)RTCircBufSize(pStream->State.pCircBuf); + /** + * Note: Only re-create the DMA buffer if the size actually has changed. + * + * Otherwise do *not* reset the stream's circular buffer here, as the audio mixer still relies on + * previously announced DMA data (via AudioMixerSinkDrainAndStop()) and processes it asynchronously. + * Resetting the buffer here will cause a race condition. See @bugref{10354}. */ + if (pStream->State.StatDmaBufSize != cbCircBuf) + { + Log3Func(("Re-creating ring buffer (%#RX32 -> %#RX32)\n", pStream->State.StatDmaBufSize, cbCircBuf)); - /* Set scheduling hint. */ - pStream->Cfg.Device.cMsSchedulingHint = RT_MS_1SEC / RT_MIN(pStream->uTimerHz, 1); + /* (Re-)create the stream's internal ring buffer. */ + if (pStream->State.pCircBuf) + { + RTCircBufDestroy(pStream->State.pCircBuf); + pStream->State.pCircBuf = NULL; + } - PAUDMIXSINK pMixerSink = sb16StreamIndexToSink(pThis, pStream->uIdx); - AssertPtrReturn(pMixerSink, VERR_INVALID_POINTER); + rc = RTCircBufCreate(&pStream->State.pCircBuf, cbCircBuf); + AssertRCReturn(rc, rc); + pStream->State.StatDmaBufSize = (uint32_t)RTCircBufSize(pStream->State.pCircBuf); - sb16RemoveDrvStreams(pDevIns, pThis, - sb16StreamIndexToSink(pThis, pStream->uIdx), pStream->Cfg.enmDir, pStream->Cfg.enmPath); + /* Set scheduling hint. */ + pStream->Cfg.Device.cMsSchedulingHint = RT_MS_1SEC / RT_MIN(pStream->uTimerHz, 1); - rc = sb16AddDrvStreams(pDevIns, pThis, pMixerSink, &pStream->Cfg); - if (RT_SUCCESS(rc)) - { - if (RT_LIKELY(!pStream->Dbg.Runtime.fEnabled)) - { /* likely */ } - else + PAUDMIXSINK pMixerSink = sb16StreamIndexToSink(pThis, pStream->uIdx); + AssertPtrReturn(pMixerSink, VERR_INVALID_POINTER); + + sb16RemoveDrvStreams(pDevIns, pThis, + sb16StreamIndexToSink(pThis, pStream->uIdx), pStream->Cfg.enmDir, pStream->Cfg.enmPath); + + rc = sb16AddDrvStreams(pDevIns, pThis, pMixerSink, &pStream->Cfg); + if (RT_SUCCESS(rc)) { - /* Make sure to close + delete a former debug file, as the PCM format has changed (e.g. U8 -> S16). */ - if (AudioHlpFileIsOpen(pStream->Dbg.Runtime.pFileDMA)) + if (RT_LIKELY(!pStream->Dbg.Runtime.fEnabled)) + { /* likely */ } + else { - AudioHlpFileClose(pStream->Dbg.Runtime.pFileDMA); - AudioHlpFileDelete(pStream->Dbg.Runtime.pFileDMA); - } + /* Make sure to close + delete a former debug file, as the PCM format has changed (e.g. U8 -> S16). */ + if (AudioHlpFileIsOpen(pStream->Dbg.Runtime.pFileDMA)) + { + AudioHlpFileClose(pStream->Dbg.Runtime.pFileDMA); + AudioHlpFileDelete(pStream->Dbg.Runtime.pFileDMA); + } - int rc2 = AudioHlpFileOpen(pStream->Dbg.Runtime.pFileDMA, AUDIOHLPFILE_DEFAULT_OPEN_FLAGS, - &pStream->Cfg.Props); - AssertRC(rc2); + int rc2 = AudioHlpFileOpen(pStream->Dbg.Runtime.pFileDMA, AUDIOHLPFILE_DEFAULT_OPEN_FLAGS, + &pStream->Cfg.Props); + AssertRC(rc2); + } } } @@ -2302,11 +2382,18 @@ static void sb16StreamTransferScheduleNext(PSB16STATE pThis, PSB16STREAM pStream * * @param pStream The SB16 stream. * @param pSink The mixer sink to push to. + * + * @thread MixAIO-n + * + * @note Takes the stream's lock. */ static void sb16StreamPushToMixer(PSB16STREAM pStream, PAUDMIXSINK pSink) { + sb16StreamLock(pStream); + #ifdef LOG_ENABLED uint64_t const offReadOld = pStream->State.offRead; + Log3Func(("[SD%RU8] cbCircBufUsed=%#RX64\n", pStream->uIdx, RTCircBufUsed(pStream->State.pCircBuf))); #endif pStream->State.offRead = AudioMixerSinkTransferFromCircBuf(pSink, pStream->State.pCircBuf, @@ -2315,11 +2402,13 @@ static void sb16StreamPushToMixer(PSB16STREAM pStream, PAUDMIXSINK pSink) /** @todo pStream->Dbg.Runtime.fEnabled ? pStream->Dbg.Runtime.pFileStream :*/ NULL); - Log3Func(("[SD%RU8] transferred=%#RX64 bytes -> @%#RX64\n", pStream->uIdx, - pStream->State.offRead - offReadOld, pStream->State.offRead)); + Log3Func(("[SD%RU8] transferred=%#RX64 bytes -> @%#RX64 (cbCircBufUsed=%#RX64)\n", pStream->uIdx, + pStream->State.offRead - offReadOld, pStream->State.offRead, RTCircBufUsed(pStream->State.pCircBuf))); /* Update buffer stats. */ pStream->State.StatDmaBufUsed = (uint32_t)RTCircBufUsed(pStream->State.pCircBuf); + + sb16StreamUnlock(pStream); } @@ -2327,8 +2416,10 @@ static void sb16StreamPushToMixer(PSB16STREAM pStream, PAUDMIXSINK pSink) * @callback_method_impl{FNAUDMIXSINKUPDATE} * * For output streams this moves data from the internal DMA buffer (in which - * ichac97R3StreamUpdateDma put it), thru the mixer and to the various backend + * sb16StreamDoDmaOutput put it), thru the mixer and to the various backend * audio devices. + * + * @thread MixAIO-n */ static DECLCALLBACK(void) sb16StreamUpdateAsyncIoJob(PPDMDEVINS pDevIns, PAUDMIXSINK pSink, void *pvUser) { |