From 6a1f1b40570f415349281325405337245c155a7a Mon Sep 17 00:00:00 2001 From: Ross Bencina Date: Mon, 12 Sep 2016 01:24:51 +1000 Subject: [PATCH] hotplug wmme: Change StopStream() timeout policy. For callback streams the maximum wait times have been tweaked. Total wait time has been reduced (Old algorithm wait time: [max(2*totalBuffersDur*1.5, 2*1second)], new algorithm wait time: [totalBuffersDur*1.5+min(totalBuffersDur*1.5, 1second)]). For blocking read write streams, the maximum wait time has been reduced to approximately totalBuffersDur*1.5. Added test patest_unplug_readwrite.c for testing stopping read/write streams after unplug. Tweaked patest_unplug.c to build on Windows, and to use 1 input channel (for headset). (Timeout changes made with reference to Jitsi [lyubomir a45b993, Thank you!]. --- src/hostapi/wmme/pa_win_wmme.c | 70 +++++++-- test/patest_unplug.c | 19 ++- test/patest_unplug_readwrite.c | 273 +++++++++++++++++++++++++++++++++ 3 files changed, 343 insertions(+), 19 deletions(-) create mode 100644 test/patest_unplug_readwrite.c diff --git a/src/hostapi/wmme/pa_win_wmme.c b/src/hostapi/wmme/pa_win_wmme.c index 66645f2..e4fe92c 100644 --- a/src/hostapi/wmme/pa_win_wmme.c +++ b/src/hostapi/wmme/pa_win_wmme.c @@ -3768,11 +3768,13 @@ static PaError StopStream( PaStream *s ) { PaError result = paNoError; PaWinMmeStream *stream = (PaWinMmeStream*)s; - int timeout; + DWORD timeoutMs, totalTimeoutMs; + DWORD waitStartTime, elapsedMs; DWORD waitResult; MMRESULT mmresult; signed int hostOutputBufferIndex; - unsigned int channel, waitCount, i; + unsigned int channel, waitCount, i; + unsigned int maxWaitCount; /** @todo REVIEW: the error checking in this function needs review. the basic @@ -3785,23 +3787,26 @@ static PaError StopStream( PaStream *s ) { /* callback stream */ + /* First-chance timeout is for draining the the buffer cleanly. Use a time longer than to total buffers duration. */ + timeoutMs = (DWORD)(stream->allBuffersDurationMs * 1.5) + 1; + /* Tell processing thread to stop generating more data and to let current data play out. */ stream->stopProcessing = 1; - /* Calculate timeOut longer than longest time it could take to return all buffers. */ - timeout = (int)(stream->allBuffersDurationMs * 1.5); - if( timeout < PA_MME_MIN_TIMEOUT_MSEC_ ) - timeout = PA_MME_MIN_TIMEOUT_MSEC_; - PA_DEBUG(("WinMME StopStream: waiting for background thread.\n")); - waitResult = WaitForSingleObject( stream->processingThread, timeout ); + waitResult = WaitForSingleObject( stream->processingThread, timeoutMs ); if( waitResult == WAIT_TIMEOUT ) { /* try to abort */ + + /* Last-chance timeout results in no more than PA_MME_MIN_TIMEOUT_MSEC_ of additional waiting. */ + if( timeoutMs > PA_MME_MIN_TIMEOUT_MSEC_ ) + timeoutMs = PA_MME_MIN_TIMEOUT_MSEC_; + stream->abortProcessing = 1; SetEvent( stream->abortEvent ); - waitResult = WaitForSingleObject( stream->processingThread, timeout ); + waitResult = WaitForSingleObject( stream->processingThread, timeoutMs ); if( waitResult == WAIT_TIMEOUT ) { PA_DEBUG(("WinMME StopStream: timed out while waiting for background thread to finish.\n")); @@ -3853,22 +3858,57 @@ static PaError StopStream( PaStream *s ) } - timeout = (stream->allBuffersDurationMs / stream->output.bufferCount) + 1; - if( timeout < PA_MME_MIN_TIMEOUT_MSEC_ ) - timeout = PA_MME_MIN_TIMEOUT_MSEC_; + /* + The purpose of the following wait/poll loop is to wait for + the output to play out (all queued buffers to be returned). + We expect all queued buffers to be returned to us within + allBuffersDurationMs (plus some margin to avoid cutting off + the tail). + + When functioning as intended, WaitForSingleObject will wake + after each buffer completes, taking at most bufferCount + loop iterations. We're done when NoBuffersAreQueued() returns true. + When not functioning as intended, we want to bound the + duration of the wait, but also poll frequently enough to + pick up an early-exit from NoBuffersAreQueued. + + Two mechanisms are used to ensure that this loop does not hang: + + - The total elapsed wait time is limited to totalTimeoutMs. + - The loop is limited to maxWaitCount iterations, and the + timeout for each iteration equals totalTimeoutMs/maxWaitCount. + + This combination should cover cases where the timers or + timeout times are unreliable (e.g. if WFSO takes longer + than timeoutMs to return). + */ + + totalTimeoutMs = (DWORD)(1.5 * stream->allBuffersDurationMs); + + /* poll every 1.5 buffer durations. */ + timeoutMs = (DWORD)((1.5 * stream->allBuffersDurationMs) / stream->output.bufferCount) + 1; + /* for a total of maxWaitCount iterations, duration: maxWaitCount*timeoutMs = 1.5 * stream->allBuffersDurationMs */ + maxWaitCount = stream->output.bufferCount; + + waitStartTime = GetTickCount(); waitCount = 0; - while( !NoBuffersAreQueued( &stream->output ) && waitCount <= stream->output.bufferCount ) + while( !NoBuffersAreQueued( &stream->output ) && waitCount <= maxWaitCount ) { /* wait for MME to signal that a buffer is available */ - waitResult = WaitForSingleObject( stream->output.bufferEvent, timeout ); + waitResult = WaitForSingleObject( stream->output.bufferEvent, timeoutMs ); if( waitResult == WAIT_FAILED ) { break; } else if( waitResult == WAIT_TIMEOUT ) { - /* keep waiting */ + elapsedMs = GetTickCount() - waitStartTime; + if( elapsedMs >= totalTimeoutMs ) + { + result = paTimedOut; + break; + } } ++waitCount; diff --git a/test/patest_unplug.c b/test/patest_unplug.c index ba55b7d..f9b8997 100644 --- a/test/patest_unplug.c +++ b/test/patest_unplug.c @@ -59,9 +59,9 @@ typedef struct { short sine[TABLE_SIZE]; - int32_t phases[MAX_CHANNELS]; - int32_t numChannels; - int32_t sampsToGo; + long phases[MAX_CHANNELS]; + long numChannels; + long sampsToGo; } paTestData; @@ -171,7 +171,7 @@ int main(int argc, char **args) goto error; } - inputParameters.channelCount = 2; + inputParameters.channelCount = 1; inputParameters.sampleFormat = paInt16; deviceInfo = Pa_GetDeviceInfo( inputParameters.device ); if( deviceInfo == NULL ) @@ -227,10 +227,21 @@ int main(int argc, char **args) printf("Pa_IsStreamActive(outputStream) = %d\n", Pa_IsStreamActive(outputStream)); } while( Pa_IsStreamActive(inputStream) && Pa_IsStreamActive(outputStream) ); +#if 1 + err = Pa_StopStream( inputStream ); + if( err != paNoError ) goto error; + printf("Input stream stopped OK.\n"); + err = Pa_StopStream( outputStream ); + if( err != paNoError ) goto error; + printf("Output stream stopped OK.\n"); +#endif + err = Pa_CloseStream( inputStream ); if( err != paNoError ) goto error; + printf("Input stream closed OK.\n"); err = Pa_CloseStream( outputStream ); if( err != paNoError ) goto error; + printf("Output stream closed OK.\n"); Pa_Terminate(); return paNoError; error: diff --git a/test/patest_unplug_readwrite.c b/test/patest_unplug_readwrite.c new file mode 100644 index 0000000..da57fe2 --- /dev/null +++ b/test/patest_unplug_readwrite.c @@ -0,0 +1,273 @@ +/** @file patest_unplug.c + @ingroup test_src + @brief Debug a crash involving unplugging a USB device. + @author Phil Burk http://www.softsynth.com +*/ +/* + * $Id$ + * + * This program uses the PortAudio Portable Audio Library. + * For more information see: http://www.portaudio.com + * Copyright (c) 1999-2000 Ross Bencina and Phil Burk + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files + * (the "Software"), to deal in the Software without restriction, + * including without limitation the rights to use, copy, modify, merge, + * publish, distribute, sublicense, and/or sell copies of the Software, + * and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF + * CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +/* + * The text above constitutes the entire PortAudio license; however, + * the PortAudio community also makes the following non-binding requests: + * + * Any person wishing to distribute modifications to the Software is + * requested to send the modifications to the original developer so that + * they can be incorporated into the canonical version. It is also + * requested that these non-binding requests be included along with the + * license above. + */ + +#include +#include +#include +#include +#include "portaudio.h" + +#define NUM_SECONDS (8) +#define SAMPLE_RATE (44100) +#ifndef M_PI +#define M_PI (3.14159265) +#endif +#define TABLE_SIZE (200) +#define FRAMES_PER_BUFFER (64) +#define MAX_CHANNELS (8) + +#define INPUT_CHANNELS (1) /* 1 so it works with headset mic */ +#define OUTPUT_CHANNELS (2) + +typedef struct +{ + short sine[TABLE_SIZE]; + long phases[MAX_CHANNELS]; + long numChannels; +} +paTestData; + +static void generateSine( short *out, + unsigned long framesPerBuffer, + paTestData *data ) +{ + unsigned int i; + int channelIndex; + + for( i=0; inumChannels; channelIndex++) + { + int phase = data->phases[channelIndex]; + *out++ = data->sine[phase]; + phase += channelIndex + 2; + if( phase >= TABLE_SIZE ) phase -= TABLE_SIZE; + data->phases[channelIndex] = phase; + } + } +} + +/*******************************************************************/ +int main(int argc, char **args); +int main(int argc, char **args) +{ + PaStreamParameters inputParameters; + PaStreamParameters outputParameters; + PaStream *inputStream; + PaStream *outputStream; + const PaDeviceInfo *deviceInfo; + PaError err; + paTestData data; + int i; + int totalSamps; + int inputDevice = -1; + int outputDevice = -1; + int sampsToGo; + short inputBuffer[INPUT_CHANNELS * FRAMES_PER_BUFFER]; + short outputBuffer[OUTPUT_CHANNELS * FRAMES_PER_BUFFER]; + + printf("Test unplugging a USB device.\n"); + + if( argc > 1 ) { + inputDevice = outputDevice = atoi( args[1] ); + printf("Using device number %d.\n\n", inputDevice ); + } else { + printf("Using default device.\n\n" ); + } + + memset(&data, 0, sizeof(data)); + + /* initialise sinusoidal wavetable */ + for( i=0; idefaultLowInputLatency; + inputParameters.hostApiSpecificStreamInfo = NULL; + err = Pa_OpenStream( + &inputStream, + &inputParameters, + NULL, + SAMPLE_RATE, + FRAMES_PER_BUFFER, + 0, + NULL, + &data ); + if( err != paNoError ) goto error; + + outputParameters.channelCount = OUTPUT_CHANNELS; + outputParameters.sampleFormat = paInt16; + deviceInfo = Pa_GetDeviceInfo( outputParameters.device ); + if( deviceInfo == NULL ) + { + fprintf( stderr, "No matching output device.\n" ); + goto error; + } + outputParameters.suggestedLatency = deviceInfo->defaultLowOutputLatency; + outputParameters.hostApiSpecificStreamInfo = NULL; + err = Pa_OpenStream( + &outputStream, + NULL, + &outputParameters, + SAMPLE_RATE, + FRAMES_PER_BUFFER, + (paClipOff | paDitherOff), + NULL, + &data ); + if( err != paNoError ) goto error; + + err = Pa_StartStream( inputStream ); + if( err != paNoError ) goto error; + err = Pa_StartStream( outputStream ); + if( err != paNoError ) goto error; + + printf("When you hear sound, unplug the USB device.\n"); + do + { + signed long available; + available = Pa_GetStreamReadAvailable(inputStream); + while( available > 0 ) { + if( available > FRAMES_PER_BUFFER ) + available = FRAMES_PER_BUFFER; + + err = Pa_ReadStream( inputStream, inputBuffer, available ); /* reading <= available means don't block */ + if( err != paNoError ) goto done; /* Move on to stopping stream */ + + sampsToGo -= available; + + available = Pa_GetStreamReadAvailable(inputStream); + } + + available = Pa_GetStreamWriteAvailable(outputStream); + while( available > 0 ) { + if( available > FRAMES_PER_BUFFER ) + available = FRAMES_PER_BUFFER; + + generateSine( outputBuffer, available, &data ); + err = Pa_WriteStream( outputStream, outputBuffer, available ); /* reading <= available means don't block */ + if( err != paNoError ) goto done; /* Move on to stopping stream */ + + available = Pa_GetStreamWriteAvailable(outputStream); + } + + Pa_Sleep(1); + + printf("Frames remaining = %d\n", sampsToGo); + printf("Pa_IsStreamActive(inputStream) = %d\n", Pa_IsStreamActive(inputStream)); + printf("Pa_IsStreamActive(outputStream) = %d\n", Pa_IsStreamActive(outputStream)); + } while( Pa_IsStreamActive(inputStream) && Pa_IsStreamActive(outputStream) && sampsToGo > 0 ); +done: + +#if 1 + printf("Stopping input stream...\n"); + err = Pa_StopStream( inputStream ); + if( err != paNoError ) goto error; + printf("Input stream stopped OK.\n"); + + printf("Stopping output stream...\n"); + err = Pa_StopStream( outputStream ); + if( err != paNoError ) goto error; + printf("Output stream stopped OK.\n"); +#endif + +#if 0 + err = Pa_AbortStream( inputStream ); + if( err != paNoError ) goto error; + printf("Input stream stopped OK.\n"); + err = Pa_AbortStream( outputStream ); + if( err != paNoError ) goto error; + printf("Output stream stopped OK.\n"); +#endif + + err = Pa_CloseStream( inputStream ); + if( err != paNoError ) goto error; + printf("Input stream closed OK.\n"); + err = Pa_CloseStream( outputStream ); + if( err != paNoError ) goto error; + printf("Output stream closed OK.\n"); + Pa_Terminate(); + return paNoError; +error: + Pa_Terminate(); + fprintf( stderr, "An error occured while using the portaudio stream\n" ); + fprintf( stderr, "Error number: %d\n", err ); + fprintf( stderr, "Error message: %s\n", Pa_GetErrorText( err ) ); + fprintf( stderr, "Host Error message: %s\n", Pa_GetLastHostErrorInfo()->errorText ); + return err; +} -- 2.43.0