player: make playback termination asynchronous

Until now, stopping playback aborted the demuxer and I/O layer violently
by signaling mp_cancel (bound to libavformat's AVIOInterruptCB
mechanism). Change it to try closing them gracefully.

The main purpose is to silence those libavformat errors that happen when
you request termination. Most of libavformat barely cares about the
termination mechanism (AVIOInterruptCB), and essentially it's like the
network connection is abruptly severed, or file I/O suddenly returns I/O
errors. There were issues with dumb TLS warnings, parsers complaining
about incomplete data, and some special protocols that require server
communication to gracefully disconnect.

We still want to abort it forcefully if it refuses to terminate on its
own, so a timeout is required. Users can set the timeout to 0, which
should give them the old behavior.

This also removes the old mechanism that treats certain commands (like
"quit") specially, and tries to terminate the demuxers even if the core
is currently frozen. This is for situations where the core synchronized
to the demuxer or stream layer while network is unresponsive. This in
turn can only happen due to the "program" or "cache-size" properties in
the current code (see one of the previous commits). Also, the old
mechanism doesn't fit particularly well with the new one. We wouldn't
want to abort playback immediately on a "quit" command - the new code is
all about giving it a chance to end it gracefully. We'd need some sort
of watchdog thread or something equally complicated to handle this. So
just remove it.

The change in osd.c is to prevent that it clears the status line while
waiting for termination. The normal status line code doesn't output
anything useful at this point, and the code path taken clears it, both
of which is an annoying behavior change, so just let it show the old
one.

DOCS/interface-changes.rst

@@ -111,6 +111,17 @@ Interface changes
       as reference, which lists the definitive names.
     - edition and disc title switching will now fully reload playback (may have
       consequences for scripts, client API, or when using file-local options)
+    - remove async playback abort hack. This breaks aborting playback in the
+      following cases, iff the current stream is a network stream that
+      completely stopped responding:
+        - setting "program" property
+        - setting "cache-size" property
+      In earlier versions of mpv, the player core froze as well in these cases,
+      but could still be aborted with the quit, stop, playlist-prev,
+      playlist-next commands. If these properties are not accessed, frozen
+      network streams should not freeze the player core (only playback in
+      uncached regions), and differing behavior should be reported as a bug.
+      If --demuxer-thread=no is used, there are no guarantees.
  --- mpv 0.28.0 ---
     - rename --hwdec=mediacodec option to mediacodec-copy, to reflect
       conventions followed by other hardware video decoding APIs

DOCS/man/options.rst

@@ -2951,6 +2951,19 @@ Demuxer
 
     Disabling this option is not recommended. Use it for debugging only.
 
+``--demuxer-termination-timeout=<seconds>``
+    Number of seconds the player should wait to shutdown the demuxer (default:
+    0.1). The player will wait up to this much time before it closes the
+    stream layer forcefully. Forceful closing usually means the network I/O is
+    given no chance to close its connections gracefully (of course the OS can
+    still close TCP connections properly), and might result in annoying messages
+    being logged, and in some cases, confused remote servers.
+
+    This timeout is usually only applied when loading has finished properly. If
+    loading is aborted by the user, or in some corner cases like removing
+    external tracks sourced from network during playback, forceful closing is
+    always used.
+
 ``--demuxer-readahead-secs=<seconds>``
     If ``--demuxer-thread`` is enabled, this controls how much the demuxer
     should buffer ahead in seconds (default: 1). As long as no packet has

options/options.c

@@ -461,6 +461,7 @@ const m_option_t mp_opts[] = {
     OPT_STRING("audio-demuxer", audio_demuxer_name, 0),
     OPT_STRING("sub-demuxer", sub_demuxer_name, 0),
     OPT_FLAG("demuxer-thread", demuxer_thread, 0),
+    OPT_DOUBLE("demuxer-termination-timeout", demux_termination_timeout, 0),
     OPT_FLAG("prefetch-playlist", prefetch_open, 0),
     OPT_FLAG("cache-pause", cache_pause, 0),
     OPT_FLAG("cache-pause-initial", cache_pause_initial, 0),
@@ -915,6 +916,7 @@ const struct MPOpts mp_default_opts = {
     .position_resume = 1,
     .autoload_files = 1,
     .demuxer_thread = 1,
+    .demux_termination_timeout = 0.1,
     .hls_bitrate = INT_MAX,
     .cache_pause = 1,
     .cache_pause_wait = 1.0,

options/options.h

@@ -261,6 +261,7 @@ typedef struct MPOpts {
     char **audio_files;
     char *demuxer_name;
     int demuxer_thread;
+    double demux_termination_timeout;
     int prefetch_open;
     char *audio_demuxer_name;
     char *sub_demuxer_name;

player/client.c

@@ -1040,9 +1040,6 @@ static int run_client_command(mpv_handle *ctx, struct mp_cmd *cmd, mpv_node *res
     if (!cmd)
         return MPV_ERROR_INVALID_PARAMETER;
 
-    if (mp_input_is_abort_cmd(cmd))
-        mp_abort_playback_async(ctx->mpctx);
-
     cmd->sender = ctx->name;
 
     struct cmd_request req = {

player/loadfile.c

@@ -130,29 +130,111 @@ void mp_abort_trigger_locked(struct MPContext *mpctx,
     mp_cancel_trigger(abort->cancel);
 }
 
+static void kill_demuxers_reentrant(struct MPContext *mpctx,
+                                    struct demuxer **demuxers, int num_demuxers)
+{
+    struct demux_free_async_state **items = NULL;
+    int num_items = 0;
+
+    for (int n = 0; n < num_demuxers; n++) {
+        struct demuxer *d = demuxers[n];
+
+        if (!demux_cancel_test(d)) {
+            // Make sure it is set if it wasn't yet.
+            demux_set_wakeup_cb(d, wakeup_demux, mpctx);
+
+            struct demux_free_async_state *item = demux_free_async(d);
+            if (item) {
+                MP_TARRAY_APPEND(NULL, items, num_items, item);
+                d = NULL;
+            }
+        }
+
+        demux_cancel_and_free(d);
+    }
+
+    if (!num_items)
+        return;
+
+    MP_DBG(mpctx, "Terminating demuxers...\n");
+
+    double end = mp_time_sec() + mpctx->opts->demux_termination_timeout;
+    bool force = false;
+    while (num_items) {
+        double wait = end - mp_time_sec();
+
+        for (int n = 0; n < num_items; n++) {
+            struct demux_free_async_state *item = items[n];
+            if (demux_free_async_finish(item)) {
+                items[n] = items[num_items - 1];
+                num_items -= 1;
+                n--;
+                goto repeat;
+            } else if (wait < 0) {
+                demux_free_async_force(item);
+                if (!force)
+                    MP_VERBOSE(mpctx, "Forcefully terminating demuxers...\n");
+                force = true;
+            }
+        }
+
+        if (wait >= 0)
+            mp_set_timeout(mpctx, wait);
+        mp_idle(mpctx);
+    repeat:;
+    }
+
+    talloc_free(items);
+
+    MP_DBG(mpctx, "Done terminating demuxers.\n");
+}
+
 static void uninit_demuxer(struct MPContext *mpctx)
 {
     for (int r = 0; r < NUM_PTRACKS; r++) {
         for (int t = 0; t < STREAM_TYPE_COUNT; t++)
             mpctx->current_track[r][t] = NULL;
     }
+    mpctx->seek_slave = NULL;
+
     talloc_free(mpctx->chapters);
     mpctx->chapters = NULL;
     mpctx->num_chapters = 0;
 
-    // close demuxers for external tracks
+    struct demuxer **demuxers = NULL;
-    for (int n = mpctx->num_tracks - 1; n >= 0; n--) {
+    int num_demuxers = 0;
-        mpctx->tracks[n]->selected = false;
+
-        mp_remove_track(mpctx, mpctx->tracks[n]);
+    if (mpctx->demuxer)
-    }
+        MP_TARRAY_APPEND(NULL, demuxers, num_demuxers, mpctx->demuxer);
+    mpctx->demuxer = NULL;
+
     for (int i = 0; i < mpctx->num_tracks; i++) {
-        sub_destroy(mpctx->tracks[i]->d_sub);
+        struct track *track = mpctx->tracks[i];
-        talloc_free(mpctx->tracks[i]);
+
+        assert(!track->dec);
+        assert(!track->vo_c && !track->ao_c);
+        assert(!track->sink);
+        assert(!track->remux_sink);
+
+        sub_destroy(track->d_sub);
+
+        // Demuxers can be added in any order (if they appear mid-stream), and
+        // we can't know which tracks uses which, so here's some O(n^2) trash.
+        for (int n = 0; n < num_demuxers; n++) {
+            if (demuxers[n] == track->demuxer) {
+                track->demuxer = NULL;
+                break;
+            }
+        }
+        if (track->demuxer)
+            MP_TARRAY_APPEND(NULL, demuxers, num_demuxers, track->demuxer);
+
+        talloc_free(track);
     }
     mpctx->num_tracks = 0;
 
-    demux_cancel_and_free(mpctx->demuxer);
+    kill_demuxers_reentrant(mpctx, demuxers, num_demuxers);
-    mpctx->demuxer = NULL;
+    talloc_free(demuxers);
 }
 
 #define APPEND(s, ...) mp_snprintf_cat(s, sizeof(s), __VA_ARGS__)
@@ -827,8 +909,13 @@ static void process_hooks(struct MPContext *mpctx, char *name)
 {
     mp_hook_start(mpctx, name);
 
-    while (!mp_hook_test_completion(mpctx, name))
+    while (!mp_hook_test_completion(mpctx, name)) {
         mp_idle(mpctx);
+
+        // We have no idea what blocks a hook, so just do a full abort.
+        if (mpctx->stop_play)
+            mp_abort_playback_async(mpctx);
+    }
 }
 
 // to be run on a worker thread, locked (temporarily unlocks core)
@@ -1235,9 +1322,13 @@ static void load_external_opts(struct MPContext *mpctx)
         return;
     }
 
-    while (!mp_waiter_poll(&wait))
+    while (!mp_waiter_poll(&wait)) {
         mp_idle(mpctx);
 
+        if (mpctx->stop_play)
+            mp_abort_playback_async(mpctx);
+    }
+
     mp_waiter_wait(&wait);
 }
 
@@ -1488,8 +1579,6 @@ terminate_playback:
     if (mpctx->step_frames)
         opts->pause = 1;
 
-    mp_abort_playback_async(mpctx);
-
     close_recorder(mpctx);
 
     // time to uninit all, except global stuff:
@@ -1497,12 +1586,16 @@ terminate_playback:
     uninit_audio_chain(mpctx);
     uninit_video_chain(mpctx);
     uninit_sub_all(mpctx);
-    uninit_demuxer(mpctx);
     if (!opts->gapless_audio && !mpctx->encode_lavc_ctx)
         uninit_audio_out(mpctx);
 
     mpctx->playback_initialized = false;
 
+    uninit_demuxer(mpctx);
+
+    // Possibly stop ongoing async commands.
+    mp_abort_playback_async(mpctx);
+
     m_config_restore_backups(mpctx->mconfig);
 
     TA_FREEP(&mpctx->filter_root);
@@ -1676,12 +1769,6 @@ void mp_set_playlist_entry(struct MPContext *mpctx, struct playlist_entry *e)
     assert(!e || playlist_entry_to_index(mpctx->playlist, e) >= 0);
     mpctx->playlist->current = e;
     mpctx->playlist->current_was_replaced = false;
-    // If something is currently loading, abort it a bit more forcefully. This
-    // will in particular make ytdl_hook kill the script. During normal
-    // playback, we probably don't want this, because it could upset the
-    // demuxer or decoders and spam nonsense errors.
-    if (mpctx->playing && !mpctx->playback_initialized)
-        mp_abort_playback_async(mpctx);
     // Make it pick up the new entry.
     if (!mpctx->stop_play)
         mpctx->stop_play = PT_CURRENT_ENTRY;

player/main.c

@@ -244,12 +244,6 @@ static int cfg_include(void *ctx, char *filename, int flags)
     return r;
 }
 
-static void abort_playback_cb(void *ctx)
-{
-    struct MPContext *mpctx = ctx;
-    mp_abort_playback_async(mpctx);
-}
-
 // We mostly care about LC_NUMERIC, and how "." vs. "," is treated,
 // Other locale stuff might break too, but probably isn't too bad.
 static bool check_locale(void)
@@ -320,8 +314,6 @@ struct MPContext *mp_create(void)
     cocoa_set_input_context(mpctx->input);
 #endif
 
-    mp_input_set_cancel(mpctx->input, abort_playback_cb, mpctx);
-
     char *verbose_env = getenv("MPV_VERBOSE");
     if (verbose_env)
         mpctx->opts->verbose = atoi(verbose_env);

player/osd.c

@@ -269,7 +269,8 @@ static void term_osd_print_status_lazy(struct MPContext *mpctx)
 
     if (opts->quiet || !mpctx->playback_initialized || !mpctx->playing_msg_shown)
     {
-        term_osd_set_status_lazy(mpctx, "");
+        if (!mpctx->playing)
+            term_osd_set_status_lazy(mpctx, "");
         return;
     }