From 436a1d4012553a2f33d0e3a5180b3b5ae0378bdd Mon Sep 17 00:00:00 2001 From: Lars-Dominik Braun Date: Fri, 23 Oct 2015 10:27:31 +0200 Subject: Interruptible requests ^C now works as expected: It interrupts API requests, input prompts and audio streaming. Timeouts have been removed. --- src/config.h | 5 --- src/main.c | 26 +++++++++++- src/main.h | 6 +-- src/player.c | 40 +++++-------------- src/player.h | 5 +-- src/ui.c | 117 +++++++++++++++++++++++++++++------------------------- src/ui.h | 2 +- src/ui_readline.c | 37 ++++++++++------- src/ui_readline.h | 2 + 9 files changed, 129 insertions(+), 111 deletions(-) diff --git a/src/config.h b/src/config.h index 788455c..db3b66e 100644 --- a/src/config.h +++ b/src/config.h @@ -14,11 +14,6 @@ * ffmpeg and libav */ #include -/* is "timeout" option present (all versions of ffmpeg, not libav) */ -#if LIBAVFILTER_VERSION_MICRO >= 100 -#define HAVE_AV_TIMEOUT -#endif - /* does graph_send_command exist (ffmpeg >=2.2) */ #if LIBAVFILTER_VERSION_MAJOR >= 4 && \ LIBAVFILTER_VERSION_MICRO >= 100 diff --git a/src/main.c b/src/main.c index b113f4e..5d0e91b 100644 --- a/src/main.c +++ b/src/main.c @@ -199,7 +199,7 @@ static void BarMainGetInitialStation (BarApp_t *app) { static void BarMainHandleUserInput (BarApp_t *app) { char buf[2]; if (BarReadline (buf, sizeof (buf), NULL, &app->input, - BAR_RL_FULLRETURN | BAR_RL_NOECHO, 1) > 0) { + BAR_RL_FULLRETURN | BAR_RL_NOECHO | BAR_RL_NOINT, 1) > 0) { BarUiDispatch (app, buf[0], app->curStation, app->playlist, true, BAR_DC_GLOBAL); } @@ -259,6 +259,9 @@ static void BarMainStartPlayback (BarApp_t *app, pthread_t *playerThread) { pthread_mutex_init (&app->player.pauseMutex, NULL); pthread_cond_init (&app->player.pauseCond, NULL); + assert (interrupted == NULL); + interrupted = &app->player.interrupted; + /* throw event */ BarUiStartEventCmd (&app->settings, "songstart", app->curStation, curSong, &app->player, app->ph.stations, @@ -301,6 +304,9 @@ static void BarMainPlayerCleanup (BarApp_t *app, pthread_t *playerThread) { } memset (&app->player, 0, sizeof (app->player)); + + assert (interrupted == &app->player.interrupted); + interrupted = NULL; } /* print song duration @@ -384,6 +390,23 @@ static void BarMainLoop (BarApp_t *app) { } } +sig_atomic_t *interrupted = NULL; + +static void intHandler (int signal) { + if (interrupted != NULL) { + *interrupted = 1; + } +} + +static void BarMainSetupSigaction () { + struct sigaction act = { + .sa_handler = intHandler, + .sa_flags = 0, + }; + sigemptyset (&act.sa_mask); + sigaction (SIGINT, &act, NULL); +} + int main (int argc, char **argv) { static BarApp_t app; @@ -391,6 +414,7 @@ int main (int argc, char **argv) { /* save terminal attributes, before disabling echoing */ BarTermInit (); + BarMainSetupSigaction (); /* signals */ signal (SIGPIPE, SIG_IGN); diff --git a/src/main.h b/src/main.h index c8134ae..d1e71ec 100644 --- a/src/main.h +++ b/src/main.h @@ -21,8 +21,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -#ifndef SRC_MAIN_H_4ZGSCG6X -#define SRC_MAIN_H_4ZGSCG6X +#pragma once #include @@ -46,5 +45,6 @@ typedef struct { unsigned int playerErrors; } BarApp_t; -#endif /* SRC_MAIN_H_4ZGSCG6X */ +#include +extern sig_atomic_t *interrupted; diff --git a/src/player.c b/src/player.c index 6eaf382..cd666fc 100644 --- a/src/player.c +++ b/src/player.c @@ -46,9 +46,6 @@ THE SOFTWARE. #include #include #include -#ifndef HAVE_AV_TIMEOUT -#include -#endif #include "player.h" #include "ui.h" @@ -117,24 +114,20 @@ void BarPlayerSetVolume (player_t * const player) { printError (player->settings, msg, ret); \ return false; -#ifndef HAVE_AV_TIMEOUT -/* interrupt callback for libav, which lacks a timeout option - * - * obviously calling ping() a lot of times and then calling av_gettime here - * again is rather inefficient. +/* interrupt callback for blocking setup functions from openStream */ static int intCb (void * const data) { player_t * const player = data; assert (player != NULL); - /* 10 seconds timeout (usec) */ - return (av_gettime () - player->ping) > 10*1000000; + if (player->interrupted != 0) { + /* the request is retried with the same player context */ + player->interrupted = 0; + return 1; + } else { + return 0; + } } -#define ping() player->ping = av_gettime () -#else -#define ping() -#endif - static bool openStream (player_t * const player) { assert (player != NULL); /* no leak? */ @@ -143,27 +136,15 @@ static bool openStream (player_t * const player) { int ret; /* stream setup */ - AVDictionary *options = NULL; -#ifdef HAVE_AV_TIMEOUT - /* 10 seconds timeout on TCP r/w */ - av_dict_set (&options, "timeout", "10000000", 0); -#else - /* libav does not support the timeout option above. the workaround stores - * the current time with ping() now and then, registers an interrupt - * callback (below) and compares saved/current time in this callback. it’s - * not bullet-proof, but seems to work fine for av_read_frame. */ player->fctx = avformat_alloc_context (); player->fctx->interrupt_callback.callback = intCb; player->fctx->interrupt_callback.opaque = player; -#endif assert (player->url != NULL); - ping (); - if ((ret = avformat_open_input (&player->fctx, player->url, NULL, &options)) < 0) { + if ((ret = avformat_open_input (&player->fctx, player->url, NULL, NULL)) < 0) { softfail ("Unable to open audio file"); } - ping (); if ((ret = avformat_find_stream_info (player->fctx, NULL)) < 0) { softfail ("find_stream_info"); } @@ -173,7 +154,6 @@ static bool openStream (player_t * const player) { player->fctx->streams[i]->discard = AVDISCARD_ALL; } - ping (); player->streamIdx = av_find_best_stream (player->fctx, AVMEDIA_TYPE_AUDIO, -1, -1, NULL, 0); if (player->streamIdx < 0) { @@ -195,7 +175,6 @@ static bool openStream (player_t * const player) { } if (player->lastTimestamp > 0) { - ping (); av_seek_frame (player->fctx, player->streamIdx, player->lastTimestamp, 0); } @@ -315,7 +294,6 @@ static int play (player_t * const player) { assert (filteredFrame != NULL); while (!player->doQuit) { - ping (); int ret = av_read_frame (player->fctx, &pkt); if (ret < 0) { av_free_packet (&pkt); diff --git a/src/player.h b/src/player.h index e4d9f5b..4f0c963 100644 --- a/src/player.h +++ b/src/player.h @@ -30,6 +30,7 @@ THE SOFTWARE. #include #include #include +#include #include #include @@ -65,9 +66,7 @@ typedef struct { AVFilterContext *fbufsink, *fabuf; int streamIdx; int64_t lastTimestamp; -#ifndef HAVE_AV_TIMEOUT - int64_t ping; -#endif + sig_atomic_t interrupted; ao_device *aoDev; diff --git a/src/ui.c b/src/ui.c index 779dbf8..92a91c2 100644 --- a/src/ui.c +++ b/src/ui.c @@ -158,6 +158,18 @@ static size_t httpFetchCb (char *ptr, size_t size, size_t nmemb, return recvSize; } +/* libcurl progress callback. aborts the current request if user pressed ^C + */ +int progressCb (void * const data, double dltotal, double dlnow, + double ultotal, double ulnow) { + const sig_atomic_t lint = *((sig_atomic_t *) data); + if (lint) { + return 1; + } else { + return 0; + } +} + #define setAndCheck(k,v) \ httpret = curl_easy_setopt (http, k, v); \ assert (httpret == CURLE_OK); @@ -165,6 +177,7 @@ static size_t httpFetchCb (char *ptr, size_t size, size_t nmemb, static CURLcode BarPianoHttpRequest (CURL * const http, const BarSettings_t * const settings, PianoRequest_t * const req) { buffer buffer = {NULL, 0}; + sig_atomic_t lint = 0, *prevint; char url[2048]; int ret = snprintf (url, sizeof (url), "%s://%s:%s%s", req->secure ? "https" : "http", @@ -173,6 +186,10 @@ static CURLcode BarPianoHttpRequest (CURL * const http, req->urlPath); assert (ret >= 0 && ret <= (int) sizeof (url)); + /* save the previous interrupt destination */ + prevint = interrupted; + interrupted = &lint; + curl_easy_reset (http); CURLcode httpret; setAndCheck (CURLOPT_URL, url); @@ -180,8 +197,10 @@ static CURLcode BarPianoHttpRequest (CURL * const http, setAndCheck (CURLOPT_POSTFIELDS, req->postData); setAndCheck (CURLOPT_WRITEFUNCTION, httpFetchCb); setAndCheck (CURLOPT_WRITEDATA, &buffer); + setAndCheck (CURLOPT_PROGRESSFUNCTION, progressCb); + setAndCheck (CURLOPT_PROGRESSDATA, &lint); + setAndCheck (CURLOPT_NOPROGRESS, 0); setAndCheck (CURLOPT_POST, 1); - setAndCheck (CURLOPT_TIMEOUT, 30); if (settings->caBundle != NULL) { setAndCheck (CURLOPT_CAINFO, settings->caBundle); } @@ -215,90 +234,82 @@ static CURLcode BarPianoHttpRequest (CURL * const http, req->responseData = buffer.data; + interrupted = prevint; + return httpret; } /* piano wrapper: prepare/execute http request and pass result back to - * libpiano (updates data structures) - * @return 1 on success, 0 otherwise + * libpiano */ -int BarUiPianoCall (BarApp_t * const app, PianoRequestType_t type, - void *data, PianoReturn_t *pRet, CURLcode *wRet) { - PianoRequest_t req; - - memset (&req, 0, sizeof (req)); +bool BarUiPianoCall (BarApp_t * const app, const PianoRequestType_t type, + void * const data, PianoReturn_t * const pRet, CURLcode * const wRet) { + PianoReturn_t pRetLocal = PIANO_RET_OK; + CURLcode wRetLocal = CURLE_OK; + bool ret = false; /* repeat as long as there are http requests to do */ do { - req.data = data; + PianoRequest_t req = { .data = data, .responseData = NULL }; - *pRet = PianoRequest (&app->ph, &req, type); - if (*pRet != PIANO_RET_OK) { - BarUiMsg (&app->settings, MSG_NONE, "Error: %s\n", PianoErrorToStr (*pRet)); - PianoDestroyRequest (&req); - return 0; + pRetLocal = PianoRequest (&app->ph, &req, type); + if (pRetLocal != PIANO_RET_OK) { + BarUiMsg (&app->settings, MSG_NONE, "Error: %s\n", + PianoErrorToStr (pRetLocal)); + goto cleanup; } - *wRet = BarPianoHttpRequest (app->http, &app->settings, &req); - if (*wRet != CURLE_OK) { + wRetLocal = BarPianoHttpRequest (app->http, &app->settings, &req); + if (wRetLocal == CURLE_ABORTED_BY_CALLBACK) { + BarUiMsg (&app->settings, MSG_NONE, "Interrupted.\n"); + goto cleanup; + } else if (wRetLocal != CURLE_OK) { BarUiMsg (&app->settings, MSG_NONE, "Network error: %s\n", - curl_easy_strerror (*wRet)); - if (req.responseData != NULL) { - free (req.responseData); - } - PianoDestroyRequest (&req); - return 0; + curl_easy_strerror (wRetLocal)); + goto cleanup; } - *pRet = PianoResponse (&app->ph, &req); - if (*pRet != PIANO_RET_CONTINUE_REQUEST) { + pRetLocal = PianoResponse (&app->ph, &req); + if (pRetLocal != PIANO_RET_CONTINUE_REQUEST) { /* checking for request type avoids infinite loops */ - if (*pRet == PIANO_RET_P_INVALID_AUTH_TOKEN && + if (pRetLocal == PIANO_RET_P_INVALID_AUTH_TOKEN && type != PIANO_REQUEST_LOGIN) { /* reauthenticate */ - PianoReturn_t authpRet; - CURLcode authwRet; PianoRequestDataLogin_t reqData; reqData.user = app->settings.username; reqData.password = app->settings.password; reqData.step = 0; - BarUiMsg (&app->settings, MSG_NONE, "Reauthentication required... "); - if (!BarUiPianoCall (app, PIANO_REQUEST_LOGIN, &reqData, &authpRet, - &authwRet)) { - *pRet = authpRet; - *wRet = authwRet; - if (req.responseData != NULL) { - free (req.responseData); - } - PianoDestroyRequest (&req); - return 0; + BarUiMsg (&app->settings, MSG_NONE, + "Reauthentication required... "); + if (!BarUiPianoCall (app, PIANO_REQUEST_LOGIN, &reqData, + &pRetLocal, &wRetLocal)) { + goto cleanup; } else { /* try again */ - *pRet = PIANO_RET_CONTINUE_REQUEST; + pRetLocal = PIANO_RET_CONTINUE_REQUEST; BarUiMsg (&app->settings, MSG_INFO, "Trying again... "); } - } else if (*pRet != PIANO_RET_OK) { - BarUiMsg (&app->settings, MSG_NONE, "Error: %s\n", PianoErrorToStr (*pRet)); - if (req.responseData != NULL) { - free (req.responseData); - } - PianoDestroyRequest (&req); - return 0; + } else if (pRetLocal != PIANO_RET_OK) { + BarUiMsg (&app->settings, MSG_NONE, "Error: %s\n", + PianoErrorToStr (pRetLocal)); + goto cleanup; } else { BarUiMsg (&app->settings, MSG_NONE, "Ok.\n"); + ret = true; } } - /* we can destroy the request at this point, even when this call needs - * more than one http request. persistent data (step counter, e.g.) is - * stored in req.data */ - if (req.responseData != NULL) { - free (req.responseData); - } + +cleanup: + /* persistent data is stored in req.data */ + free (req.responseData); PianoDestroyRequest (&req); - } while (*pRet == PIANO_RET_CONTINUE_REQUEST); + } while (pRetLocal == PIANO_RET_CONTINUE_REQUEST); + + *pRet = pRetLocal; + *wRet = wRetLocal; - return 1; + return ret; } /* Station sorting functions */ diff --git a/src/ui.h b/src/ui.h index f49ec62..d0fb13a 100644 --- a/src/ui.h +++ b/src/ui.h @@ -50,7 +50,7 @@ size_t BarUiListSongs (const BarSettings_t *, const PianoSong_t *, const char *) void BarUiStartEventCmd (const BarSettings_t *, const char *, const PianoStation_t *, const PianoSong_t *, const player_t *, PianoStation_t *, PianoReturn_t, CURLcode); -int BarUiPianoCall (BarApp_t * const, PianoRequestType_t, +bool BarUiPianoCall (BarApp_t * const, const PianoRequestType_t, void *, PianoReturn_t *, CURLcode *); void BarUiHistoryPrepend (BarApp_t *app, PianoSong_t *song); diff --git a/src/ui_readline.c b/src/ui_readline.c index eeb5c12..f47485f 100644 --- a/src/ui_readline.c +++ b/src/ui_readline.c @@ -28,6 +28,7 @@ THE SOFTWARE. #include #include "ui_readline.h" +#include "main.h" /* return size of previous UTF-8 character */ @@ -57,16 +58,24 @@ size_t BarReadline (char *buf, const size_t bufSize, const char *mask, unsigned char escapeState = 0; fd_set set; const bool echo = !(flags & BAR_RL_NOECHO); + bool done = false; assert (buf != NULL); assert (bufSize > 0); assert (input != NULL); + /* not actually used here. just stops the player from receiving the + * signal */ + sig_atomic_t *prevInt = interrupted, localInt = 0; + if (!(flags & BAR_RL_NOINT)) { + interrupted = &localInt; + } + memset (buf, 0, bufSize); /* if fd is a fifo fgetc will always return EOF if nobody writes to * it, stdin will block */ - while (1) { + while (!done) { int curFd = -1; unsigned char chr; struct timeval timeoutstruct; @@ -78,7 +87,8 @@ size_t BarReadline (char *buf, const size_t bufSize, const char *mask, if (select (input->maxfd, &set, NULL, NULL, (timeout == -1) ? NULL : &timeoutstruct) <= 0) { - /* fail or timeout */ + /* timeout or interrupted */ + bufLen = 0; break; } @@ -103,11 +113,7 @@ size_t BarReadline (char *buf, const size_t bufSize, const char *mask, case 4: /* return */ case 10: - if (echo) { - fputs ("\n", stdout); - } - buf[bufLen] = '\0'; - return bufLen; + done = true; break; /* clear line */ @@ -180,18 +186,21 @@ size_t BarReadline (char *buf, const size_t bufSize, const char *mask, } /* buffer full => return if requested */ if (bufLen >= bufSize-1 && (flags & BAR_RL_FULLRETURN)) { - if (echo) { - fputs ("\n", stdout); - } - buf[bufLen] = '\0'; - return bufLen; + done = true; } } break; } /* end switch */ } /* end while */ - buf[0] = '\0'; - return 0; + + if (echo) { + fputs ("\n", stdout); + } + + interrupted = prevInt; + + buf[bufLen] = '\0'; + return bufLen; } /* Read string from stdin diff --git a/src/ui_readline.h b/src/ui_readline.h index cf8ed52..f8221bc 100644 --- a/src/ui_readline.h +++ b/src/ui_readline.h @@ -27,10 +27,12 @@ THE SOFTWARE. #include #include +/* bitfield */ typedef enum { BAR_RL_DEFAULT = 0, BAR_RL_FULLRETURN = 1, /* return if buffer is full */ BAR_RL_NOECHO = 2, /* don't echo to stdout */ + BAR_RL_NOINT = 4, /* don’t change interrupted variable */ } BarReadlineFlags_t; typedef struct { -- cgit v1.2.3