summaryrefslogtreecommitdiff
path: root/src/player.c
diff options
context:
space:
mode:
authorLars-Dominik Braun <lars@6xq.net>2018-03-15 12:53:56 +0100
committerLars-Dominik Braun <lars@6xq.net>2018-03-15 12:53:56 +0100
commit3c4d8f65896253a82e19adcbe2808a863a99f74f (patch)
treefc7c3fbe15027e752c8a7da3d42c9a37d4d72c87 /src/player.c
parent2e51a13fe816c0c0b02f7d7a19a4c739dcb66119 (diff)
downloadpianobar-3c4d8f65896253a82e19adcbe2808a863a99f74f.tar.gz
pianobar-3c4d8f65896253a82e19adcbe2808a863a99f74f.tar.bz2
pianobar-3c4d8f65896253a82e19adcbe2808a863a99f74f.zip
Properly protect player struct with mutex
The volatile keyword neither guarantees atomic access nor memory visibility[1]. Although this is usually not a problem on x86, it is incorrect to rely on this. Use mutex locks to protect all shared player variables and enforce memory visibility. [1] https://wiki.sei.cmu.edu/confluence/display/c/CON02-C.+Do+not+use+volatile+as+a+synchronization+primitive
Diffstat (limited to 'src/player.c')
-rw-r--r--src/player.c86
1 files changed, 72 insertions, 14 deletions
diff --git a/src/player.c b/src/player.c
index 9cbee69..977007d 100644
--- a/src/player.c
+++ b/src/player.c
@@ -1,5 +1,5 @@
/*
-Copyright (c) 2008-2014
+Copyright (c) 2008-2018
Lars-Dominik Braun <lars@6xq.net>
Permission is hereby granted, free of charge, to any person obtaining a copy
@@ -66,19 +66,46 @@ static void printError (const BarSettings_t * const settings,
* XXX: in theory we can select the filters/formats we want to support, but
* this does not work in practise.
*/
-void BarPlayerInit () {
+void BarPlayerInit (player_t * const p, const BarSettings_t * const settings) {
ao_initialize ();
av_log_set_level (AV_LOG_FATAL);
av_register_all ();
avfilter_register_all ();
avformat_network_init ();
+
+ pthread_mutex_init (&p->lock, NULL);
+ pthread_cond_init (&p->cond, NULL);
+ BarPlayerReset (p);
+ p->settings = settings;
}
-void BarPlayerDestroy () {
+void BarPlayerDestroy (player_t * const p) {
+ pthread_cond_destroy (&p->cond);
+ pthread_mutex_destroy (&p->lock);
+
avformat_network_deinit ();
ao_shutdown ();
}
+void BarPlayerReset (player_t * const p) {
+ p->doQuit = false;
+ p->doPause = false;
+ p->songDuration = 0;
+ p->songPlayed = 0;
+ p->mode = PLAYER_DEAD;
+ p->fvolume = NULL;
+ p->fgraph = NULL;
+ p->fctx = NULL;
+ p->st = NULL;
+ p->cctx = NULL;
+ p->fbufsink = NULL;
+ p->fabuf = NULL;
+ p->streamIdx = -1;
+ p->lastTimestamp = 0;
+ p->interrupted = 0;
+ p->aoDev = NULL;
+}
+
/* Update volume filter
*/
void BarPlayerSetVolume (player_t * const player) {
@@ -122,7 +149,9 @@ static int intCb (void * const data) {
assert (player != NULL);
if (player->interrupted > 1) {
/* got a sigint multiple times, quit pianobar (handled by main.c). */
+ pthread_mutex_lock (&player->lock);
player->doQuit = true;
+ pthread_mutex_unlock (&player->lock);
return 1;
} else if (player->interrupted != 0) {
/* the request is retried with the same player context */
@@ -190,9 +219,12 @@ static bool openStream (player_t * const player) {
av_seek_frame (player->fctx, player->streamIdx, player->lastTimestamp, 0);
}
- player->songPlayed = 0;
- player->songDuration = av_q2d (player->st->time_base) *
+ const unsigned int songDuration = av_q2d (player->st->time_base) *
(double) player->st->duration;
+ pthread_mutex_lock (&player->lock);
+ player->songPlayed = 0;
+ player->songDuration = songDuration;
+ pthread_mutex_unlock (&player->lock);
return true;
}
@@ -283,6 +315,29 @@ static bool openDevice (player_t * const player) {
return true;
}
+/* Operating on shared variables and must be protected by mutex
+ */
+
+static bool shouldQuit (player_t * const player) {
+ pthread_mutex_lock (&player->lock);
+ const bool ret = player->doQuit;
+ pthread_mutex_unlock (&player->lock);
+ return ret;
+}
+
+static void changeMode (player_t * const player, unsigned int mode) {
+ pthread_mutex_lock (&player->lock);
+ player->mode = mode;
+ pthread_mutex_unlock (&player->lock);
+}
+
+BarPlayerMode BarPlayerGetMode (player_t * const player) {
+ pthread_mutex_lock (&player->lock);
+ const BarPlayerMode ret = player->mode;
+ pthread_mutex_unlock (&player->lock);
+ return ret;
+}
+
/* decode and play stream. returns 0 or av error code.
*/
static int play (player_t * const player) {
@@ -302,7 +357,7 @@ static int play (player_t * const player) {
enum { FILL, DRAIN, DONE } drainMode = FILL;
int ret = 0;
- while (!player->doQuit && drainMode != DONE) {
+ while (!shouldQuit (player) && drainMode != DONE) {
if (drainMode == FILL) {
ret = av_read_frame (player->fctx, &pkt);
if (ret == AVERROR_EOF) {
@@ -323,17 +378,17 @@ static int play (player_t * const player) {
}
/* pausing */
- pthread_mutex_lock (&player->pauseMutex);
+ pthread_mutex_lock (&player->lock);
if (player->doPause) {
av_read_pause (player->fctx);
do {
- pthread_cond_wait (&player->pauseCond, &player->pauseMutex);
+ pthread_cond_wait (&player->cond, &player->lock);
} while (player->doPause);
av_read_play (player->fctx);
}
- pthread_mutex_unlock (&player->pauseMutex);
+ pthread_mutex_unlock (&player->lock);
- while (!player->doQuit) {
+ while (!shouldQuit (player)) {
ret = avcodec_receive_frame (cctx, frame);
if (ret == AVERROR_EOF) {
/* done draining */
@@ -367,7 +422,10 @@ static int play (player_t * const player) {
}
}
- player->songPlayed = av_q2d (player->st->time_base) * (double) pkt.pts;
+ const unsigned int songPlayed = av_q2d (player->st->time_base) * (double) pkt.pts;
+ pthread_mutex_lock (&player->lock);
+ player->songPlayed = songPlayed;
+ pthread_mutex_unlock (&player->lock);
player->lastTimestamp = pkt.pts;
av_packet_unref (&pkt);
@@ -410,7 +468,7 @@ void *BarPlayerThread (void *data) {
retry = false;
if (openStream (player)) {
if (openFilter (player) && openDevice (player)) {
- player->mode = PLAYER_PLAYING;
+ changeMode (player, PLAYER_PLAYING);
BarPlayerSetVolume (player);
retry = play (player) == AVERROR_INVALIDDATA &&
!player->interrupted;
@@ -422,11 +480,11 @@ void *BarPlayerThread (void *data) {
/* stream not found */
pret = PLAYER_RET_SOFTFAIL;
}
- player->mode = PLAYER_WAITING;
+ changeMode (player, PLAYER_WAITING);
finish (player);
} while (retry);
- player->mode = PLAYER_FINISHED;
+ changeMode (player, PLAYER_FINISHED);
return (void *) pret;
}