From 6db5467e74c03e486e0f3bfae52c1a8985901dc8 Mon Sep 17 00:00:00 2001 From: Trent Piepho Date: Tue, 12 Jan 2021 08:32:14 -0800 Subject: [PATCH 1/2] Fix race with light mode switch and audio callback When light mode is toggled, it's possible for the buffer pointer or timestamp to get messed up if it happens while the callback is running. The callback will read the current values when it starts, then light mode switch resets them to zero, and then when the callback finishes it updates them based on the original values, effectvely undoing to reset to zero. This doesn't cause a crash, but it does mean the timestamp isn't quite right. It also will cause problems if any code wants to process the audio data as it arrives. It could cause a single callback to be processed with the incorrect high pass filter. The easiest way to fix this is to pause the input stream while doing the mode switch. It could be fixed by making the entire callback a critical section or having the mode switch done asyncrounsly via the callback, but this makes the callback more complex and adds overhead. It seems better to add the extra work to the mode switch rather than the callback, since the former is a rare operation that isn't performance critical while the latter is. --- src/audio.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/audio.c b/src/audio.c index 05574fe..5e7cea1 100644 --- a/src/audio.c +++ b/src/audio.c @@ -25,6 +25,9 @@ int write_pointer = 0; uint64_t timestamp = 0; pthread_mutex_t audio_mutex; +/* Audio input stream object */ +static PaStream *stream; + /* Data for PA callback to use */ static struct callback_info { int channels; //!< Number of channels @@ -90,8 +93,6 @@ static int paudio_callback(const void *input_buffer, int start_portaudio(int *nominal_sample_rate, double *real_sample_rate) { - PaStream *stream; - if(pthread_mutex_init(&audio_mutex,NULL)) { error("Failed to setup audio mutex"); return 1; @@ -235,11 +236,18 @@ int analyze_pa_data_cal(struct processing_data *pd, struct calibration_data *cd) void set_audio_light(bool light) { if(info.light != light) { + Pa_StopStream(stream); pthread_mutex_lock(&audio_mutex); + info.light = light; memset(pa_buffers, 0, sizeof(pa_buffers)); write_pointer = 0; timestamp = 0; + pthread_mutex_unlock(&audio_mutex); + + PaError err = Pa_StartStream(stream); + if(err != paNoError) + error("Error re-starting audio input: %s", Pa_GetErrorText(err)); } } From 4282a1c870f14cb36a41689d50617edfa35ca358 Mon Sep 17 00:00:00 2001 From: Trent Piepho Date: Tue, 12 Jan 2021 08:19:01 -0800 Subject: [PATCH 2/2] Read config file before starting audio This way settings that affect audio can be read in before audio starts. Since the config file is now read before starting audio, it is possible to start in light or normal mode as configured. If light mode was configured, then previously audio would be started in normal mode and then immediatly switch to light mode after processing a few callbacks. --- src/audio.c | 4 ++-- src/interface.c | 10 +++++----- src/tg.h | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/audio.c b/src/audio.c index 5e7cea1..05890c2 100644 --- a/src/audio.c +++ b/src/audio.c @@ -91,7 +91,7 @@ static int paudio_callback(const void *input_buffer, return 0; } -int start_portaudio(int *nominal_sample_rate, double *real_sample_rate) +int start_portaudio(int *nominal_sample_rate, double *real_sample_rate, bool light) { if(pthread_mutex_init(&audio_mutex,NULL)) { error("Failed to setup audio mutex"); @@ -122,7 +122,7 @@ int start_portaudio(int *nominal_sample_rate, double *real_sample_rate) } if(channels > 2) channels = 2; info.channels = channels; - info.light = false; + info.light = light; err = Pa_OpenDefaultStream(&stream,channels,0,paFloat32,PA_SAMPLE_RATE,paFramesPerBufferUnspecified,paudio_callback,&info); if(err!=paNoError) goto error; diff --git a/src/interface.c b/src/interface.c index 7c1d3ca..55c564d 100644 --- a/src/interface.c +++ b/src/interface.c @@ -917,11 +917,6 @@ static void start_interface(GApplication* app, void *p) struct main_window *w = malloc(sizeof(struct main_window)); - if(start_portaudio(&w->nominal_sr, &real_sr)) { - g_application_quit(app); - return; - } - w->app = GTK_APPLICATION(app); w->zombie = 0; @@ -934,6 +929,11 @@ static void start_interface(GApplication* app, void *p) load_config(w); + if(start_portaudio(&w->nominal_sr, &real_sr, w->is_light)) { + g_application_quit(app); + return; + } + if(w->la < MIN_LA || w->la > MAX_LA) w->la = DEFAULT_LA; if(w->bph < MIN_BPH || w->bph > MAX_BPH) w->bph = 0; if(w->cal < MIN_CAL || w->cal > MAX_CAL) diff --git a/src/tg.h b/src/tg.h index 789f66c..24eecea 100644 --- a/src/tg.h +++ b/src/tg.h @@ -125,7 +125,7 @@ struct processing_data { int is_light; }; -int start_portaudio(int *nominal_sample_rate, double *real_sample_rate); +int start_portaudio(int *nominal_sample_rate, double *real_sample_rate, bool light); int terminate_portaudio(); uint64_t get_timestamp(int light); int analyze_pa_data(struct processing_data *pd, int bph, double la, uint64_t events_from);