From 12d1404b04e90f5357882e5c1048d92305248cb9 Mon Sep 17 00:00:00 2001 From: wm4 Date: Fri, 18 May 2018 21:38:17 +0200 Subject: 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. --- player/command.c | 21 +++++++++-- player/core.h | 5 +-- player/loadfile.c | 109 +++++++++++++++++++++++++++++++----------------------- 3 files changed, 81 insertions(+), 54 deletions(-) (limited to 'player') 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, ¶ms, mpctx->playback_abort, mpctx->global); + demux_open_url(filename, ¶ms, 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); -- cgit v1.2.3