From 2c5bcc5ee10d6e982daa6660ba1925b3d95a2922 Mon Sep 17 00:00:00 2001 From: Lars-Dominik Braun Date: Sat, 26 May 2012 15:06:05 +0200 Subject: Revert "Remove pause mutex/add pthread cleanup function" This reverts commit 7df9371491e96a99c1e463f7787aede352ac5a37. --- src/main.c | 20 +++++---- src/player.c | 135 +++++++++++++++++++++++++++-------------------------------- src/player.h | 13 +++--- src/ui_act.c | 25 ++++------- 4 files changed, 87 insertions(+), 106 deletions(-) (limited to 'src') diff --git a/src/main.c b/src/main.c index 5050b38..3e7ac54 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) { +static void BarMainStartPlayback (BarApp_t *app, pthread_t *playerThread) { BarUiPrintSong (&app->settings, app->playlist, app->curStation->isQuickMix ? PianoFindStationById (app->ph.stations, app->playlist->stationId) : NULL); @@ -216,24 +216,26 @@ static void BarMainStartPlayback (BarApp_t *app) { * thread has been started */ app->player.mode = PLAYER_STARTING; /* start player */ - pthread_create (&app->player.thread, NULL, BarPlayerThread, + pthread_create (playerThread, NULL, BarPlayerThread, &app->player); } } /* player is done, clean up */ -static void BarMainPlayerCleanup (BarApp_t *app) { +static void BarMainPlayerCleanup (BarApp_t *app, pthread_t *playerThread) { + void *threadRet; + 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 (app->player.thread, NULL); + pthread_join (*playerThread, &threadRet); /* don't continue playback if thread reports error */ - if (app->player.ret != PLAYER_RET_OK) { + if (threadRet != (void *) PLAYER_RET_OK) { app->curStation = NULL; } @@ -263,6 +265,8 @@ 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); @@ -284,7 +288,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); + BarMainPlayerCleanup (app, &playerThread); } /* check whether player finished playing and start playing new @@ -301,7 +305,7 @@ static void BarMainLoop (BarApp_t *app) { } /* song ready to play */ if (app->playlist != NULL) { - BarMainStartPlayback (app); + BarMainStartPlayback (app, &playerThread); } } @@ -315,7 +319,7 @@ static void BarMainLoop (BarApp_t *app) { } if (app->player.mode != PLAYER_FREED) { - BarMainPlayerCleanup (app); + pthread_join (playerThread, NULL); } } diff --git a/src/player.c b/src/player.c index b743ee8..733a315 100644 --- a/src/player.c +++ b/src/player.c @@ -1,5 +1,5 @@ /* -Copyright (c) 2008-2012 +Copyright (c) 2008-2011 Lars-Dominik Braun Permission is hereby granted, free of charge, to any person obtaining a copy @@ -23,17 +23,12 @@ THE SOFTWARE. /* receive/play audio stream */ -#ifndef __FreeBSD__ -#define _POSIX_C_SOURCE 1 /* sigaction() */ -#endif - #include #include #include #include #include #include -#include #include "player.h" #include "config.h" @@ -42,6 +37,16 @@ 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 @@ -74,20 +79,6 @@ 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 @@ -133,6 +124,8 @@ 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; } @@ -166,6 +159,9 @@ 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) { @@ -209,7 +205,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->ret = PLAYER_RET_ERR; + player->aoError = 1; BarUiMsg (player->settings, MSG_ERR, "Cannot open audio device\n"); return WAITRESS_CB_RET_ERR; @@ -318,6 +314,8 @@ 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; } @@ -370,7 +368,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->ret = PLAYER_RET_ERR; + player->aoError = 1; BarUiMsg (player->settings, MSG_ERR, "Cannot open audio device\n"); return WAITRESS_CB_RET_ERR; @@ -397,6 +395,8 @@ 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,42 +407,6 @@ 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_* @@ -450,28 +414,18 @@ static void BarPlayerCleanup (void *data) { 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 @@ -500,8 +454,9 @@ void *BarPlayerThread (void *data) { #endif /* ENABLE_MAD */ default: + /* FIXME: leaks memory */ BarUiMsg (player->settings, MSG_ERR, "Unsupported audio format!\n"); - pthread_exit (PLAYER_RET_OK); + return PLAYER_RET_OK; break; } @@ -516,7 +471,39 @@ void *BarPlayerThread (void *data) { } while (wRet == WAITRESS_RET_PARTIAL_FILE || wRet == WAITRESS_RET_TIMEOUT || wRet == WAITRESS_RET_READ_ERR); - /* cleanup */ - pthread_cleanup_pop (!0); - return NULL; + 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; } diff --git a/src/player.h b/src/player.h index 2261d44..ac6853c 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,10 +64,6 @@ 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; @@ -105,11 +101,12 @@ struct audioPlayer { unsigned char *buffer; - bool paused; - pthread_t thread; + pthread_mutex_t pauseMutex; 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 2cc3197..0c9ed1b 100644 --- a/src/ui_act.c +++ b/src/ui_act.c @@ -23,14 +23,10 @@ THE SOFTWARE. /* functions responding to user's keystrokes */ -#ifndef __FreeBSD__ -#define _POSIX_C_SOURCE 200112L /* pthread_kill() */ -#endif - #include #include +#include #include -#include #include "ui.h" #include "ui_readline.h" @@ -53,9 +49,10 @@ THE SOFTWARE. static inline void BarUiDoSkipSong (struct audioPlayer *player) { assert (player != NULL); - if (player->mode != PLAYER_FINISHED_PLAYBACK && player->mode != PLAYER_FREED) { - pthread_cancel (player->thread); - } + player->doQuit = 1; + /* unlocking an unlocked mutex is forbidden by some implementations */ + pthread_mutex_trylock (&player->pauseMutex); + pthread_mutex_unlock (&player->pauseMutex); } /* transform station if necessary to allow changes like rename, rate, ... @@ -347,12 +344,9 @@ BarUiActCallback(BarUiActMoveSong) { /* pause */ BarUiActCallback(BarUiActPause) { - 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; + /* already locked => unlock/unpause */ + if (pthread_mutex_trylock (&app->player.pauseMutex) == EBUSY) { + pthread_mutex_unlock (&app->player.pauseMutex); } } @@ -495,9 +489,8 @@ BarUiActCallback(BarUiActSelectQuickMix) { /* quit */ BarUiActCallback(BarUiActQuit) { - /* cancels player thread */ - BarUiDoSkipSong (&app->player); app->doQuit = 1; + BarUiDoSkipSong (&app->player); } /* song history -- cgit v1.2.3