From 486479214010541c6d235c7b4ea040a1f0bf8e59 Mon Sep 17 00:00:00 2001 From: Ross Bencina Date: Sun, 12 May 2019 21:38:30 +1000 Subject: [PATCH 1/3] fix uninitialized dir variable in pa_linux_alsa.c GropeDevice() by removing it. pass NULL as parameter to ALSA functions instead. see ticket #263. --- src/hostapi/alsa/pa_linux_alsa.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/hostapi/alsa/pa_linux_alsa.c b/src/hostapi/alsa/pa_linux_alsa.c index 584cde8..b5bdd3a 100644 --- a/src/hostapi/alsa/pa_linux_alsa.c +++ b/src/hostapi/alsa/pa_linux_alsa.c @@ -842,7 +842,6 @@ static PaError GropeDevice( snd_pcm_t* pcm, int isPlug, StreamDirection mode, in double * defaultLowLatency, * defaultHighLatency, * defaultSampleRate = &devInfo->baseDeviceInfo.defaultSampleRate; double defaultSr = *defaultSampleRate; - int dir; assert( pcm ); @@ -920,7 +919,7 @@ static PaError GropeDevice( snd_pcm_t* pcm, int isPlug, StreamDirection mode, in alsaBufferFrames = 512; alsaPeriodFrames = 128; ENSURE_( alsa_snd_pcm_hw_params_set_buffer_size_near( pcm, hwParams, &alsaBufferFrames ), paUnanticipatedHostError ); - ENSURE_( alsa_snd_pcm_hw_params_set_period_size_near( pcm, hwParams, &alsaPeriodFrames, &dir ), paUnanticipatedHostError ); + ENSURE_( alsa_snd_pcm_hw_params_set_period_size_near( pcm, hwParams, &alsaPeriodFrames, NULL ), paUnanticipatedHostError ); *defaultLowLatency = (double) (alsaBufferFrames - alsaPeriodFrames) / defaultSr; /* Base the high latency case on values four times larger */ @@ -930,7 +929,7 @@ static PaError GropeDevice( snd_pcm_t* pcm, int isPlug, StreamDirection mode, in ENSURE_( alsa_snd_pcm_hw_params_any( pcm, hwParams ), paUnanticipatedHostError ); ENSURE_( SetApproximateSampleRate( pcm, hwParams, defaultSr ), paUnanticipatedHostError ); ENSURE_( alsa_snd_pcm_hw_params_set_buffer_size_near( pcm, hwParams, &alsaBufferFrames ), paUnanticipatedHostError ); - ENSURE_( alsa_snd_pcm_hw_params_set_period_size_near( pcm, hwParams, &alsaPeriodFrames, &dir ), paUnanticipatedHostError ); + ENSURE_( alsa_snd_pcm_hw_params_set_period_size_near( pcm, hwParams, &alsaPeriodFrames, NULL ), paUnanticipatedHostError ); *defaultHighLatency = (double) (alsaBufferFrames - alsaPeriodFrames) / defaultSr; *minChannels = (int)minChans; From 22a233161f3ab4eb5f0dd7dce32e68e6cb79b188 Mon Sep 17 00:00:00 2001 From: Ross Bencina Date: Sun, 12 May 2019 22:01:29 +1000 Subject: [PATCH 2/3] set dir=0 prior to passing its address in to ALSA function in _PA_LOCAL_IMPL(snd_pcm_hw_params_get_buffer_size_max) as is done elsewhere in the code. mark all places in the code where dir is not set dir=0 prior to passing in to ALSA function (i.e. where the previous value returned by ALSA is used. Are these omissions or is it intentional to use the rounding mode of the previous call? --- src/hostapi/alsa/pa_linux_alsa.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/hostapi/alsa/pa_linux_alsa.c b/src/hostapi/alsa/pa_linux_alsa.c index b5bdd3a..09a1e61 100644 --- a/src/hostapi/alsa/pa_linux_alsa.c +++ b/src/hostapi/alsa/pa_linux_alsa.c @@ -323,8 +323,10 @@ int _PA_LOCAL_IMPL(snd_pcm_hw_params_get_buffer_size_max) (const snd_pcm_hw_para snd_pcm_uframes_t pmax = 0; unsigned int pcnt = 0; + dir = 0; if(( ret = _PA_LOCAL_IMPL(snd_pcm_hw_params_get_period_size_max)(params, &pmax, &dir) ) < 0 ) return ret; + /* REVIEW: why no dir=0 here? is it intended to use the rounding mode of get_period_size_max? either set dir=0 or add explanatory comment. -- rossb 12 May 2019 */ if(( ret = _PA_LOCAL_IMPL(snd_pcm_hw_params_get_periods_max)(params, &pcnt, &dir) ) < 0 ) return ret; @@ -2323,6 +2325,7 @@ static PaError PaAlsaStreamComponent_DetermineFramesPerBuffer( PaAlsaStreamCompo /* It may be that the device only supports 2 periods for instance */ dir = 0; ENSURE_( alsa_snd_pcm_hw_params_get_periods_min( hwParams, &minPeriods, &dir ), paUnanticipatedHostError ); + /* REVIEW: why no dir=0 here? is it intended to use the rounding mode of get_periods_min? either set dir=0 or add explanatory comment. -- rossb 12 May 2019 */ ENSURE_( alsa_snd_pcm_hw_params_get_periods_max( hwParams, &maxPeriods, &dir ), paUnanticipatedHostError ); assert( maxPeriods > 1 ); @@ -3208,6 +3211,7 @@ error: unsigned int _min = 0, _max = 0; int _dir = 0; ENSURE_( alsa_snd_pcm_hw_params_get_rate_min( hwParams, &_min, &_dir ), paUnanticipatedHostError ); + /* REVIEW: why no dir=0 here? is it intended to use the rounding mode of get_rate_min? either set dir=0 or add explanatory comment -- rossb 12 May 2019 */ ENSURE_( alsa_snd_pcm_hw_params_get_rate_max( hwParams, &_max, &_dir ), paUnanticipatedHostError ); PA_DEBUG(( "%s: SR min = %u, max = %u, req = %u\n", __FUNCTION__, _min, _max, reqRate )); } From 4dea694220d6c83e830ce2a8bc41ca631de48d47 Mon Sep 17 00:00:00 2001 From: Ross Bencina Date: Sun, 12 May 2019 22:18:05 +1000 Subject: [PATCH 3/3] rename pa_linux_alsa.c local function GropeDevice() to more descriptive DetermineDeviceInfos(). update related comments. --- src/hostapi/alsa/pa_linux_alsa.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/hostapi/alsa/pa_linux_alsa.c b/src/hostapi/alsa/pa_linux_alsa.c index 09a1e61..287bc15 100644 --- a/src/hostapi/alsa/pa_linux_alsa.c +++ b/src/hostapi/alsa/pa_linux_alsa.c @@ -827,13 +827,13 @@ static void Terminate( struct PaUtilHostApiRepresentation *hostApi ) PaAlsa_CloseLibrary(); } -/** Determine max channels and default latencies. +/** Determine max channels, default latencies, default sample rate. * - * This function provides functionality to grope an opened (might be opened for capture or playback) pcm device for + * This function provides functionality to query an opened (might be opened for capture or playback) pcm device for * traits like max channels, suitable default latencies and default sample rate. Upon error, max channels is set to zero, * and a suitable result returned. The device is closed before returning. */ -static PaError GropeDevice( snd_pcm_t* pcm, int isPlug, StreamDirection mode, int openBlocking, +static PaError DetermineDeviceInfos( snd_pcm_t* pcm, int isPlug, StreamDirection mode, int openBlocking, PaAlsaDeviceInfo* devInfo ) { PaError result = paNoError; @@ -1167,10 +1167,10 @@ static PaError FillInDevInfo( PaAlsaHostApiRepresentation *alsaApi, HwDevInfo* d if( deviceHwInfo->hasCapture && OpenPcm( &pcm, deviceHwInfo->alsaName, SND_PCM_STREAM_CAPTURE, blocking, 0 ) >= 0 ) { - if( GropeDevice( pcm, deviceHwInfo->isPlug, StreamDirection_In, blocking, devInfo ) != paNoError ) + if( DetermineDeviceInfos( pcm, deviceHwInfo->isPlug, StreamDirection_In, blocking, devInfo ) != paNoError ) { /* Error */ - PA_DEBUG(( "%s: Failed groping %s for capture\n", __FUNCTION__, deviceHwInfo->alsaName )); + PA_DEBUG(( "%s: Failed querying %s for capture device info\n", __FUNCTION__, deviceHwInfo->alsaName )); goto end; } } @@ -1179,10 +1179,10 @@ static PaError FillInDevInfo( PaAlsaHostApiRepresentation *alsaApi, HwDevInfo* d if( deviceHwInfo->hasPlayback && OpenPcm( &pcm, deviceHwInfo->alsaName, SND_PCM_STREAM_PLAYBACK, blocking, 0 ) >= 0 ) { - if( GropeDevice( pcm, deviceHwInfo->isPlug, StreamDirection_Out, blocking, devInfo ) != paNoError ) + if( DetermineDeviceInfos( pcm, deviceHwInfo->isPlug, StreamDirection_Out, blocking, devInfo ) != paNoError ) { /* Error */ - PA_DEBUG(( "%s: Failed groping %s for playback\n", __FUNCTION__, deviceHwInfo->alsaName )); + PA_DEBUG(( "%s: Failed querying %s for playback device info\n", __FUNCTION__, deviceHwInfo->alsaName )); goto end; } }