player: make various commands for managing external tracks abortable

Until now, they could be aborted only by ending playback, and calling
mpv_abort_async_command didn't do anything.

This requires furthering the mess how playback abort is done. The main
reason why mp_cancel exists at all is to avoid that a "frozen" demuxer
(blocked on network I/O or whatever) cannot freeze the core. The core
should always get its way. Previously, there was a single mp_cancel
handle, that could be signaled, and all demuxers would unfreeze. With
external files, we might want to abort loading of a certain external
file, which automatically means they need a separate mp_cancel. So give
every demuxer its own mp_cancel, and "slave" it to whatever parent
mp_cancel handles aborting.

Since the mpv demuxer API conflates creating the demuxer and reading the
file headers, mp_cancel strictly need to be created before the demuxer
is created (or we couldn't abort loading). Although we give every
demuxer its own mp_cancel (as "enforced" by cancel_and_free_demuxer),
it's still rather messy to create/destroy it along with the demuxer.

5 files changed, 96 insertions, 54 deletions

diff --git a/TOOLS/lua/command-test.lua b/TOOLS/lua/command-test.lua
index 79c1127cbf..dd384818cc 100644
--- a/TOOLS/lua/command-test.lua
+++ b/TOOLS/lua/command-test.lua
@@ -65,6 +65,18 @@ mp.observe_property("vo-configured", "bool", function(_, v)
         mp.abort_async_command(x)
     end)
 
+    -- (assuming this "freezes")
+    local y = mp.command_native_async({name = "sub-add", url = "-"},
+        function(res, val, err)
+            print("done sub-add stdin: " .. join(" ", {res, val, err}))
+        end)
+    mp.add_timeout(20, function()
+        print("aborting sub-add stdin after timeout")
+        mp.abort_async_command(y)
+    end)
+
+
+
     -- This should get killed on script exit.
     mp.command_native_async({name = "subprocess", playback_only = false,
                              args = {"sleep", "inf"}}, function()end)
diff --git a/input/cmd.h b/input/cmd.h
index 0f74424f87..a2cbaae510 100644
--- a/input/cmd.h
+++ b/input/cmd.h
@@ -58,6 +58,9 @@ struct mp_cmd_def {
     // asynchronous abort of the command, and explicitly uses mp_cmd_ctx.abort.
     // (Not setting it when it's not needed can save resources.)
     bool can_abort;
+    // If playback ends, and the command is still running, an abort is
+    // automatically triggered.
+    bool abort_on_playback_end;
 };
 
 enum mp_cmd_flags {
diff --git a/player/command.c b/player/command.c
index 9c6bce6aa1..fb8f6f14c5 100644
--- a/player/command.c
+++ b/player/command.c
@@ -4981,8 +4981,10 @@ void run_command(struct MPContext *mpctx, struct mp_cmd *cmd,
 
     assert(cmd->def->can_abort == !!ctx->abort);
 
-    if (ctx->abort)
+    if (ctx->abort) {
+        ctx->abort->coupled_to_playback |= cmd->def->abort_on_playback_end;
         mp_abort_add(mpctx, ctx->abort);
+    }
 
     struct MPOpts *opts = mpctx->opts;
     ctx->on_osd = cmd->flags & MP_ON_OSD_FLAGS;
@@ -5534,7 +5536,8 @@ static void cmd_track_add(void *p)
             return;
         }
     }
-    int first = mp_add_external_file(mpctx, cmd->args[0].v.s, type);
+    int first = mp_add_external_file(mpctx, cmd->args[0].v.s, type,
+                                     cmd->abort->cancel);
     if (first < 0) {
         cmd->success = false;
         return;
@@ -5598,7 +5601,7 @@ static void cmd_track_reload(void *p)
     if (t && t->is_external && t->external_filename) {
         char *filename = talloc_strdup(NULL, t->external_filename);
         mp_remove_track(mpctx, t);
-        nt_num = mp_add_external_file(mpctx, filename, type);
+        nt_num = mp_add_external_file(mpctx, filename, type, cmd->abort->cancel);
         talloc_free(filename);
     }
 
@@ -5622,7 +5625,7 @@ static void cmd_rescan_external_files(void *p)
         return;
     }
 
-    autoload_external_files(mpctx);
+    autoload_external_files(mpctx, cmd->abort->cancel);
     if (!cmd->args[0].v.i && mpctx->playback_initialized) {
         // somewhat fuzzy and not ideal
         struct track *a = select_default_track(mpctx, 0, STREAM_AUDIO);
@@ -6098,6 +6101,8 @@ const struct mp_cmd_def mp_cmds[] = {
         },
         .priv = &(const int){STREAM_SUB},
         .spawn_thread = true,
+        .can_abort = true,
+        .abort_on_playback_end = true,
     },
     { "audio-add", cmd_track_add,
         {
@@ -6109,6 +6114,8 @@ const struct mp_cmd_def mp_cmds[] = {
         },
         .priv = &(const int){STREAM_AUDIO},
         .spawn_thread = true,
+        .can_abort = true,
+        .abort_on_playback_end = true,
     },
 
     { "sub-remove", cmd_track_remove, { OPT_INT("id", v.i, 0, OPTDEF_INT(-1)) },
@@ -6119,10 +6126,14 @@ const struct mp_cmd_def mp_cmds[] = {
     { "sub-reload", cmd_track_reload, { OPT_INT("id", v.i, 0, OPTDEF_INT(-1)) },
         .priv = &(const int){STREAM_SUB},
         .spawn_thread = true,
+        .can_abort = true,
+        .abort_on_playback_end = true,
     },
     { "audio-reload", cmd_track_reload, { OPT_INT("id", v.i, 0, OPTDEF_INT(-1)) },
         .priv = &(const int){STREAM_AUDIO},
         .spawn_thread = true,
+        .can_abort = true,
+        .abort_on_playback_end = true,
     },
 
     { "rescan-external-files", cmd_rescan_external_files,
@@ -6132,6 +6143,8 @@ const struct mp_cmd_def mp_cmds[] = {
                         {"reselect", 0})),
         },
         .spawn_thread = true,
+        .can_abort = true,
+        .abort_on_playback_end = true,
     },
 
     { "tv-last-channel", cmd_tv_last_channel, },
diff --git a/player/core.h b/player/core.h
index b0d9b2a5ea..43fc824e4f 100644
--- a/player/core.h
+++ b/player/core.h
@@ -441,7 +441,6 @@ typedef struct MPContext {
     pthread_mutex_t abort_lock;
 
     // --- The following fields are protected by abort_lock
-    struct mp_cancel *demuxer_cancel; // cancel handle for MPContext.demuxer
     struct mp_abort_entry **abort_list;
     int num_abort_list;
     bool abort_all; // during final termination
@@ -513,7 +512,7 @@ void mp_abort_trigger_locked(struct MPContext *mpctx,
                              struct mp_abort_entry *abort);
 void uninit_player(struct MPContext *mpctx, unsigned int mask);
 int mp_add_external_file(struct MPContext *mpctx, char *filename,
-                         enum stream_type filter);
+                         enum stream_type filter, struct mp_cancel *cancel);
 #define FLAG_MARK_SELECTION 1
 void mp_switch_track(struct MPContext *mpctx, enum stream_type type,
                      struct track *track, int flags);
@@ -532,7 +531,7 @@ void update_demuxer_properties(struct MPContext *mpctx);
 void print_track_list(struct MPContext *mpctx, const char *msg);
 void reselect_demux_stream(struct MPContext *mpctx, struct track *track);
 void prepare_playlist(struct MPContext *mpctx, struct playlist *pl);
-void autoload_external_files(struct MPContext *mpctx);
+void autoload_external_files(struct MPContext *mpctx, struct mp_cancel *cancel);
 struct track *select_default_track(struct MPContext *mpctx, int order,
                                    enum stream_type type);
 void prefetch_next(struct MPContext *mpctx);
diff --git a/player/loadfile.c b/player/loadfile.c
index a16df30eed..dbee6ecd69 100644
--- a/player/loadfile.c
+++ b/player/loadfile.c
@@ -68,9 +68,6 @@ void mp_abort_playback_async(struct MPContext *mpctx)
 
     pthread_mutex_lock(&mpctx->abort_lock);
 
-    if (mpctx->demuxer_cancel)
-        mp_cancel_trigger(mpctx->demuxer_cancel);
-
     for (int n = 0; n < mpctx->num_abort_list; n++) {
         struct mp_abort_entry *abort = mpctx->abort_list[n];
         if (abort->coupled_to_playback)
@@ -126,6 +123,25 @@ void mp_abort_trigger_locked(struct MPContext *mpctx,
     mp_cancel_trigger(abort->cancel);
 }
 
+static void cancel_and_free_demuxer(struct MPContext *mpctx,
+                                    struct demuxer **demuxer)
+{
+    if (!*demuxer)
+        return;
+
+    struct mp_cancel *cancel = (*demuxer)->cancel;
+    assert(cancel != mpctx->playback_abort);
+
+    // Explicitly trigger it so freeing the demuxer can't block on I/O.
+    if (cancel)
+        mp_cancel_trigger(cancel);
+
+    free_demuxer_and_stream(*demuxer);
+    *demuxer = NULL;
+
+    talloc_free(cancel);
+}
+
 static void uninit_demuxer(struct MPContext *mpctx)
 {
     for (int r = 0; r < NUM_PTRACKS; r++) {
@@ -147,13 +163,7 @@ static void uninit_demuxer(struct MPContext *mpctx)
     }
     mpctx->num_tracks = 0;
 
-    free_demuxer_and_stream(mpctx->demuxer);
-    mpctx->demuxer = NULL;
-
-    pthread_mutex_lock(&mpctx->abort_lock);
-    talloc_free(mpctx->demuxer_cancel);
-    mpctx->demuxer_cancel = NULL;
-    pthread_mutex_unlock(&mpctx->abort_lock);
+    cancel_and_free_demuxer(mpctx, &mpctx->demuxer);
 }
 
 #define APPEND(s, ...) mp_snprintf_cat(s, sizeof(s), __VA_ARGS__)
@@ -625,7 +635,7 @@ bool mp_remove_track(struct MPContext *mpctx, struct track *track)
         in_use |= mpctx->tracks[n]->demuxer == d;
 
     if (!in_use)
-        free_demuxer_and_stream(d);
+        cancel_and_free_demuxer(mpctx, &d);
 
     mp_notify(mpctx, MPV_EVENT_TRACKS_CHANGED, NULL);
 
@@ -635,11 +645,13 @@ bool mp_remove_track(struct MPContext *mpctx, struct track *track)
 // Add the given file as additional track. The filter argument controls how or
 // if tracks are auto-selected at any point.
 // To be run on a worker thread, locked (temporarily unlocks core).
+// cancel will generally be used to abort the loading process, but on success
+// the demuxer is changed to be slaved to mpctx->playback_abort instead.
 int mp_add_external_file(struct MPContext *mpctx, char *filename,
-                         enum stream_type filter)
+                         enum stream_type filter, struct mp_cancel *cancel)
 {
     struct MPOpts *opts = mpctx->opts;
-    if (!filename || mp_cancel_test(mpctx->playback_abort))
+    if (!filename || mp_cancel_test(cancel))
         return -1;
 
     char *disp_filename = filename;
@@ -657,10 +669,13 @@ int mp_add_external_file(struct MPContext *mpctx, char *filename,
         break;
     }
 
+    struct mp_cancel *demux_cancel = mp_cancel_new(NULL);
+    mp_cancel_add_slave(cancel, demux_cancel);
+
     mp_core_unlock(mpctx);
 
     struct demuxer *demuxer =
-        demux_open_url(filename, &params, mpctx->playback_abort, mpctx->global);
+        demux_open_url(filename, &params, demux_cancel, mpctx->global);
     if (demuxer)
         enable_demux_thread(mpctx, demuxer);
 
@@ -668,12 +683,8 @@ int mp_add_external_file(struct MPContext *mpctx, char *filename,
 
     // The command could have overlapped with playback exiting. (We don't care
     // if playback has started again meanwhile - weird, but not a problem.)
-    if (!mpctx->playing) {
-        mp_core_unlock(mpctx);
-        free_demuxer_and_stream(demuxer);
-        mp_core_lock(mpctx);
-        return -1;
-    }
+    if (!mpctx->playing)
+        goto err_out;
 
     if (!demuxer)
         goto err_out;
@@ -691,14 +702,11 @@ int mp_add_external_file(struct MPContext *mpctx, char *filename,
     }
 
     if (!has_any) {
-        mp_core_unlock(mpctx);
-        free_demuxer_and_stream(demuxer);
-        mp_core_lock(mpctx);
         char *tname = mp_tprintf(20, "%s ", stream_type_name(filter));
         if (filter == STREAM_TYPE_COUNT)
             tname = "";
         MP_ERR(mpctx, "No %sstreams in file %s.\n", tname, disp_filename);
-        return -1;
+        goto err_out;
     }
 
     int first_num = -1;
@@ -714,10 +722,18 @@ int mp_add_external_file(struct MPContext *mpctx, char *filename,
             first_num = mpctx->num_tracks - 1;
     }
 
+    mp_cancel_remove_slave(cancel, demux_cancel);
+    mp_cancel_add_slave(mpctx->playback_abort, demux_cancel);
+
     return first_num;
 
 err_out:
-    if (!mp_cancel_test(mpctx->playback_abort))
+    if (demuxer) {
+        cancel_and_free_demuxer(mpctx, &demuxer);
+    } else {
+        talloc_free(demux_cancel);
+    }
+    if (!mp_cancel_test(cancel))
         MP_ERR(mpctx, "Can not open external file %s.\n", disp_filename);
     return -1;
 }
@@ -731,12 +747,13 @@ static void open_external_files(struct MPContext *mpctx, char **files,
     files = mp_dup_str_array(tmp, files);
 
     for (int n = 0; files && files[n]; n++)
-        mp_add_external_file(mpctx, files[n], filter);
+        mp_add_external_file(mpctx, files[n], filter, mpctx->playback_abort);
 
     talloc_free(tmp);
 }
 
-void autoload_external_files(struct MPContext *mpctx)
+// See mp_add_external_file() for meaning of cancel parameter.
+void autoload_external_files(struct MPContext *mpctx, struct mp_cancel *cancel)
 {
     if (mpctx->opts->sub_auto < 0 && mpctx->opts->audiofile_auto < 0)
         return;
@@ -772,7 +789,7 @@ void autoload_external_files(struct MPContext *mpctx)
             goto skip;
         if (list[i].type == STREAM_AUDIO && !sc[STREAM_VIDEO])
             goto skip;
-        int first = mp_add_external_file(mpctx, filename, list[i].type);
+        int first = mp_add_external_file(mpctx, filename, list[i].type, cancel);
         if (first < 0)
             goto skip;
 
@@ -848,13 +865,17 @@ static void load_chapters(struct MPContext *mpctx)
     char *chapter_file = mpctx->opts->chapter_file;
     if (chapter_file && chapter_file[0]) {
         chapter_file = talloc_strdup(NULL, chapter_file);
+        struct mp_cancel *cancel = mp_cancel_new(NULL);
+        mp_cancel_add_slave(mpctx->playback_abort, cancel);
         mp_core_unlock(mpctx);
-        struct demuxer *demux = demux_open_url(chapter_file, NULL,
-                                        mpctx->playback_abort, mpctx->global);
+        struct demuxer *demux = demux_open_url(chapter_file, NULL, cancel,
+                                               mpctx->global);
         mp_core_lock(mpctx);
         if (demux) {
             src = demux;
             free_src = true;
+        } else {
+            talloc_free(cancel);
         }
         talloc_free(mpctx->chapters);
         mpctx->chapters = NULL;
@@ -869,11 +890,8 @@ static void load_chapters(struct MPContext *mpctx)
                 mpctx->chapters[n].pts -= src->start_time;
         }
     }
-    if (free_src) {
-        mp_core_unlock(mpctx);
-        free_demuxer_and_stream(src);
-        mp_core_lock(mpctx);
-    }
+    if (free_src)
+        cancel_and_free_demuxer(mpctx, &src);
 }
 
 static void load_per_file_options(m_config_t *conf,
@@ -926,14 +944,16 @@ static void cancel_open(struct MPContext *mpctx)
         pthread_join(mpctx->open_thread, NULL);
     mpctx->open_active = false;
 
+    if (mpctx->open_res_demuxer) {
+        assert(mpctx->open_res_demuxer->cancel == mpctx->open_cancel);
+        mpctx->open_cancel = NULL;
+        cancel_and_free_demuxer(mpctx, &mpctx->open_res_demuxer);
+    }
+
     TA_FREEP(&mpctx->open_cancel);
     TA_FREEP(&mpctx->open_url);
     TA_FREEP(&mpctx->open_format);
 
-    if (mpctx->open_res_demuxer)
-        free_demuxer_and_stream(mpctx->open_res_demuxer);
-    mpctx->open_res_demuxer = NULL;
-
     atomic_store(&mpctx->open_done, false);
 }
 
@@ -990,9 +1010,7 @@ static void open_demux_reentrant(struct MPContext *mpctx)
         start_open(mpctx, url, mpctx->playing->stream_flags);
 
     // User abort should cancel the opener now.
-    pthread_mutex_lock(&mpctx->abort_lock);
-    mpctx->demuxer_cancel = mpctx->open_cancel;
-    pthread_mutex_unlock(&mpctx->abort_lock);
+    mp_cancel_add_slave(mpctx->playback_abort, mpctx->open_cancel);
 
     while (!atomic_load(&mpctx->open_done)) {
         mp_idle(mpctx);
@@ -1002,15 +1020,12 @@ static void open_demux_reentrant(struct MPContext *mpctx)
     }
 
     if (mpctx->open_res_demuxer) {
-        assert(mpctx->demuxer_cancel == mpctx->open_cancel);
+        assert(mpctx->open_res_demuxer->cancel == mpctx->open_cancel);
         mpctx->demuxer = mpctx->open_res_demuxer;
         mpctx->open_res_demuxer = NULL;
         mpctx->open_cancel = NULL;
     } else {
         mpctx->error_playing = mpctx->open_res_error;
-        pthread_mutex_lock(&mpctx->abort_lock);
-        mpctx->demuxer_cancel = NULL;
-        pthread_mutex_unlock(&mpctx->abort_lock);
     }
 
     cancel_open(mpctx); // cleanup
@@ -1235,7 +1250,7 @@ static void load_external_opts_thread(void *p)
     open_external_files(mpctx, mpctx->opts->audio_files, STREAM_AUDIO);
     open_external_files(mpctx, mpctx->opts->sub_name, STREAM_SUB);
     open_external_files(mpctx, mpctx->opts->external_files, STREAM_TYPE_COUNT);
-    autoload_external_files(mpctx);
+    autoload_external_files(mpctx, mpctx->playback_abort);
 
     mp_waiter_wakeup(waiter, 0);
     mp_wakeup_core(mpctx);