summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwm4 <wm4@nowhere>2018-03-23 20:54:04 +0100
committerKevin Mitchell <kevmitch@gmail.com>2018-03-26 23:06:50 -0700
commitcdbd20581e7da8657c4b17784546f2af6c579d36 (patch)
tree8b4b7153f89efa23329cbc319d28b98d6ac89c89
parentf60826c3a14ba3b49077f17e5364b7347f9b468a (diff)
downloadmpv-cdbd20581e7da8657c4b17784546f2af6c579d36.tar.bz2
mpv-cdbd20581e7da8657c4b17784546f2af6c579d36.tar.xz
player: fix hook processing consistency and code duplication issues
There was a "generic" function to run a hook and to wait for its completion, yet there were two duplicated functions doing the same anyway. Replace them with a single function. They differed in how stop_play was handled, but it was broken anyway. stop_play is set when playback is stopped due to quitting or changing the playlist entry - but we still can't stop hook processing, because that would mean asynchronously doing something else while the user hook code is still busy and might still have the expectation that running the hook stops everything else. So not waiting until the hook ends properly is against the whole hook idea. That this was done inconsistently is even worse. (Though it could be argued that when quitting the player, everything should just be stopped violently. But I still think that's up to the hook handler.) process_hooks() does not return anything, since hook processing doesn't really have a result (it's all about blocking and letting some other code synchronously do something). Just let the caller check whether loading was aborted in the meantime. Also change the potentially misleading name of mp_hook_run().
-rw-r--r--player/command.c4
-rw-r--r--player/command.h2
-rw-r--r--player/loadfile.c59
3 files changed, 20 insertions, 45 deletions
diff --git a/player/command.c b/player/command.c
index da5a7e39f5..70a0f824a8 100644
--- a/player/command.c
+++ b/player/command.c
@@ -219,7 +219,9 @@ static int run_next_hook_handler(struct MPContext *mpctx, char *type, int index)
return 0;
}
-void mp_hook_run(struct MPContext *mpctx, char *type)
+// Start processing script/client API hooks. This is asynchronous, and the
+// caller needs to use mp_hook_test_completion() to check whether they're done.
+void mp_hook_start(struct MPContext *mpctx, char *type)
{
while (run_next_hook_handler(mpctx, type, 0) < 0) {
// We can repeat this until all broken clients have been removed, and
diff --git a/player/command.h b/player/command.h
index 2a102ddfbe..a8638d6666 100644
--- a/player/command.h
+++ b/player/command.h
@@ -61,7 +61,7 @@ enum {
};
bool mp_hook_test_completion(struct MPContext *mpctx, char *type);
-void mp_hook_run(struct MPContext *mpctx, char *type);
+void mp_hook_start(struct MPContext *mpctx, char *type);
int mp_hook_continue(struct MPContext *mpctx, char *client, uint64_t id);
void mp_hook_add(struct MPContext *mpctx, const char *client, const char *name,
uint64_t user_id, int pri, bool legacy);
diff --git a/player/loadfile.c b/player/loadfile.c
index 32f41213fd..673ede1a71 100644
--- a/player/loadfile.c
+++ b/player/loadfile.c
@@ -750,42 +750,11 @@ static void transfer_playlist(struct MPContext *mpctx, struct playlist *pl)
}
}
-static int process_open_hooks(struct MPContext *mpctx, char *name)
+static void process_hooks(struct MPContext *mpctx, char *name)
{
+ mp_hook_start(mpctx, name);
- mp_hook_run(mpctx, name);
-
- while (!mp_hook_test_completion(mpctx, name)) {
- mp_idle(mpctx);
- if (mpctx->stop_play) {
- // Can't exit immediately, the script would interfere with the
- // next file being loaded.
- if (mpctx->stop_play == PT_QUIT)
- return -1;
- }
- }
-
- return 0;
-}
-
-static int process_preloaded_hooks(struct MPContext *mpctx)
-{
- mp_hook_run(mpctx, "on_preloaded");
-
- while (!mp_hook_test_completion(mpctx, "on_preloaded")) {
- mp_idle(mpctx);
- if (mpctx->stop_play)
- return -1;
- }
-
- return 0;
-}
-
-static void process_unload_hooks(struct MPContext *mpctx)
-{
- mp_hook_run(mpctx, "on_unload");
-
- while (!mp_hook_test_completion(mpctx, "on_unload"))
+ while (!mp_hook_test_completion(mpctx, name))
mp_idle(mpctx);
}
@@ -1239,7 +1208,8 @@ reopen_file:
assert(mpctx->demuxer == NULL);
- if (process_open_hooks(mpctx, "on_load") < 0)
+ process_hooks(mpctx, "on_load");
+ if (mpctx->stop_play)
goto terminate_playback;
if (opts->stream_dump && opts->stream_dump[0]) {
@@ -1249,12 +1219,14 @@ reopen_file:
}
open_demux_reentrant(mpctx);
- if (!mpctx->stop_play && !mpctx->demuxer &&
- process_open_hooks(mpctx, "on_load_fail") >= 0 &&
- strcmp(mpctx->stream_open_filename, mpctx->filename) != 0)
- {
- mpctx->error_playing = MPV_ERROR_LOADING_FAILED;
- open_demux_reentrant(mpctx);
+ if (!mpctx->stop_play && !mpctx->demuxer) {
+ process_hooks(mpctx, "on_load_fail");
+ if (strcmp(mpctx->stream_open_filename, mpctx->filename) != 0 &&
+ !mpctx->stop_play)
+ {
+ mpctx->error_playing = MPV_ERROR_LOADING_FAILED;
+ open_demux_reentrant(mpctx);
+ }
}
if (!mpctx->demuxer || mpctx->stop_play)
goto terminate_playback;
@@ -1289,7 +1261,8 @@ reopen_file:
check_previous_track_selection(mpctx);
- if (process_preloaded_hooks(mpctx))
+ process_hooks(mpctx, "on_preloaded");
+ if (mpctx->stop_play)
goto terminate_playback;
if (reinit_complex_filters(mpctx, false) < 0)
@@ -1400,7 +1373,7 @@ terminate_playback:
update_core_idle_state(mpctx);
- process_unload_hooks(mpctx);
+ process_hooks(mpctx, "on_unload");
if (mpctx->stop_play == KEEP_PLAYING)
mpctx->stop_play = AT_END_OF_FILE;