commit 15aecb89d41f59ac375c7a2a2194ab7cddcaa020 from: Omar Polo date: Tue Jul 12 22:05:35 2022 UTC monitor: fix reported event rethink a bit which event are reported and when. - drop 'toggle' event as it's not useful; now 'amused toggle' will report the play/pause event. - replace 'flush' with 'load'. there's no real difference between a flush and an empty load. (also, less event the better) - some events (next/prev/jump) are reported earlier to avoid possible issues (when messing around with the playing queue we can end up in every possible state.) - report 'seek' only after it really happened (i.e. after the player process sent the new position.) There's still a possible race here (we can receive a previous IMSG_POS and think it's the reply to the seek) but it it's hard/impossible to work around. - drop 'restart'. we have 'seek' now which is better (and actually restart is implemented on top of seek.) - add 'seek' to the default list of events. commit - 87abdf60b66234cded899c05ebd50fe261a047eb commit + 15aecb89d41f59ac375c7a2a2194ab7cddcaa020 blob - f57e6cfb71160e07a8b93e2140184f870868027e blob + 4d7957cecda7b5f2fd118d3c6a408773c513a5f4 --- amused.c +++ amused.c @@ -47,6 +47,8 @@ pid_t player_pid; struct event ev_sigint; struct event ev_sigterm; +static int notify_seek; + enum amused_process { PROC_MAIN, PROC_PLAYER, @@ -136,6 +138,10 @@ main_dispatch_player(int sig, short event, void *d) sizeof(current_position)); if (current_position < 0) current_position = -1; + if (notify_seek) { + notify_seek = 0; + control_notify(IMSG_CTL_SEEK); + } break; case IMSG_LEN: if (datalen != sizeof(current_duration)) @@ -435,6 +441,8 @@ main_playlist_jump(struct imsgev *iev, struct imsg *im return; } + control_notify(IMSG_CTL_JUMP); + main_send_player(IMSG_STOP, -1, NULL, 0); if (!main_play_song(song)) { main_senderr(iev, "can't play"); @@ -566,4 +574,22 @@ main_send_status(struct imsgev *iev) s.rp.repeat_one = repeat_one; imsg_compose_event(iev, IMSG_CTL_STATUS, 0, 0, -1, &s, sizeof(s)); +} + +void +main_seek(struct player_seek *s) +{ + switch (play_state) { + case STATE_STOPPED: + main_playlist_resume(); + break; + case STATE_PLAYING: + break; + case STATE_PAUSED: + play_state = STATE_PLAYING; + break; + } + + notify_seek = 1; + main_send_player(IMSG_CTL_SEEK, -1, s, sizeof(*s)); } blob - 1e6e863f9f9375ee460cd143634e837ef73083d4 blob + 940c6f3e44493e581b166e148052859aef01f73e --- amused.h +++ amused.h @@ -143,6 +143,7 @@ void main_senderr(struct imsgev *, const char *); void main_enqueue(int, struct playlist *, struct imsgev *, struct imsg *); void main_send_playlist(struct imsgev *); void main_send_status(struct imsgev *); +void main_seek(struct player_seek *); /* ctl.c */ __dead void usage(void); blob - 8a2c1030cdb8036e6d3a558d4a563739144ec5d1 blob + 0b9e7f0a14f2b475d0a8ee38c162acf2f3fa0ecb --- control.c +++ control.c @@ -246,6 +246,7 @@ control_dispatch_imsg(int fd, short event, void *bula) struct ctl_conn *c; struct imsg imsg; struct player_repeat rp; + struct player_seek seek; ssize_t n, off; if ((c = control_connbyfd(fd)) == NULL) { @@ -294,18 +295,20 @@ control_dispatch_imsg(int fd, short event, void *bula) case IMSG_CTL_TOGGLE_PLAY: switch (play_state) { case STATE_STOPPED: + control_notify(IMSG_CTL_PLAY); main_playlist_resume(); break; case STATE_PLAYING: + control_notify(IMSG_CTL_PAUSE); play_state = STATE_PAUSED; main_send_player(IMSG_PAUSE, -1, NULL, 0); break; case STATE_PAUSED: + control_notify(IMSG_CTL_PLAY); play_state = STATE_PLAYING; main_send_player(IMSG_RESUME, -1, NULL, 0); break; } - control_notify(imsg.hdr.type); break; case IMSG_CTL_PAUSE: if (play_state != STATE_PLAYING) @@ -323,7 +326,7 @@ control_dispatch_imsg(int fd, short event, void *bula) break; case IMSG_CTL_FLUSH: playlist_truncate(); - control_notify(imsg.hdr.type); + control_notify(IMSG_CTL_COMMIT); break; case IMSG_CTL_SHOW: main_send_playlist(&c->iev); @@ -332,18 +335,17 @@ control_dispatch_imsg(int fd, short event, void *bula) main_send_status(&c->iev); break; case IMSG_CTL_NEXT: + control_notify(imsg.hdr.type); main_send_player(IMSG_STOP, -1, NULL, 0); main_playlist_advance(); - control_notify(imsg.hdr.type); break; case IMSG_CTL_PREV: + control_notify(imsg.hdr.type); main_send_player(IMSG_STOP, -1, NULL, 0); main_playlist_previous(); - control_notify(imsg.hdr.type); break; case IMSG_CTL_JUMP: main_playlist_jump(&c->iev, &imsg); - control_notify(imsg.hdr.type); break; case IMSG_CTL_REPEAT: if (IMSG_DATA_SIZE(imsg) != sizeof(rp)) { @@ -399,24 +401,12 @@ control_dispatch_imsg(int fd, short event, void *bula) c->monitor = 1; break; case IMSG_CTL_SEEK: - if (IMSG_DATA_SIZE(imsg) != - sizeof(struct player_seek)) { + if (IMSG_DATA_SIZE(imsg) != sizeof(seek)) { main_senderr(&c->iev, "wrong size"); break; } - switch (play_state) { - case STATE_STOPPED: - main_playlist_resume(); - break; - case STATE_PLAYING: - break; - case STATE_PAUSED: - play_state = STATE_PLAYING; - break; - } - main_send_player(IMSG_CTL_SEEK, -1, imsg.data, - IMSG_DATA_SIZE(imsg)); - control_notify(imsg.hdr.type); + memcpy(&seek, imsg.data, sizeof(seek)); + main_seek(&seek); break; default: log_debug("%s: error handling imsg %d", __func__, blob - dd7b94f7ce9191b416561e685a269a2fd1c3ccad blob + 2dc2c91eec648890a1814d302a46d9f07f72e594 --- ctl.c +++ ctl.c @@ -247,26 +247,22 @@ imsg_name(int type) switch (type) { case IMSG_CTL_PLAY: return "play"; - case IMSG_CTL_TOGGLE_PLAY: - return "toggle"; case IMSG_CTL_PAUSE: return "pause"; case IMSG_CTL_STOP: return "stop"; - case IMSG_CTL_FLUSH: - return "flush"; case IMSG_CTL_NEXT: return "next"; case IMSG_CTL_PREV: return "prev"; case IMSG_CTL_JUMP: return "jump"; - case IMSG_CTL_REPEAT: - return "repeat"; case IMSG_CTL_ADD: return "add"; case IMSG_CTL_COMMIT: return "load"; + case IMSG_CTL_SEEK: + return "seek"; default: return "unknown"; } @@ -704,7 +700,7 @@ ctl_repeat(struct parse_result *res, int argc, char ** static int ctl_monitor(struct parse_result *res, int argc, char **argv) { - int ch; + int ch, n = 0; const char *events; char *dup, *tmp, *tok; @@ -716,29 +712,22 @@ ctl_monitor(struct parse_result *res, int argc, char * if (argc > 1) ctl_usage(res->ctl); + events = "play,pause,stop,next,prev,jump,repeat,add,load,seek"; if (argc == 1) events = *argv; - else - events = "play,toggle,pause,stop,restart,flush,next,prev," - "jump,repeat,add,load"; tmp = dup = xstrdup(events); while ((tok = strsep(&tmp, ",")) != NULL) { if (*tok == '\0') continue; + n++; if (!strcmp(tok, "play")) res->monitor[IMSG_CTL_PLAY] = 1; - else if (!strcmp(tok, "toggle")) - res->monitor[IMSG_CTL_TOGGLE_PLAY] = 1; else if (!strcmp(tok, "pause")) res->monitor[IMSG_CTL_PAUSE] = 1; else if (!strcmp(tok, "stop")) res->monitor[IMSG_CTL_STOP] = 1; - else if (!strcmp(tok, "restart")) /* compat */ - res->monitor[IMSG_CTL_SEEK] = 1; - else if (!strcmp(tok, "flush")) - res->monitor[IMSG_CTL_FLUSH] = 1; else if (!strcmp(tok, "next")) res->monitor[IMSG_CTL_NEXT] = 1; else if (!strcmp(tok, "prev")) @@ -753,11 +742,15 @@ ctl_monitor(struct parse_result *res, int argc, char * res->monitor[IMSG_CTL_COMMIT] = 1; else if (!strcmp(tok, "seek")) res->monitor[IMSG_CTL_SEEK] = 1; - else - fatalx("unknown event \"%s\"", tok); + else { + log_warnx("unknown event \"%s\"", tok); + n--; + } } free(dup); + if (n == 0) + ctl_usage(res->ctl); return ctlaction(res); }