sbitx_sound.c: sound_thread_start function: possible code 'smell'

[g8kig]

Can you clarify the purpose of the sleep(1) in the sound_thread_start function.

	pthread_create( &sound_thread, NULL, sound_thread_function, (void*)device);
	sleep(1);
	pthread_create( &loopback_thread, NULL, loopback_thread_function, (void*)device);

If this is attempting to get the sound_thread function running before loopback_thread runs it will not work as intended. This is because there are a large number of threads potentially runnable as well as sound_thread. Using sleep or any of the related functions is always a code 'smell'

[afarhan]

i wrote that to allow the first thread to open up the alsa device before the loopback is initialized. what alternative do you suggest?

[g8kig]

1. Declare a condition variable and a mutex along with the thread 'handles'

pthread_t sound_thread, loopback_thread;
static pthread_mutex_t sound_mutex;
static pthread_cond_t sound_cond_var;

2. Initialise the mutex and condition variable at the same time as creating the threads

pthread_mutex_init(&sound_mutex, NULL);
pthread_cond_init(&sound_cond_var, NULL);
pthread_create( &sound_thread, NULL, sound_thread_function, device);

Note: Any of these functions could potentially fail. Best practice would check the return codes (if any).

3). In the thread that must wait for the alsa device to be opened before the loopback is initialised, wait on the condition variable

 pthread_mutex_lock(&sound_mutex);
 pthread_cond_wait(&sound_cond_var, &sound_mutex);
 pthread_mutex_unlock(&sound_mutex);

note: pthead_cond_wait unlocks and locks the mutex as required.

4. In the sound thread when the alsa device is open, signal the condition variable allowing the loopback thread to run

 pthread_mutex_lock(&sound_mutex);
 pthread_cond_signal(&sound_cond_var);
 pthread_mutex_unlock(&sound_mutex);

5). Correctly the threads, condition variables should be closed (finalisation) however everything should be cleaned up when the program exits.

pthread_join(sound_thread, NULL);
pthread_join(loopback_thread, NULL);
pthread_cond_destroy(&sound_cond_var);
pthread_mutex_destroy(&sound_mutex);

[rafael2k]

I agree this is good solution. For less code verbosity, an atomic variable and spinlock could be used too. G8KIG, could you make a PR? I could review it.

[g8kig]

It's already done and tested in my fork. I will do a PR for everything when things settle down.

[rafael2k]

Cool! Just realized about your fork.