From 7df9371491e96a99c1e463f7787aede352ac5a37 Mon Sep 17 00:00:00 2001 From: Lars-Dominik Braun Date: Fri, 20 Apr 2012 11:49:02 +0200 Subject: 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. --- src/main.c | 20 ++++----- src/player.c | 135 ++++++++++++++++++++++++++++++++--------------------------- src/player.h | 13 +++--- src/ui_act.c | 25 +++++++---- 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 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 #include #include #include #include #include +#include #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 #include -#include #include +#include #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 -- cgit v1.2.3