VirtualBox

Changeset 98405 in vbox for trunk/src/VBox


Ignore:
Timestamp:
Feb 1, 2023 3:13:23 PM (23 months ago)
Author:
vboxsync
Message:

Audio: Fixed race condition between announcing remaining device DMA data on draining and fetching it in the async I/O mixer thread via AudioMixerSinkTransferFromCircBuf(). This is due to the device's DMA buffer being reset via its device-specific functions (i.e. ichac97R3StreamReset()) immediately after disabling the stream (i.e. ichac97R3StreamDisable()). See comment 2 for more details. bugref:10354

Location:
trunk/src/VBox/Devices/Audio
Files:
5 edited

Legend:

Unmodified
Added
Removed

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

    r98103 r98405  
    113113static int audioMixerSinkRemoveStreamInternal(PAUDMIXSINK pSink, PAUDMIXSTREAM pStream);
    114114static void audioMixerSinkResetInternal(PAUDMIXSINK pSink);
     115static int audioMixerSinkWaitForDrainedLocked(PAUDMIXSINK pSink, RTMSINTERVAL msTimeout);
    115116
    116117static int audioMixerStreamCtlInternal(PAUDMIXSTREAM pMixStream, PDMAUDIOSTREAMCMD enmCmd);
    … …  
    414415            /* AIO */
    415416            AssertPtr(pDevIns);
    416             pSink->AIO.pDevIns     = pDevIns;
    417             pSink->AIO.hThread     = NIL_RTTHREAD;
    418             pSink->AIO.hEvent      = NIL_RTSEMEVENT;
    419             pSink->AIO.fStarted    = false;
    420             pSink->AIO.fShutdown   = false;
    421             pSink->AIO.cUpdateJobs = 0;
     417            pSink->AIO.pDevIns       = pDevIns;
     418            pSink->AIO.hThread       = NIL_RTTHREAD;
     419            pSink->AIO.hEvent        = NIL_RTSEMEVENT;
     420            pSink->AIO.hEventDrained = NIL_RTSEMEVENT;
     421            pSink->AIO.fStarted      = false;
     422            pSink->AIO.fShutdown     = false;
     423            pSink->AIO.cUpdateJobs   = 0;
    422424
    423425            /*
    … …  
    578580 *                      buffers that the update job has yet to transfer.  This
    579581 *                      is ignored for input streams.
    580  */
    581 int AudioMixerSinkDrainAndStop(PAUDMIXSINK pSink, uint32_t cbComming)
     582 * @param   msTimeout   Timeout to use for synchronously waiting for the draining / update jobs.
     583 *                      Set to 0 for running asynchronously.
     584 */
     585int AudioMixerSinkDrainAndStopEx(PAUDMIXSINK pSink, uint32_t cbComming, RTMSINTERVAL msTimeout)
    582586{
    583587    AssertPtrReturn(pSink, VERR_INVALID_POINTER);
    … …  
    620624                       status. (The device's DMA timer won't kick it any more, so we must.) */
    621625                    AudioMixerSinkSignalUpdateJob(pSink);
     626#ifdef LOG_ENABLED
     627                    uint64_t const msStart = RTTimeMilliTS();
     628                    Log3Func(("Waiting for %RU32 update jobs (msTimeout=%RU32)\n", pSink->AIO.cUpdateJobs, msTimeout));
     629#endif
     630                    if (   msTimeout
     631                        && pSink->AIO.cUpdateJobs)
     632                    {
     633                        rc = audioMixerSinkWaitForDrainedLocked(pSink, msTimeout);
     634                        AssertRCReturn(rc, rc);
     635#ifdef LOG_ENABLED
     636                        Log3Func(("Waiting for update jobs done (rc=%Rrc, took %RU64ms)\n", rc, RTTimeMilliTS() - msStart));
     637#endif
     638                    }
    622639                }
    623640                else
    … …  
    662679
    663680/**
     681 * Kicks off the draining and stopping playback/capture on the mixer sink.
     682 *
     683 * For input streams this causes an immediate stop, as draining only makes sense
     684 * to output stream in the VBox device context.
     685 *
     686 * @returns VBox status code.  Generally always VINF_SUCCESS unless the input
     687 *          is invalid.  Individual driver errors are suppressed and ignored.
     688 * @param   pSink       Mixer sink to control.
     689 * @param   cbComming   The number of bytes still left in the device's DMA
     690 *                      buffers that the update job has yet to transfer.  This
     691 *                      is ignored for input streams.
     692 */
     693int AudioMixerSinkDrainAndStop(PAUDMIXSINK pSink, uint32_t cbComming)
     694{
     695    return AudioMixerSinkDrainAndStopEx(pSink, cbComming, 0 /* Don't wait */);
     696}
     697
     698
     699/**
    664700 * Destroys and frees a mixer sink.
    665701 *
    … …  
    720756    {
    721757        int rc2 = RTSemEventSignal(pSink->AIO.hEvent);
     758        AssertRC(rc2);
     759    }
     760    if (pSink->AIO.hEventDrained != NIL_RTSEMEVENT)
     761    {
     762        int rc2 = RTSemEventSignal(pSink->AIO.hEventDrained);
    722763        AssertRC(rc2);
    723764    }
    … …  
    734775        AssertRC(rc2);
    735776        pSink->AIO.hEvent = NIL_RTSEMEVENT;
     777    }
     778    if (pSink->AIO.hEventDrained != NIL_RTSEMEVENT)
     779    {
     780        int rc2 = RTSemEventDestroy(pSink->AIO.hEventDrained);
     781        AssertRC(rc2);
     782        pSink->AIO.hEventDrained = NIL_RTSEMEVENT;
    736783    }
    737784
    … …  
    17711818     * The run loop.
    17721819     */
     1820    bool fDrainingFinished = false; /* Whether draining the stream is finished. */
     1821#ifdef LOG_ENABLED
     1822    char szStatus[AUDIOMIXERSINK_STATUS_STR_MAX];
     1823#endif
     1824
    17731825    LogFlowFunc(("%s: Entering run loop...\n", pSink->pszName));
    17741826    while (!pSink->AIO.fShutdown)
    … …  
    17771829
    17781830        RTCritSectEnter(&pSink->CritSect);
     1831
     1832#ifdef LOG_ENABLED
     1833        Log3Func(("%s: Running async I/O (fStatus=%s) ...\n", pSink->pszName,
     1834                  dbgAudioMixerSinkStatusToStr(pSink->fStatus, szStatus)));
     1835#endif
    17791836        if (pSink->fStatus & (AUDMIXSINK_STS_RUNNING | AUDMIXSINK_STS_DRAINING))
    17801837        {
     1838            uint32_t fStatusOld = pSink->fStatus;
     1839
    17811840            /*
    17821841             * Before doing jobs, always update input sinks.
    … …  
    18041863             */
    18051864            if (!(pSink->fStatus & AUDMIXSINK_STS_DRAINING))
    1806             { /* likely */ }
     1865            {
     1866                /* likely */
     1867                if (fStatusOld & AUDMIXSINK_STS_DRAINING)
     1868                {
     1869                    Log3Func(("%s: Started draining\n", pSink->pszName));
     1870                    fDrainingFinished = true;
     1871                }
     1872            }
    18071873            else
    18081874            {
    … …  
    18121878            }
    18131879
     1880            fStatusOld = pSink->fStatus;
     1881
     1882            Log3Func(("%s: Running async I/O finished (fStatus=%s, cMsSleep=%RU32)\n",
     1883                      pSink->pszName, dbgAudioMixerSinkStatusToStr(pSink->fStatus, szStatus), cMsSleep));
    18141884        }
    18151885        RTCritSectLeave(&pSink->CritSect);
     1886
     1887        /*
     1888         * Signal waiters.
     1889         */
     1890        if (fDrainingFinished)
     1891        {
     1892            Log3Func(("%s: Finished draining\n", pSink->pszName));
     1893            int rc = RTSemEventSignal(pSink->AIO.hEventDrained);
     1894            AssertLogRelMsgReturn(RT_SUCCESS(rc), ("%s: RTSemEventSignal(hEventDrained) -> %Rrc\n", pSink->pszName, rc), rc);
     1895            fDrainingFinished = false;
     1896        }
    18161897
    18171898        /*
    … …  
    18221903            int rc = RTSemEventWait(pSink->AIO.hEvent, cMsSleep);
    18231904            AssertLogRelMsgReturn(RT_SUCCESS(rc) || rc == VERR_TIMEOUT, ("%s: RTSemEventWait -> %Rrc\n", pSink->pszName, rc), rc);
     1905#ifdef LOG_ENABLED
     1906            Log3Func(("%s: Got signalled (fStatus=%s)\n", pSink->pszName, dbgAudioMixerSinkStatusToStr(pSink->fStatus, szStatus)));
     1907#endif
    18241908        }
    18251909    }
    … …  
    18871971        {
    18881972            rc = RTSemEventCreate(&pSink->AIO.hEvent);
     1973            AssertRCReturnStmt(rc, RTCritSectLeave(&pSink->CritSect), rc);
     1974        }
     1975        if (pSink->AIO.hEventDrained == NIL_RTSEMEVENT)
     1976        {
     1977            rc = RTSemEventCreate(&pSink->AIO.hEventDrained);
    18891978            AssertRCReturnStmt(rc, RTCritSectLeave(&pSink->CritSect), rc);
    18901979        }
    … …  
    19602049
    19612050/**
     2051 * Waits for the sink's draining to complete, internal version.
     2052 *
     2053 * @returns VBox status code.
     2054 * @param   pSink       The mixer sink which AIO thread needs waiting on.
     2055 * @þaram   msTimeout   Timeout (in ms) to use for waiting on draining to be finished.
     2056 *
     2057 * @note    Caller holds the sink's lock.
     2058 */
     2059static int audioMixerSinkWaitForDrainedLocked(PAUDMIXSINK pSink, RTMSINTERVAL msTimeout)
     2060{
     2061    AssertReturn((pSink->fStatus & AUDMIXSINK_STS_DRAINING), VERR_WRONG_ORDER);
     2062
     2063    int rc;
     2064
     2065    if (pSink->AIO.cUpdateJobs)
     2066    {
     2067        rc = RTCritSectLeave(&pSink->CritSect);
     2068        AssertRCReturn(rc, rc);
     2069
     2070        rc = RTSemEventWait(pSink->AIO.hEventDrained, msTimeout);
     2071
     2072        int rc2 = RTCritSectEnter(&pSink->CritSect);
     2073        AssertRCReturn(rc2, rc2);
     2074
     2075    }
     2076    else
     2077        rc = VINF_SUCCESS;
     2078
     2079    return rc;
     2080}
     2081
     2082
     2083/**
     2084 * Waits for the sink's draining to complete.
     2085 *
     2086 * @returns VBox status code.
     2087 * @param   pSink       The mixer sink which AIO thread needs waiting on.
     2088 * @þaram   msTimeout   Timeout (in ms) to use for waiting on draining to be finished.
     2089 */
     2090int AudioMixerSinkWaitForDrained(PAUDMIXSINK pSink, RTMSINTERVAL msTimeout)
     2091{
     2092    AssertPtrReturn(pSink, VERR_INVALID_POINTER);
     2093    Assert(pSink->uMagic == AUDMIXSINK_MAGIC);
     2094
     2095    int rc = RTCritSectEnter(&pSink->CritSect);
     2096    AssertRCReturn(rc, rc);
     2097
     2098    rc = audioMixerSinkWaitForDrainedLocked(pSink, msTimeout);
     2099
     2100    int rc2 = RTCritSectLeave(&pSink->CritSect);
     2101    AssertRCReturn(rc2, rc2);
     2102
     2103    return rc;
     2104}
     2105
     2106
     2107/**
    19622108 * Writes data to a mixer output sink.
    19632109 *
    … …  
    20242170              idStream, cbSinkWritable, cbCircBufReadable, cbToTransfer, offStream));
    20252171    AssertMsg(!(pSink->fStatus & AUDMIXSINK_STS_DRAINING) || cbCircBufReadable == pSink->cbDmaLeftToDrain,
    2026               ("cbCircBufReadable=%#x cbDmaLeftToDrain=%#x\n", cbCircBufReadable, pSink->cbDmaLeftToDrain));
     2172              ("idStream=%u: fStatus=%#x cbCircBufReadable=%#x cbDmaLeftToDrain=%#x\n",
     2173               idStream, pSink->fStatus, cbCircBufReadable, pSink->cbDmaLeftToDrain));
    20272174
    20282175    /*

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

    r98103 r98405  
    244244        /** Event for letting the thread know there is some data to process. */
    245245        RTSEMEVENT              hEvent;
     246        /** Event for letting the waiters know that draining the sink is done. */
     247        RTSEMEVENT              hEventDrained;
    246248        /** The device instance (same for all update jobs). */
    247249        PPDMDEVINS              pDevIns;
    … …  
    309311 * @{ */
    310312int         AudioMixerSinkStart(PAUDMIXSINK pSink);
     313int         AudioMixerSinkDrainAndStopEx(PAUDMIXSINK pSink, uint32_t cbComming, RTMSINTERVAL msTimeout);
    311314int         AudioMixerSinkDrainAndStop(PAUDMIXSINK pSink, uint32_t cbComming);
    312315void        AudioMixerSinkDestroy(PAUDMIXSINK pSink, PPDMDEVINS pDevIns);
    … …  
    324327int         AudioMixerSinkRemoveUpdateJob(PAUDMIXSINK pSink, PFNAUDMIXSINKUPDATE pfnUpdate, void *pvUser);
    325328int         AudioMixerSinkSignalUpdateJob(PAUDMIXSINK pSink);
     329int         AudioMixerSinkWaitForDrained(PAUDMIXSINK pSink, RTMSINTERVAL msTimeout);
    326330uint64_t    AudioMixerSinkTransferFromCircBuf(PAUDMIXSINK pSink, PRTCIRCBUF pCircBuf, uint64_t offStream,
    327331                                              uint32_t idStream, PAUDIOHLPFILE pDbgFile);

  • trunk/src/VBox/Devices/Audio/DevHdaStream.cpp

    r98103 r98405  
    10591059        }
    10601060        else
    1061             rc = AudioMixerSinkDrainAndStop(pSink,
    1062                                             pStreamR3->State.pCircBuf ? (uint32_t)RTCircBufUsed(pStreamR3->State.pCircBuf) : 0);
     1061            rc = AudioMixerSinkDrainAndStopEx(pSink,
     1062                                              pStreamR3->State.pCircBuf ? (uint32_t)RTCircBufUsed(pStreamR3->State.pCircBuf) : 0,
     1063                                              RT_MS_5SEC);
    10631064    }
    10641065    if (   RT_SUCCESS(rc)

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

    r98333 r98405  
    23482348    else
    23492349    {
    2350         rc = AudioMixerSinkDrainAndStop(pSink, pStreamCC->State.pCircBuf ? (uint32_t)RTCircBufUsed(pStreamCC->State.pCircBuf) : 0);
     2350        rc = AudioMixerSinkDrainAndStopEx(pSink,
     2351                                          pStreamCC->State.pCircBuf ? (uint32_t)RTCircBufUsed(pStreamCC->State.pCircBuf) : 0,
     2352                                          RT_MS_5SEC);
    23512353        ichac97R3StreamTearDown(pStream);
    23522354    }

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

    r98103 r98405  
    20122012    else
    20132013    {
    2014         rc = AudioMixerSinkDrainAndStop(pSink, pStream->State.pCircBuf ? (uint32_t)RTCircBufUsed(pStream->State.pCircBuf) : 0);
     2014        rc = AudioMixerSinkDrainAndStopEx(pSink, pStream->State.pCircBuf ? (uint32_t)RTCircBufUsed(pStream->State.pCircBuf) : 0,
     2015                                          RT_MS_5SEC);
    20152016        AssertRCReturn(rc, rc);
    20162017    }
Note: See TracChangeset for help on using the changeset viewer.

© 2024 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette