From c50fffc0d28b2fd57165e5e1088d5105c845d404 Mon Sep 17 00:00:00 2001 From: Etienne Dechamps Date: Mon, 6 Mar 2023 18:07:49 +0000 Subject: [PATCH 1/2] dsound: simplify CalculateBufferSettings() This commit rewrites CalculateBufferSettings() in an attempt to make it a bit less headache-inducing. The basic idea is to reduce branching, reduce code duplication, and share code paths between the various cases (fixed/variable user buffer, half/full duplex) as much as possible. Also, min()/max() are easier to read than if statements. This is a pure refactoring - there should be no change in observable behaviour. The math stays the same, it is merely reshuffled around in place. --- src/hostapi/dsound/pa_win_ds.c | 70 +++++++++------------------------- 1 file changed, 18 insertions(+), 52 deletions(-) diff --git a/src/hostapi/dsound/pa_win_ds.c b/src/hostapi/dsound/pa_win_ds.c index b0ab18725..3d852e196 100644 --- a/src/hostapi/dsound/pa_win_ds.c +++ b/src/hostapi/dsound/pa_win_ds.c @@ -1734,63 +1734,29 @@ static void CalculateBufferSettings( unsigned long *hostBufferSizeFrames, unsigned long maximumPollingPeriodFrames = (unsigned long)(sampleRate * PA_DS_MAXIMUM_POLLING_PERIOD_SECONDS); unsigned long pollingJitterFrames = (unsigned long)(sampleRate * PA_DS_POLLING_JITTER_SECONDS); - if( userFramesPerBuffer == paFramesPerBufferUnspecified ) + unsigned long adjustedSuggestedOutputLatencyFrames = suggestedOutputLatencyFrames; + if( userFramesPerBuffer != paFramesPerBufferUnspecified && isFullDuplex ) { - unsigned long targetBufferingLatencyFrames = max( suggestedInputLatencyFrames, suggestedOutputLatencyFrames ); - - *pollingPeriodFrames = targetBufferingLatencyFrames / 4; - if( *pollingPeriodFrames < minimumPollingPeriodFrames ) - { - *pollingPeriodFrames = minimumPollingPeriodFrames; - } - else if( *pollingPeriodFrames > maximumPollingPeriodFrames ) - { - *pollingPeriodFrames = maximumPollingPeriodFrames; - } - - *hostBufferSizeFrames = *pollingPeriodFrames - + max( *pollingPeriodFrames + pollingJitterFrames, targetBufferingLatencyFrames); + /* In full duplex streams we know that the buffer adapter adds userFramesPerBuffer + extra fixed latency. so we subtract it here as a fixed latency before computing + the buffer size. being careful not to produce an unrepresentable negative result. + */ + adjustedSuggestedOutputLatencyFrames -= min( userFramesPerBuffer, adjustedSuggestedOutputLatencyFrames ); } - else - { - unsigned long targetBufferingLatencyFrames = suggestedInputLatencyFrames; - if( isFullDuplex ) - { - /* In full duplex streams we know that the buffer adapter adds userFramesPerBuffer - extra fixed latency. so we subtract it here as a fixed latency before computing - the buffer size. being careful not to produce an unrepresentable negative result. - Note: this only works as expected if output latency is greater than input latency. - Otherwise we use input latency anyway since we do max(in,out). - */ + const unsigned long targetBufferingLatencyFrames = max( suggestedInputLatencyFrames, adjustedSuggestedOutputLatencyFrames ); - if( userFramesPerBuffer < suggestedOutputLatencyFrames ) - { - unsigned long adjustedSuggestedOutputLatencyFrames = - suggestedOutputLatencyFrames - userFramesPerBuffer; + *pollingPeriodFrames = (userFramesPerBuffer == paFramesPerBufferUnspecified) ? + targetBufferingLatencyFrames / 4 : + max( max( 1, userFramesPerBuffer / 4 ), targetBufferingLatencyFrames / 16 ); + *pollingPeriodFrames = min( max( *pollingPeriodFrames, minimumPollingPeriodFrames ), maximumPollingPeriodFrames ); - /* maximum of input and adjusted output suggested latency */ - if( adjustedSuggestedOutputLatencyFrames > targetBufferingLatencyFrames ) - targetBufferingLatencyFrames = adjustedSuggestedOutputLatencyFrames; - } - } - else - { - /* maximum of input and output suggested latency */ - if( suggestedOutputLatencyFrames > suggestedInputLatencyFrames ) - targetBufferingLatencyFrames = suggestedOutputLatencyFrames; - } - - *hostBufferSizeFrames = userFramesPerBuffer - + max( userFramesPerBuffer + pollingJitterFrames, targetBufferingLatencyFrames); - - *pollingPeriodFrames = max( max(1, userFramesPerBuffer / 4), targetBufferingLatencyFrames / 16 ); - - if( *pollingPeriodFrames > maximumPollingPeriodFrames ) - { - *pollingPeriodFrames = maximumPollingPeriodFrames; - } - } + const unsigned long intendedUserFramesPerBuffer = + (userFramesPerBuffer == paFramesPerBufferUnspecified) ? + *pollingPeriodFrames : + userFramesPerBuffer; + *hostBufferSizeFrames = intendedUserFramesPerBuffer + + max( intendedUserFramesPerBuffer + pollingJitterFrames, targetBufferingLatencyFrames ); } From 4a658fad11e3685cc8d20b2568d4e324c0e1e949 Mon Sep 17 00:00:00 2001 From: Etienne Dechamps Date: Mon, 6 Mar 2023 18:22:37 +0000 Subject: [PATCH 2/2] dsound: clamp input buffer size at 62.5 ms This works around a DirectSound limitation where input host buffer sizes smaller than 31.25 ms are basically unworkable and make PortAudio hang. The workaround is to impose a minimal buffer size of 2*31.25 ms on input-only and full-duplex streams. This is enough for the read cursor to advance twice around the buffer, basically resulting in de facto double buffering. This change was tested with `paloopback` under a wide variety of half/full-duplex, framesPerBuffer, and suggested latency parameters. (Note the testing was done on top of #772 as otherwise paloopback is not usable.) Fixes #775 --- src/hostapi/dsound/pa_win_ds.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/hostapi/dsound/pa_win_ds.c b/src/hostapi/dsound/pa_win_ds.c index 3d852e196..bebe2297f 100644 --- a/src/hostapi/dsound/pa_win_ds.c +++ b/src/hostapi/dsound/pa_win_ds.c @@ -1725,7 +1725,7 @@ static HRESULT InitOutputBuffer( PaWinDsStream *stream, PaWinDsDeviceInfo *devic static void CalculateBufferSettings( unsigned long *hostBufferSizeFrames, unsigned long *pollingPeriodFrames, - int isFullDuplex, + int hasInput, int hasOutput, unsigned long suggestedInputLatencyFrames, unsigned long suggestedOutputLatencyFrames, double sampleRate, unsigned long userFramesPerBuffer ) @@ -1735,7 +1735,7 @@ static void CalculateBufferSettings( unsigned long *hostBufferSizeFrames, unsigned long pollingJitterFrames = (unsigned long)(sampleRate * PA_DS_POLLING_JITTER_SECONDS); unsigned long adjustedSuggestedOutputLatencyFrames = suggestedOutputLatencyFrames; - if( userFramesPerBuffer != paFramesPerBufferUnspecified && isFullDuplex ) + if( userFramesPerBuffer != paFramesPerBufferUnspecified && hasInput && hasOutput ) { /* In full duplex streams we know that the buffer adapter adds userFramesPerBuffer extra fixed latency. so we subtract it here as a fixed latency before computing @@ -1757,6 +1757,21 @@ static void CalculateBufferSettings( unsigned long *hostBufferSizeFrames, userFramesPerBuffer; *hostBufferSizeFrames = intendedUserFramesPerBuffer + max( intendedUserFramesPerBuffer + pollingJitterFrames, targetBufferingLatencyFrames ); + + /* In some (most?) systems, DirectSound has an odd limitation where it always uses + a fixed 31.25 ms granularity for the read cursor, regardless of parameters. + This in turn means that if we allocate an input buffer that is less than 31.25 ms, + the read cursor will stay stuck at zero. See https://github.com/PortAudio/portaudio/issues/775 + To work around this problem, ensure that the input host buffer is large enough + for at least two 31.25 ms buffer "halves". + + On pre-Vista Windows, we don't do this because DirectSound is implemented very + differently, and is therefore unlikely to suffer from the same issue. + */ + if( hasInput && PaWinUtil_GetOsVersion() >= paOsVersionWindowsVistaServer2008 ) + { + *hostBufferSizeFrames = max( *hostBufferSizeFrames, 2 * 0.03125 * sampleRate ); + } } @@ -2086,7 +2101,8 @@ static PaError OpenStream( struct PaUtilHostApiRepresentation *hostApi, else { CalculateBufferSettings( (unsigned long*)&stream->hostBufferSizeFrames, &pollingPeriodFrames, - /* isFullDuplex = */ (inputParameters && outputParameters), + /* hasInput = */ !!inputParameters, + /* hasOutput = */ !!outputParameters, suggestedInputLatencyFrames, suggestedOutputLatencyFrames, sampleRate, framesPerBuffer );