Remove pause mutex/add pthread cleanup function

No more mutex locking/checking for quit condition. Should (slightly)
increase responsiveness of the player thread. Closes #250.

4 files changed, 106 insertions, 87 deletions

diff --git a/src/main.c b/src/main.c
index 3e7ac54..5050b38 100644
--- a/src/main.c
+++ b/src/main.c
@@ -183,7 +183,7 @@ static void BarMainGetPlaylist (BarApp_t *app) {
 
 /*	start new player thread
  */
-static void BarMainStartPlayback (BarApp_t *app, pthread_t *playerThread) {
+static void BarMainStartPlayback (BarApp_t *app) {
 	BarUiPrintSong (&app->settings, app->playlist, app->curStation->isQuickMix ?
 			PianoFindStationById (app->ph.stations,
 			app->playlist->stationId) : NULL);
@@ -216,26 +216,24 @@ static void BarMainStartPlayback (BarApp_t *app, pthread_t *playerThread) {
 		 * thread has been started */
 		app->player.mode = PLAYER_STARTING;
 		/* start player */
-		pthread_create (playerThread, NULL, BarPlayerThread,
+		pthread_create (&app->player.thread, NULL, BarPlayerThread,
 				&app->player);
 	}
 }
 
 /*	player is done, clean up
  */
-static void BarMainPlayerCleanup (BarApp_t *app, pthread_t *playerThread) {
-	void *threadRet;
-
+static void BarMainPlayerCleanup (BarApp_t *app) {
 	BarUiStartEventCmd (&app->settings, "songfinish", app->curStation,
 			app->playlist, &app->player, app->ph.stations, PIANO_RET_OK,
 			WAITRESS_RET_OK);
 
 	/* FIXME: pthread_join blocks everything if network connection
 	 * is hung up e.g. */
-	pthread_join (*playerThread, &threadRet);
+	pthread_join (app->player.thread, NULL);
 
 	/* don't continue playback if thread reports error */
-	if (threadRet != (void *) PLAYER_RET_OK) {
+	if (app->player.ret != PLAYER_RET_OK) {
 		app->curStation = NULL;
 	}
 
@@ -265,8 +263,6 @@ static void BarMainPrintTime (BarApp_t *app) {
 /*	main loop
  */
 static void BarMainLoop (BarApp_t *app) {
-	pthread_t playerThread;
-
 	BarMainGetLoginCredentials (&app->settings, &app->input);
 
 	BarMainLoadProxy (&app->settings, &app->waith);
@@ -288,7 +284,7 @@ static void BarMainLoop (BarApp_t *app) {
 	while (!app->doQuit) {
 		/* song finished playing, clean up things/scrobble song */
 		if (app->player.mode == PLAYER_FINISHED_PLAYBACK) {
-			BarMainPlayerCleanup (app, &playerThread);
+			BarMainPlayerCleanup (app);
 		}
 
 		/* check whether player finished playing and start playing new
@@ -305,7 +301,7 @@ static void BarMainLoop (BarApp_t *app) {
 			}
 			/* song ready to play */
 			if (app->playlist != NULL) {
-				BarMainStartPlayback (app, &playerThread);
+				BarMainStartPlayback (app);
 			}
 		}
 
@@ -319,7 +315,7 @@ static void BarMainLoop (BarApp_t *app) {
 	}
 
 	if (app->player.mode != PLAYER_FREED) {
-		pthread_join (playerThread, NULL);
+		BarMainPlayerCleanup (app);
 	}
 }
 
diff --git a/src/player.c b/src/player.c
index 733a315..b743ee8 100644
--- a/src/player.c
+++ b/src/player.c
@@ -1,5 +1,5 @@
 /*
-Copyright (c) 2008-2011
+Copyright (c) 2008-2012
 	Lars-Dominik Braun <lars@6xq.net>
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
@@ -23,12 +23,17 @@ THE SOFTWARE.
 
 /* receive/play audio stream */
 
+#ifndef __FreeBSD__
+#define _POSIX_C_SOURCE 1 /* sigaction() */
+#endif
+
 #include <unistd.h>
 #include <string.h>
 #include <math.h>
 #include <stdint.h>
 #include <limits.h>
 #include <arpa/inet.h>
+#include <signal.h>
 
 #include "player.h"
 #include "config.h"
@@ -37,16 +42,6 @@ THE SOFTWARE.
 
 #define bigToHostEndian32(x) ntohl(x)
 
-/* wait while locked, but don't slow down main thread by keeping
- * locks too long */
-#define QUIT_PAUSE_CHECK \
-	pthread_mutex_lock (&player->pauseMutex); \
-	pthread_mutex_unlock (&player->pauseMutex); \
-	if (player->doQuit) { \
-		/* err => abort playback */ \
-		return WAITRESS_CB_RET_ERR; \
-	}
-
 /* pandora uses float values with 2 digits precision. Scale them by 100 to get
  * a "nice" integer */
 #define RG_SCALE_FACTOR 100
@@ -79,6 +74,20 @@ static inline signed short int applyReplayGain (const signed short int value,
 	}
 }
 
+/*	handles bogus signal BAR_PLAYER_SIGCONT
+ */
+static void BarPlayerNullHandler (int sig) {
+}
+
+/*	handler signal BAR_PLAYER_SIGSTOP and pauses player thread
+ */
+static void BarPlayerPauseHandler (int sig) {
+	/* for a reason I don’t know sigsuspend does not work here, so we use
+	 * pause, which should be fine as there are no other (expected) signals
+	 * than SIGCONT */
+	pause ();
+}
+
 /*	Refill player's buffer with dataSize of data
  *	@param player structure
  *	@param new data
@@ -124,8 +133,6 @@ static WaitressCbReturn_t BarPlayerAACCb (void *ptr, size_t size,
 	const char *data = ptr;
 	struct audioPlayer *player = stream;
 
-	QUIT_PAUSE_CHECK;
-
 	if (!BarPlayerBufferFill (player, data, size)) {
 		return WAITRESS_CB_RET_ERR;
 	}
@@ -159,9 +166,6 @@ static WaitressCbReturn_t BarPlayerAACCb (void *ptr, size_t size,
 					(unsigned long long int) player->channels;
 			player->bufferRead += frameInfo.bytesconsumed;
 			player->sampleSizeCurr++;
-			/* going through this loop can take up to a few seconds =>
-			 * allow earlier thread abort */
-			QUIT_PAUSE_CHECK;
 		}
 	} else {
 		if (player->mode == PLAYER_INITIALIZED) {
@@ -205,7 +209,7 @@ static WaitressCbReturn_t BarPlayerAACCb (void *ptr, size_t size,
 					if ((player->audioOutDevice = ao_open_live (audioOutDriver,
 							&format, NULL)) == NULL) {
 						/* we're not interested in the errno */
-						player->aoError = 1;
+						player->ret = PLAYER_RET_ERR;
 						BarUiMsg (player->settings, MSG_ERR,
 								"Cannot open audio device\n");
 						return WAITRESS_CB_RET_ERR;
@@ -314,8 +318,6 @@ static WaitressCbReturn_t BarPlayerMp3Cb (void *ptr, size_t size,
 	struct audioPlayer *player = stream;
 	size_t i;
 
-	QUIT_PAUSE_CHECK;
-
 	if (!BarPlayerBufferFill (player, data, size)) {
 		return WAITRESS_CB_RET_ERR;
 	}
@@ -368,7 +370,7 @@ static WaitressCbReturn_t BarPlayerMp3Cb (void *ptr, size_t size,
 			format.byte_format = AO_FMT_NATIVE;
 			if ((player->audioOutDevice = ao_open_live (audioOutDriver,
 					&format, NULL)) == NULL) {
-				player->aoError = 1;
+				player->ret = PLAYER_RET_ERR;
 				BarUiMsg (player->settings, MSG_ERR,
 						"Cannot open audio device\n");
 				return WAITRESS_CB_RET_ERR;
@@ -395,8 +397,6 @@ static WaitressCbReturn_t BarPlayerMp3Cb (void *ptr, size_t size,
 					(unsigned long long int) BAR_PLAYER_MS_TO_S_FACTOR /
 					(unsigned long long int) player->samplerate;
 		}
-
-		QUIT_PAUSE_CHECK;
 	} while (player->mp3Stream.error != MAD_ERROR_BUFLEN);
 
 	player->bufferRead += player->mp3Stream.next_frame - player->buffer;
@@ -407,6 +407,42 @@ static WaitressCbReturn_t BarPlayerMp3Cb (void *ptr, size_t size,
 }
 #endif /* ENABLE_MAD */
 
+/*	player cleanup function
+ * 	@param player structure
+ */
+static void BarPlayerCleanup (void *data) {
+	struct audioPlayer *player = data;
+
+	switch (player->audioFormat) {
+		#ifdef ENABLE_FAAD
+		case PIANO_AF_AACPLUS_LO:
+		case PIANO_AF_AACPLUS:
+			NeAACDecClose(player->aacHandle);
+			free (player->sampleSize);
+			break;
+		#endif /* ENABLE_FAAD */
+
+		#ifdef ENABLE_MAD
+		case PIANO_AF_MP3:
+		case PIANO_AF_MP3_HI:
+			mad_synth_finish (&player->mp3Synth);
+			mad_frame_finish (&player->mp3Frame);
+			mad_stream_finish (&player->mp3Stream);
+			break;
+		#endif /* ENABLE_MAD */
+
+		default:
+			/* this should never happen */
+			break;
+	}
+
+	ao_close(player->audioOutDevice);
+	WaitressFree (&player->waith);
+	free (player->buffer);
+
+	player->mode = PLAYER_FINISHED_PLAYBACK;
+}
+
 /*	player thread; for every song a new thread is started
  *	@param audioPlayer structure
  *	@return PLAYER_RET_*
@@ -414,18 +450,28 @@ static WaitressCbReturn_t BarPlayerMp3Cb (void *ptr, size_t size,
 void *BarPlayerThread (void *data) {
 	struct audioPlayer *player = data;
 	char extraHeaders[32];
-	void *ret = PLAYER_RET_OK;
 	#ifdef ENABLE_FAAD
 	NeAACDecConfigurationPtr conf;
 	#endif
 	WaitressReturn_t wRet = WAITRESS_RET_ERR;
+	struct sigaction sa;
+
+	/* set up pause signals */
+	memset (&sa, 0, sizeof (sa));
+	sa.sa_handler = BarPlayerPauseHandler;
+	sigaction (BAR_PLAYER_SIGSTOP, &sa, NULL);
+	memset (&sa, 0, sizeof (sa));
+	sa.sa_handler = BarPlayerNullHandler;
+	sigaction (BAR_PLAYER_SIGCONT, &sa, NULL);
+	/* set up cleanup function */
+	pthread_cleanup_push (BarPlayerCleanup, data);
 
 	/* init handles */
-	pthread_mutex_init (&player->pauseMutex, NULL);
 	player->waith.data = (void *) player;
 	/* extraHeaders will be initialized later */
 	player->waith.extraHeaders = extraHeaders;
 	player->buffer = malloc (BAR_PLAYER_BUFSIZE);
+	player->ret = PLAYER_RET_OK;
 
 	switch (player->audioFormat) {
 		#ifdef ENABLE_FAAD
@@ -454,9 +500,8 @@ void *BarPlayerThread (void *data) {
 		#endif /* ENABLE_MAD */
 
 		default:
-			/* FIXME: leaks memory */
 			BarUiMsg (player->settings, MSG_ERR, "Unsupported audio format!\n");
-			return PLAYER_RET_OK;
+			pthread_exit (PLAYER_RET_OK);
 			break;
 	}
 	
@@ -471,39 +516,7 @@ void *BarPlayerThread (void *data) {
 	} while (wRet == WAITRESS_RET_PARTIAL_FILE || wRet == WAITRESS_RET_TIMEOUT
 			|| wRet == WAITRESS_RET_READ_ERR);
 
-	switch (player->audioFormat) {
-		#ifdef ENABLE_FAAD
-		case PIANO_AF_AACPLUS_LO:
-		case PIANO_AF_AACPLUS:
-			NeAACDecClose(player->aacHandle);
-			free (player->sampleSize);
-			break;
-		#endif /* ENABLE_FAAD */
-
-		#ifdef ENABLE_MAD
-		case PIANO_AF_MP3:
-		case PIANO_AF_MP3_HI:
-			mad_synth_finish (&player->mp3Synth);
-			mad_frame_finish (&player->mp3Frame);
-			mad_stream_finish (&player->mp3Stream);
-			break;
-		#endif /* ENABLE_MAD */
-
-		default:
-			/* this should never happen: thread is aborted above */
-			break;
-	}
-
-	if (player->aoError) {
-		ret = (void *) PLAYER_RET_ERR;
-	}
-
-	ao_close(player->audioOutDevice);
-	WaitressFree (&player->waith);
-	pthread_mutex_destroy (&player->pauseMutex);
-	free (player->buffer);
-
-	player->mode = PLAYER_FINISHED_PLAYBACK;
-
-	return ret;
+	/* cleanup */
+	pthread_cleanup_pop (!0);
+	return NULL;
 }
diff --git a/src/player.h b/src/player.h
index ac6853c..2261d44 100644
--- a/src/player.h
+++ b/src/player.h
@@ -47,11 +47,11 @@ THE SOFTWARE.
 
 #define BAR_PLAYER_MS_TO_S_FACTOR 1000
 #define BAR_PLAYER_BUFSIZE (WAITRESS_BUFFER_SIZE*2)
+#define BAR_PLAYER_SIGSTOP SIGRTMIN
+#define BAR_PLAYER_SIGCONT SIGRTMIN+1
 
 struct audioPlayer {
-	char doQuit;
 	unsigned char channels;
-	unsigned char aoError;
 
 	enum {
 		PLAYER_FREED = 0, /* thread is not running */
@@ -64,6 +64,10 @@ struct audioPlayer {
 		PLAYER_RECV_DATA, /* playing track */
 		PLAYER_FINISHED_PLAYBACK
 	} mode;
+	enum {
+		PLAYER_RET_OK = 0,
+		PLAYER_RET_ERR = 1,
+	} ret;
 	PianoAudioFormat_t audioFormat;
 
 	unsigned int scale;
@@ -101,12 +105,11 @@ struct audioPlayer {
 
 	unsigned char *buffer;
 
-	pthread_mutex_t pauseMutex;
+	bool paused;
+	pthread_t thread;
 	WaitressHandle_t waith;
 };
 
-enum {PLAYER_RET_OK = 0, PLAYER_RET_ERR = 1};
-
 void *BarPlayerThread (void *data);
 unsigned int BarPlayerCalcScale (float);
 
diff --git a/src/ui_act.c b/src/ui_act.c
index 0c9ed1b..2cc3197 100644
--- a/src/ui_act.c
+++ b/src/ui_act.c
@@ -23,10 +23,14 @@ THE SOFTWARE.
 
 /* functions responding to user's keystrokes */
 
+#ifndef __FreeBSD__
+#define _POSIX_C_SOURCE 200112L /* pthread_kill() */
+#endif
+
 #include <string.h>
 #include <unistd.h>
-#include <pthread.h>
 #include <assert.h>
+#include <signal.h>
 
 #include "ui.h"
 #include "ui_readline.h"
@@ -49,10 +53,9 @@ THE SOFTWARE.
 static inline void BarUiDoSkipSong (struct audioPlayer *player) {
 	assert (player != NULL);
 
-	player->doQuit = 1;
-	/* unlocking an unlocked mutex is forbidden by some implementations */
-	pthread_mutex_trylock (&player->pauseMutex);
-	pthread_mutex_unlock (&player->pauseMutex);
+	if (player->mode != PLAYER_FINISHED_PLAYBACK && player->mode != PLAYER_FREED) {
+		pthread_cancel (player->thread);
+	}
 }
 
 /*	transform station if necessary to allow changes like rename, rate, ...
@@ -344,9 +347,12 @@ BarUiActCallback(BarUiActMoveSong) {
 /*	pause
  */
 BarUiActCallback(BarUiActPause) {
-	/* already locked => unlock/unpause */
-	if (pthread_mutex_trylock (&app->player.pauseMutex) == EBUSY) {
-		pthread_mutex_unlock (&app->player.pauseMutex);
+	if (app->player.paused) {
+		pthread_kill (app->player.thread, BAR_PLAYER_SIGCONT);
+		app->player.paused = false;
+	} else {
+		pthread_kill (app->player.thread, BAR_PLAYER_SIGSTOP);
+		app->player.paused = true;
 	}
 }
 
@@ -489,8 +495,9 @@ BarUiActCallback(BarUiActSelectQuickMix) {
 /*	quit
  */
 BarUiActCallback(BarUiActQuit) {
-	app->doQuit = 1;
+	/* cancels player thread */
 	BarUiDoSkipSong (&app->player);
+	app->doQuit = 1;
 }
 
 /*	song history