From ef9c0117c98eef0657cf3f892428807f9a12a012 Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Wed, 28 Aug 2019 22:23:11 +0200 Subject: [PATCH] [input] Fix pipe playback bringing cpu to 100% When pipe playback is started, but no data is written to the pipe, the input loop would bring the cpu to 100%. This fix limits the loop like it was before player refactor. --- src/input.c | 65 +++++++++++++++++++++++++++--------------- src/input.h | 13 ++------- src/inputs/file_http.c | 4 --- src/inputs/pipe.c | 5 +--- 4 files changed, 45 insertions(+), 42 deletions(-) diff --git a/src/input.c b/src/input.c index a0903b36..3283fc1c 100644 --- a/src/input.c +++ b/src/input.c @@ -44,8 +44,8 @@ // Disallow further writes to the buffer when its size exceeds this threshold. // The below gives us room to buffer 2 seconds of 48000/16/2 audio. #define INPUT_BUFFER_THRESHOLD STOB(96000, 16, 2) -// How long (in sec) to wait for player read before looping -#define INPUT_LOOP_TIMEOUT 1 +// How long (in nsec) to wait when the input buffer is full before looping +#define INPUT_LOOP_TIMEOUT_NSEC 10000000 // How long (in sec) to keep an input open without the player reading from it #define INPUT_OPEN_TIMEOUT 600 @@ -134,7 +134,7 @@ static struct input_source input_now_reading; static struct input_buffer input_buffer; // Timeout waiting in playback loop -static struct timespec input_loop_timeout = { INPUT_LOOP_TIMEOUT, 0 }; +static struct timespec input_loop_timeout = { 0, INPUT_LOOP_TIMEOUT_NSEC }; // Timeout waiting for player read static struct timeval input_open_timeout = { INPUT_OPEN_TIMEOUT, 0 }; @@ -639,31 +639,13 @@ input_wait(void) pthread_mutex_lock(&input_buffer.mutex); - // Is the buffer full? Then wait for a read or for loop_timeout to elapse - if (evbuffer_get_length(input_buffer.evbuf) > INPUT_BUFFER_THRESHOLD) - { - buffer_full_cb(); - - ts = timespec_reltoabs(input_loop_timeout); - pthread_cond_timedwait(&input_buffer.cond, &input_buffer.mutex, &ts); - - if (evbuffer_get_length(input_buffer.evbuf) > INPUT_BUFFER_THRESHOLD) - { - pthread_mutex_unlock(&input_buffer.mutex); - return -1; - } - } + ts = timespec_reltoabs(input_loop_timeout); + pthread_cond_timedwait(&input_buffer.cond, &input_buffer.mutex, &ts); pthread_mutex_unlock(&input_buffer.mutex); return 0; } -/*void -input_next(void) -{ - commands_exec_async(cmdbase, next, NULL); -}*/ - /* ---------------------------------- MAIN ---------------------------------- */ /* Thread: input */ @@ -694,6 +676,32 @@ input(void *arg) pthread_exit(NULL); } +static int +wait_buffer_ready(void) +{ + struct timespec ts; + + pthread_mutex_lock(&input_buffer.mutex); + + // Is the buffer full? Then wait for a read or for loop_timeout to elapse + if (evbuffer_get_length(input_buffer.evbuf) > INPUT_BUFFER_THRESHOLD) + { + buffer_full_cb(); + + ts = timespec_reltoabs(input_loop_timeout); + pthread_cond_timedwait(&input_buffer.cond, &input_buffer.mutex, &ts); + + if (evbuffer_get_length(input_buffer.evbuf) > INPUT_BUFFER_THRESHOLD) + { + pthread_mutex_unlock(&input_buffer.mutex); + return -1; + } + } + + pthread_mutex_unlock(&input_buffer.mutex); + return 0; +} + static void play(evutil_socket_t fd, short flags, void *arg) { @@ -705,6 +713,17 @@ play(evutil_socket_t fd, short flags, void *arg) if (!inputs[input_now_reading.type]->play) return; + // If the buffer is full we wait until either the player has consumed enough + // data or INPUT_LOOP_TIMEOUT has elapsed (so we don't hang the input event + // thread when the player doesn't consume data quickly). If the return is + // negative then the buffer is still full, so we loop. + ret = wait_buffer_ready(); + if (ret < 0) + { + event_add(input_ev, &tv); + return; + } + // Return will be negative if there is an error or EOF. Here, we just don't // loop any more. input_write() will pass the message to the player. ret = inputs[input_now_reading.type]->play(&input_now_reading); diff --git a/src/input.h b/src/input.h index 38635f0e..0e0c6afa 100644 --- a/src/input.h +++ b/src/input.h @@ -143,21 +143,12 @@ int input_write(struct evbuffer *evbuf, struct media_quality *quality, short flags); /* - * Input modules can use this to wait for the input_buffer to be ready for - * writing. The wait is max INPUT_LOOP_TIMEOUT, which allows the event base to - * loop and process pending commands once in a while. + * Input modules can use this to wait for the player to read, so the module's + * playback-loop doesn't spin out of control. */ int input_wait(void); -/* - * Async switch to the next song in the queue. Mostly for internal use, but - * might be relevant some day externally? - */ -//void -//input_next(void); - - /* ---------------------- Interface towards player thread ------------------- */ /* Thread: player */ diff --git a/src/inputs/file_http.c b/src/inputs/file_http.c index 9d6d8a38..01ff0e20 100644 --- a/src/inputs/file_http.c +++ b/src/inputs/file_http.c @@ -87,10 +87,6 @@ play(struct input_source *source) int ret; short flags; - ret = input_wait(); - if (ret < 0) - return 0; // Loop, input_buffer is not ready for writing - // We set "wanted" to 1 because the read size doesn't matter to us // TODO optimize? ret = transcode(source->evbuf, &icy_timer, ctx, 1); diff --git a/src/inputs/pipe.c b/src/inputs/pipe.c index 9e394821..ae5a1c83 100644 --- a/src/inputs/pipe.c +++ b/src/inputs/pipe.c @@ -872,10 +872,6 @@ play(struct input_source *source) short flags; int ret; - ret = input_wait(); - if (ret < 0) - return 0; // Loop, input_buffer is not ready for writing - ret = evbuffer_read(source->evbuf, pipe->fd, PIPE_READ_MAX); if ((ret == 0) && (pipe->is_autostarted)) { @@ -885,6 +881,7 @@ play(struct input_source *source) } else if ((ret == 0) || ((ret < 0) && (errno == EAGAIN))) { + input_wait(); return 0; // Loop } else if (ret < 0)