From: Phil Burk Date: Thu, 19 May 2016 04:41:01 +0000 (-0700) Subject: Fix assert when reading or writing with non-power of 2 channels. X-Git-Tag: pa_stable_v190600_20161030~17^2~3 X-Git-Url: https://andrewgundersen.net/repos?a=commitdiff_plain;h=110043f84225aa12750d299152addd295184619c;p=portaudio Fix assert when reading or writing with non-power of 2 channels. --- diff --git a/examples/paex_read_write_wire.c b/examples/paex_read_write_wire.c index 2f13426..586dd17 100644 --- a/examples/paex_read_write_wire.c +++ b/examples/paex_read_write_wire.c @@ -33,13 +33,13 @@ */ /* - * The text above constitutes the entire PortAudio license; however, + * 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 + * they can be incorporated into the canonical version. It is also + * requested that these non-binding requests be included along with the * license above. */ @@ -51,7 +51,7 @@ /* #define SAMPLE_RATE (17932) // Test failure to open with this value. */ #define SAMPLE_RATE (44100) #define FRAMES_PER_BUFFER (1024) -#define NUM_CHANNELS (2) +#define NUM_CHANNELS (6) #define NUM_SECONDS (15) /* #define DITHER_FLAG (paDitherOff) */ #define DITHER_FLAG (0) /**/ @@ -109,8 +109,8 @@ int main(void) char *sampleBlock; int i; int numBytes; - - + + printf("patest_read_write_wire.c\n"); fflush(stdout); numBytes = FRAMES_PER_BUFFER * NUM_CHANNELS * SAMPLE_SIZE ; @@ -170,24 +170,6 @@ int main(void) err = Pa_StopStream( stream ); if( err != paNoError ) goto error; - CLEAR( sampleBlock ); -/* - err = Pa_StartStream( stream ); - if( err != paNoError ) goto error; - printf("Wire on. Interrupt to stop.\n"); fflush(stdout); - - while( 1 ) - { - err = Pa_WriteStream( stream, sampleBlock, FRAMES_PER_BUFFER ); - if( err ) goto xrun; - err = Pa_ReadStream( stream, sampleBlock, FRAMES_PER_BUFFER ); - if( err ) goto xrun; - } - err = Pa_StopStream( stream ); - if( err != paNoError ) goto error; - - Pa_CloseStream( stream ); -*/ free( sampleBlock ); Pa_Terminate(); diff --git a/src/hostapi/coreaudio/pa_mac_core.c b/src/hostapi/coreaudio/pa_mac_core.c index 0aede34..4b7b43c 100644 --- a/src/hostapi/coreaudio/pa_mac_core.c +++ b/src/hostapi/coreaudio/pa_mac_core.c @@ -1921,14 +1921,13 @@ static PaError OpenStream( struct PaUtilHostApiRepresentation *hostApi, /* * If input and output devs are different or we are doing SR conversion, - * we also need a - * ring buffer to store inpt data while waiting for output - * data. + * we also need a ring buffer to store input data while waiting for + * output data. */ if( (stream->outputUnit && (stream->inputUnit != stream->outputUnit)) || stream->inputSRConverter ) { - /* May want the ringSize ot initial position in + /* May want the ringSize or initial position in ring buffer to depend somewhat on sample rate change */ void *data; @@ -1951,7 +1950,15 @@ static PaError OpenStream( struct PaUtilHostApiRepresentation *hostApi, } /* now we can initialize the ring buffer */ - PaUtil_InitializeRingBuffer( &stream->inputRingBuffer, szfl*inputParameters->channelCount, ringSize, data ) ; + result = PaUtil_InitializeRingBuffer( &stream->inputRingBuffer, szfl*inputParameters->channelCount, ringSize, data ); + if( result != 0 ) + { + /* The only reason this should fail is if ringSize is not a power of 2, which we do not anticipate happening. */ + result = paUnanticipatedHostError; + free(data); + goto error; + } + /* advance the read point a little, so we are reading from the middle of the buffer */ if( stream->outputUnit ) @@ -1973,12 +1980,11 @@ static PaError OpenStream( struct PaUtilHostApiRepresentation *hostApi, stream->outputFramesPerBuffer, sampleRate ); result = initializeBlioRingBuffers( &stream->blio, - inputParameters?inputParameters->sampleFormat:0 , - outputParameters?outputParameters->sampleFormat:0 , - MAX(stream->inputFramesPerBuffer,stream->outputFramesPerBuffer), + inputParameters ? inputParameters->sampleFormat : 0, + outputParameters ? outputParameters->sampleFormat : 0, ringSize, - inputParameters?inputChannelCount:0 , - outputParameters?outputChannelCount:0 ) ; + inputParameters ? inputChannelCount : 0, + outputParameters ? outputChannelCount : 0 ) ; if( result != paNoError ) goto error; diff --git a/src/hostapi/coreaudio/pa_mac_core_blocking.c b/src/hostapi/coreaudio/pa_mac_core_blocking.c index e366aa3..485c373 100644 --- a/src/hostapi/coreaudio/pa_mac_core_blocking.c +++ b/src/hostapi/coreaudio/pa_mac_core_blocking.c @@ -108,14 +108,16 @@ static size_t computeSampleSizeFromFormatPow2( PaSampleFormat format ) * */ -/* This should be called with the relevant info when initializing a stream for - callback. */ +/** + * This should be called with the relevant info when initializing a stream for callback. + * + * @param ringBufferSizeInFrames must be a power of 2 + */ PaError initializeBlioRingBuffers( PaMacBlio *blio, PaSampleFormat inputSampleFormat, PaSampleFormat outputSampleFormat, - size_t framesPerBuffer, - long ringBufferSize, + long ringBufferSizeInFrames, int inChan, int outChan ) { @@ -126,20 +128,19 @@ PaError initializeBlioRingBuffers( /* zeroify things */ bzero( blio, sizeof( PaMacBlio ) ); /* this is redundant, but the buffers are used to check - if the bufffers have been initialized, so we do it explicitly. */ + if the buffers have been initialized, so we do it explicitly. */ blio->inputRingBuffer.buffer = NULL; blio->outputRingBuffer.buffer = NULL; /* initialize simple data */ - blio->ringBufferFrames = ringBufferSize; + blio->ringBufferFrames = ringBufferSizeInFrames; blio->inputSampleFormat = inputSampleFormat; blio->inputSampleSizeActual = computeSampleSizeFromFormat(inputSampleFormat); - blio->inputSampleSizePow2 = computeSampleSizeFromFormatPow2(inputSampleFormat); + blio->inputSampleSizePow2 = computeSampleSizeFromFormatPow2(inputSampleFormat); // FIXME: WHY? blio->outputSampleFormat = outputSampleFormat; blio->outputSampleSizeActual = computeSampleSizeFromFormat(outputSampleFormat); blio->outputSampleSizePow2 = computeSampleSizeFromFormatPow2(outputSampleFormat); - blio->framesPerBuffer = framesPerBuffer; blio->inChan = inChan; blio->outChan = outChan; blio->statusFlags = 0; @@ -163,7 +164,7 @@ PaError initializeBlioRingBuffers( result = UNIX_ERR( pthread_cond_init( &(blio->outputCond), NULL ) ); #endif if( inChan ) { - data = calloc( ringBufferSize, blio->inputSampleSizePow2*inChan ); + data = calloc( ringBufferSizeInFrames, blio->inputSampleSizePow2 * inChan ); if( !data ) { result = paInsufficientMemory; @@ -172,12 +173,13 @@ PaError initializeBlioRingBuffers( err = PaUtil_InitializeRingBuffer( &blio->inputRingBuffer, - 1, ringBufferSize*blio->inputSampleSizePow2*inChan, + blio->inputSampleSizePow2 * inChan, + ringBufferSizeInFrames, data ); assert( !err ); } if( outChan ) { - data = calloc( ringBufferSize, blio->outputSampleSizePow2*outChan ); + data = calloc( ringBufferSizeInFrames, blio->outputSampleSizePow2 * outChan ); if( !data ) { result = paInsufficientMemory; @@ -186,7 +188,8 @@ PaError initializeBlioRingBuffers( err = PaUtil_InitializeRingBuffer( &blio->outputRingBuffer, - 1, ringBufferSize*blio->outputSampleSizePow2*outChan, + blio->outputSampleSizePow2 * outChan, + ringBufferSizeInFrames, data ); assert( !err ); } @@ -266,12 +269,11 @@ PaError resetBlioRingBuffers( PaMacBlio *blio ) #endif blio->statusFlags = 0; if( blio->outputRingBuffer.buffer ) { - PaUtil_FlushRingBuffer( &blio->outputRingBuffer ); - bzero( blio->outputRingBuffer.buffer, - blio->outputRingBuffer.bufferSize ); - /* Advance buffer */ - PaUtil_AdvanceRingBufferWriteIndex( &blio->outputRingBuffer, blio->ringBufferFrames*blio->outputSampleSizeActual*blio->outChan ); - //PaUtil_AdvanceRingBufferWriteIndex( &blio->outputRingBuffer, blio->outputRingBuffer.bufferSize ); + PaUtil_FlushRingBuffer( &blio->outputRingBuffer ); + /* Fill the buffer with zeros. */ + bzero( blio->outputRingBuffer.buffer, + blio->outputRingBuffer.bufferSize * blio->outputRingBuffer.elementSizeBytes ); + PaUtil_AdvanceRingBufferWriteIndex( &blio->outputRingBuffer, blio->ringBufferFrames ); /* Update isOutputFull. */ #ifdef PA_MAC__BLIO_MUTEX @@ -280,16 +282,14 @@ PaError resetBlioRingBuffers( PaMacBlio *blio ) goto error; #endif /* - printf( "------%d\n" , blio->framesPerBuffer ); printf( "------%d\n" , blio->outChan ); printf( "------%d\n" , blio->outputSampleSize ); - printf( "------%d\n" , blio->framesPerBuffer*blio->outChan*blio->outputSampleSize ); */ } if( blio->inputRingBuffer.buffer ) { PaUtil_FlushRingBuffer( &blio->inputRingBuffer ); bzero( blio->inputRingBuffer.buffer, - blio->inputRingBuffer.bufferSize ); + blio->inputRingBuffer.bufferSize * blio->inputRingBuffer.elementSizeBytes ); /* Update isInputEmpty. */ #ifdef PA_MAC__BLIO_MUTEX result = blioSetIsInputEmpty( blio, true ); @@ -344,30 +344,32 @@ int BlioCallback( const void *input, void *output, unsigned long frameCount, void *userData ) { PaMacBlio *blio = (PaMacBlio*)userData; - long avail; - long toRead; - long toWrite; - long read; - long written; + ring_buffer_size_t framesAvailable; + ring_buffer_size_t framesToTransfer; + ring_buffer_size_t framesTransferred; /* set flags returned by OS: */ OSAtomicOr32( statusFlags, &blio->statusFlags ) ; /* --- Handle Input Buffer --- */ if( blio->inChan ) { - avail = PaUtil_GetRingBufferWriteAvailable( &blio->inputRingBuffer ); + framesAvailable = PaUtil_GetRingBufferWriteAvailable( &blio->inputRingBuffer ); /* check for underflow */ - if( avail < frameCount * blio->inputSampleSizeActual * blio->inChan ) + if( framesAvailable < frameCount ) { - OSAtomicOr32( paInputOverflow, &blio->statusFlags ); + OSAtomicOr32( paInputOverflow, &blio->statusFlags ); + framesToTransfer = framesAvailable; + } + else + { + framesToTransfer = (ring_buffer_size_t)frameCount; } - toRead = MIN( avail, frameCount * blio->inputSampleSizeActual * blio->inChan ); - /* copy the data */ + /* Copy the data from the audio input to the application ring buffer. */ /*printf( "reading %d\n", toRead );*/ - read = PaUtil_WriteRingBuffer( &blio->inputRingBuffer, input, toRead ); - assert( toRead == read ); + framesTransferred = PaUtil_WriteRingBuffer( &blio->inputRingBuffer, input, framesToTransfer ); + assert( framesToTransfer == framesTransferred ); #ifdef PA_MAC__BLIO_MUTEX /* Priority inversion. See notes below. */ blioSetIsInputEmpty( blio, false ); @@ -377,21 +379,31 @@ int BlioCallback( const void *input, void *output, unsigned long frameCount, /* --- Handle Output Buffer --- */ if( blio->outChan ) { - avail = PaUtil_GetRingBufferReadAvailable( &blio->outputRingBuffer ); + framesAvailable = PaUtil_GetRingBufferReadAvailable( &blio->outputRingBuffer ); /* check for underflow */ - if( avail < frameCount * blio->outputSampleSizeActual * blio->outChan ) - OSAtomicOr32( paOutputUnderflow, &blio->statusFlags ); + if( framesAvailable < frameCount ) + { + /* zero out the end of the output buffer that we do not have data for */ + framesToTransfer = framesAvailable; - toWrite = MIN( avail, frameCount * blio->outputSampleSizeActual * blio->outChan ); + size_t bytesPerFrame = blio->outputSampleSizeActual * blio->outChan; + size_t offsetInBytes = framesToTransfer * bytesPerFrame; + size_t countInBytes = (frameCount - framesToTransfer) * bytesPerFrame; + bzero( ((char *)output) + offsetInBytes, countInBytes ); + + OSAtomicOr32( paOutputUnderflow, &blio->statusFlags ); + framesToTransfer = framesAvailable; + } + else + { + framesToTransfer = (ring_buffer_size_t)frameCount; + } - if( toWrite != frameCount * blio->outputSampleSizeActual * blio->outChan ) - bzero( ((char *)output)+toWrite, - frameCount * blio->outputSampleSizeActual * blio->outChan - toWrite ); /* copy the data */ /*printf( "writing %d\n", toWrite );*/ - written = PaUtil_ReadRingBuffer( &blio->outputRingBuffer, output, toWrite ); - assert( toWrite == written ); + framesTransferred = PaUtil_ReadRingBuffer( &blio->outputRingBuffer, output, framesToTransfer ); + assert( framesToTransfer == framesTransferred ); #ifdef PA_MAC__BLIO_MUTEX /* We have a priority inversion here. However, we will only have to wait if this was true and is now false, which means we've got @@ -406,24 +418,25 @@ int BlioCallback( const void *input, void *output, unsigned long frameCount, PaError ReadStream( PaStream* stream, void *buffer, - unsigned long frames ) + unsigned long framesRequested ) { PaMacBlio *blio = & ((PaMacCoreStream*)stream) -> blio; char *cbuf = (char *) buffer; PaError ret = paNoError; VVDBUG(("ReadStream()\n")); - while( frames > 0 ) { - long avail; - long toRead; + while( framesRequested > 0 ) { + ring_buffer_size_t framesAvailable; + ring_buffer_size_t framesToTransfer; + ring_buffer_size_t framesTransferred; do { - avail = PaUtil_GetRingBufferReadAvailable( &blio->inputRingBuffer ); + framesAvailable = PaUtil_GetRingBufferReadAvailable( &blio->inputRingBuffer ); /* printf( "Read Buffer is %%%g full: %ld of %ld.\n", 100 * (float)avail / (float) blio->inputRingBuffer.bufferSize, - avail, blio->inputRingBuffer.bufferSize ); + framesAvailable, blio->inputRingBuffer.bufferSize ); */ - if( avail == 0 ) { + if( framesAvailable == 0 ) { #ifdef PA_MAC_BLIO_MUTEX /**block when empty*/ ret = UNIX_ERR( pthread_mutex_lock( &blio->inputMutex ) ); @@ -441,14 +454,13 @@ PaError ReadStream( PaStream* stream, Pa_Sleep( PA_MAC_BLIO_BUSY_WAIT_SLEEP_INTERVAL ); #endif } - } while( avail == 0 ); - toRead = MIN( avail, frames * blio->inputSampleSizeActual * blio->inChan ); - toRead -= toRead % blio->inputSampleSizeActual * blio->inChan ; - PaUtil_ReadRingBuffer( &blio->inputRingBuffer, (void *)cbuf, toRead ); - cbuf += toRead; - frames -= toRead / ( blio->inputSampleSizeActual * blio->inChan ); - - if( toRead == avail ) { + } while( framesAvailable == 0 ); + framesToTransfer = MIN( framesAvailable, framesRequested ); + PaUtil_ReadRingBuffer( &blio->inputRingBuffer, (void *)cbuf, framesToTransfer ); + cbuf += framesToTransfer * blio->inputSampleSizeActual * blio->inChan; + framesRequested -= framesToTransfer; + + if( framesToTransfer == framesAvailable ) { #ifdef PA_MAC_BLIO_MUTEX /* we just emptied the buffer, so we need to mark it as empty. */ ret = blioSetIsInputEmpty( blio, true ); @@ -457,8 +469,10 @@ PaError ReadStream( PaStream* stream, /* of course, in the meantime, the callback may have put some sats in, so so check for that, too, to avoid a race condition. */ + /* FIXME - this does not seem to fix any race condition. */ if( PaUtil_GetRingBufferReadAvailable( &blio->inputRingBuffer ) ) { blioSetIsInputEmpty( blio, false ); + /* FIXME - why check? ret has not been set? */ if( ret ) return ret; } @@ -468,6 +482,7 @@ PaError ReadStream( PaStream* stream, /* Report either paNoError or paInputOverflowed. */ /* may also want to report other errors, but this is non-standard. */ + /* FIXME should not clobber ret, use if(blio->statusFlags & paInputOverflow) */ ret = blio->statusFlags & paInputOverflow; /* report underflow only once: */ @@ -482,25 +497,27 @@ PaError ReadStream( PaStream* stream, PaError WriteStream( PaStream* stream, const void *buffer, - unsigned long frames ) + unsigned long framesRequested ) { PaMacBlio *blio = & ((PaMacCoreStream*)stream) -> blio; char *cbuf = (char *) buffer; PaError ret = paNoError; VVDBUG(("WriteStream()\n")); - while( frames > 0 ) { - long avail = 0; - long toWrite; + while( framesRequested > 0 ) { + ring_buffer_size_t framesAvailable; + ring_buffer_size_t framesToTransfer; + ring_buffer_size_t framesTransferred; + do { - avail = PaUtil_GetRingBufferWriteAvailable( &blio->outputRingBuffer ); + framesAvailable = PaUtil_GetRingBufferWriteAvailable( &blio->outputRingBuffer ); /* printf( "Write Buffer is %%%g full: %ld of %ld.\n", 100 - 100 * (float)avail / (float) blio->outputRingBuffer.bufferSize, - avail, blio->outputRingBuffer.bufferSize ); + framesAvailable, blio->outputRingBuffer.bufferSize ); */ - if( avail == 0 ) { + if( framesAvailable == 0 ) { #ifdef PA_MAC_BLIO_MUTEX /*block while full*/ ret = UNIX_ERR( pthread_mutex_lock( &blio->outputMutex ) ); @@ -518,16 +535,15 @@ PaError WriteStream( PaStream* stream, Pa_Sleep( PA_MAC_BLIO_BUSY_WAIT_SLEEP_INTERVAL ); #endif } - } while( avail == 0 ); + } while( framesAvailable == 0 ); - toWrite = MIN( avail, frames * blio->outputSampleSizeActual * blio->outChan ); - toWrite -= toWrite % blio->outputSampleSizeActual * blio->outChan ; - PaUtil_WriteRingBuffer( &blio->outputRingBuffer, (void *)cbuf, toWrite ); - cbuf += toWrite; - frames -= toWrite / ( blio->outputSampleSizeActual * blio->outChan ); + framesToTransfer = MIN( framesAvailable, framesRequested ); + PaUtil_WriteRingBuffer( &blio->outputRingBuffer, (void *)cbuf, framesToTransfer ); + cbuf += framesToTransfer * blio->outputSampleSizeActual * blio->outChan; + framesRequested -= framesToTransfer; #ifdef PA_MAC_BLIO_MUTEX - if( toWrite == avail ) { + if( framesToTransfer == framesAvailable ) { /* we just filled up the buffer, so we need to mark it as filled. */ ret = blioSetIsOutputFull( blio, true ); if( ret ) @@ -536,6 +552,7 @@ PaError WriteStream( PaStream* stream, so check for that, too, to avoid a race condition. */ if( PaUtil_GetRingBufferWriteAvailable( &blio->outputRingBuffer ) ) { blioSetIsOutputFull( blio, false ); + /* FIXME remove or review this code, does not fix race, ret not set! */ if( ret ) return ret; } @@ -545,6 +562,7 @@ PaError WriteStream( PaStream* stream, /* Report either paNoError or paOutputUnderflowed. */ /* may also want to report other errors, but this is non-standard. */ + /* FIXME should not clobber ret, use if(blio->statusFlags & paInputOverflow) */ ret = blio->statusFlags & paOutputUnderflow; /* report underflow only once: */ @@ -562,11 +580,12 @@ PaError WriteStream( PaStream* stream, void waitUntilBlioWriteBufferIsFlushed( PaMacBlio *blio ) { if( blio->outputRingBuffer.buffer ) { - long avail = PaUtil_GetRingBufferWriteAvailable( &blio->outputRingBuffer ); - while( avail != blio->outputRingBuffer.bufferSize ) { - if( avail == 0 ) + /* FIXME loop until PaUtil_GetRingBufferReadAvailable==0 */ + ring_buffer_size_t framesAvailable = PaUtil_GetRingBufferWriteAvailable( &blio->outputRingBuffer ); + while( framesAvailable != blio->outputRingBuffer.bufferSize ) { + if( framesAvailable == 0 ) Pa_Sleep( PA_MAC_BLIO_BUSY_WAIT_SLEEP_INTERVAL ); - avail = PaUtil_GetRingBufferWriteAvailable( &blio->outputRingBuffer ); + framesAvailable = PaUtil_GetRingBufferWriteAvailable( &blio->outputRingBuffer ); } } } @@ -577,8 +596,7 @@ signed long GetStreamReadAvailable( PaStream* stream ) PaMacBlio *blio = & ((PaMacCoreStream*)stream) -> blio; VVDBUG(("GetStreamReadAvailable()\n")); - return PaUtil_GetRingBufferReadAvailable( &blio->inputRingBuffer ) - / ( blio->inputSampleSizeActual * blio->inChan ); + return PaUtil_GetRingBufferReadAvailable( &blio->inputRingBuffer ); } @@ -587,7 +605,6 @@ signed long GetStreamWriteAvailable( PaStream* stream ) PaMacBlio *blio = & ((PaMacCoreStream*)stream) -> blio; VVDBUG(("GetStreamWriteAvailable()\n")); - return PaUtil_GetRingBufferWriteAvailable( &blio->outputRingBuffer ) - / ( blio->outputSampleSizeActual * blio->outChan ); + return PaUtil_GetRingBufferWriteAvailable( &blio->outputRingBuffer ); } diff --git a/src/hostapi/coreaudio/pa_mac_core_blocking.h b/src/hostapi/coreaudio/pa_mac_core_blocking.h index a6f0ad5..c669e0b 100644 --- a/src/hostapi/coreaudio/pa_mac_core_blocking.h +++ b/src/hostapi/coreaudio/pa_mac_core_blocking.h @@ -64,7 +64,7 @@ #include "pa_mac_core_utilities.h" /* - * Number of miliseconds to busy wait whil waiting for data in blocking calls. + * Number of milliseconds to busy wait while waiting for data in blocking calls. */ #define PA_MAC_BLIO_BUSY_WAIT_SLEEP_INTERVAL (5) /* @@ -79,7 +79,7 @@ typedef struct { PaUtilRingBuffer inputRingBuffer; PaUtilRingBuffer outputRingBuffer; - size_t ringBufferFrames; + ring_buffer_size_t ringBufferFrames; PaSampleFormat inputSampleFormat; size_t inputSampleSizeActual; size_t inputSampleSizePow2; @@ -87,8 +87,6 @@ typedef struct { size_t outputSampleSizeActual; size_t outputSampleSizePow2; - size_t framesPerBuffer; - int inChan; int outChan; @@ -117,8 +115,7 @@ PaError initializeBlioRingBuffers( PaMacBlio *blio, PaSampleFormat inputSampleFormat, PaSampleFormat outputSampleFormat, - size_t framesPerBuffer, - long ringBufferSize, + long ringBufferSizeInFrames, int inChan, int outChan ); PaError destroyBlioRingBuffers( PaMacBlio *blio );