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().

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;