diff --git a/DOCS/interface-changes.rst b/DOCS/interface-changes.rst index b2974eff12..177cc908fd 100644 --- a/DOCS/interface-changes.rst +++ b/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 diff --git a/DOCS/man/options.rst b/DOCS/man/options.rst index 4c4954a99d..d56b52b87b 100644 --- a/DOCS/man/options.rst +++ b/DOCS/man/options.rst @@ -2951,6 +2951,19 @@ Demuxer Disabling this option is not recommended. Use it for debugging only. +``--demuxer-termination-timeout=`` + 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=`` If ``--demuxer-thread`` is enabled, this controls how much the demuxer should buffer ahead in seconds (default: 1). As long as no packet has diff --git a/options/options.c b/options/options.c index 49aac52ba2..bfaf374365 100644 --- a/options/options.c +++ b/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, diff --git a/options/options.h b/options/options.h index c38b1b9f73..b4c0f9e719 100644 --- a/options/options.h +++ b/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; diff --git a/player/client.c b/player/client.c index 16e01aa43e..03e0cbfeb6 100644 --- a/player/client.c +++ b/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 = { diff --git a/player/loadfile.c b/player/loadfile.c index a28bf491b4..bc2886a5d6 100644 --- a/player/loadfile.c +++ b/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 - for (int n = mpctx->num_tracks - 1; n >= 0; n--) { - mpctx->tracks[n]->selected = false; - mp_remove_track(mpctx, mpctx->tracks[n]); - } + struct demuxer **demuxers = NULL; + int num_demuxers = 0; + + 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); - talloc_free(mpctx->tracks[i]); + struct track *track = 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); - mpctx->demuxer = NULL; + kill_demuxers_reentrant(mpctx, demuxers, num_demuxers); + 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; diff --git a/player/main.c b/player/main.c index 6a7fc68004..ec5cdd69b1 100644 --- a/player/main.c +++ b/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); diff --git a/player/osd.c b/player/osd.c index 52c9b83286..0ae8dcd1f4 100644 --- a/player/osd.c +++ b/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; }