client API: cleanup mpv_handle termination

This changes how mpv_terminate_destroy() and mpv_detach_destroy()
behave. The doxygen in client.h tries to point out the differences. The
goal is to make this more useful to the API user (making it behave like
refcounting).

This will be refined in follow up commits.

Initialization is unfortunately closely tied to termination, so that
changes as well. This also removes earlier hacks that make sure that
some parts of FFmpeg initialization are run in the playback thread
(instead of the user's thread). This does not matter with standard
FFmpeg, and I have no reason to care about this anymore.

4 files changed, 121 insertions, 106 deletions

diff --git a/player/client.c b/player/client.c
index 4a7307194a..a887a16d2a 100644
--- a/player/client.c
+++ b/player/client.c
@@ -69,6 +69,8 @@ struct mp_client_api {
     int num_clients;
     uint64_t event_masks; // combined events of all clients, or 0 if unknown
     bool shutting_down; // do not allow new clients
+    bool have_terminator; // a client took over the role of destroying the core
+    bool terminate_core_thread; // make libmpv core thread exit
 
     struct mp_custom_protocol *custom_protocols;
     int num_custom_protocols;
@@ -95,7 +97,6 @@ struct observe_property {
 struct mpv_handle {
     // -- immmutable
     char name[MAX_CLIENT_NAME];
-    bool owner;
     struct mp_log *log;
     struct MPContext *mpctx;
     struct mp_client_api *clients;
@@ -172,14 +173,6 @@ void mp_clients_destroy(struct MPContext *mpctx)
     mpctx->clients = NULL;
 }
 
-int mp_clients_num(struct MPContext *mpctx)
-{
-    pthread_mutex_lock(&mpctx->clients->lock);
-    int num_clients = mpctx->clients->num_clients;
-    pthread_mutex_unlock(&mpctx->clients->lock);
-    return num_clients;
-}
-
 // Test for "fuzzy" initialization of all clients. That is, all clients have
 // at least called mpv_wait_event() at least once since creation (or exited).
 bool mp_clients_all_initialized(struct MPContext *mpctx)
@@ -221,13 +214,6 @@ bool mp_client_exists(struct MPContext *mpctx, const char *client_name)
     return r;
 }
 
-void mp_client_enter_shutdown(struct MPContext *mpctx)
-{
-    pthread_mutex_lock(&mpctx->clients->lock);
-    mpctx->clients->shutting_down = true;
-    pthread_mutex_unlock(&mpctx->clients->lock);
-}
-
 struct mpv_handle *mp_new_client(struct mp_client_api *clients, const char *name)
 {
     pthread_mutex_lock(&clients->lock);
@@ -272,6 +258,9 @@ struct mpv_handle *mp_new_client(struct mp_client_api *clients, const char *name
 
     MP_TARRAY_APPEND(clients, clients->clients, clients->num_clients, client);
 
+    if (clients->num_clients == 1 && !clients->mpctx->is_cli)
+        client->fuzzy_initialized = true;
+
     clients->event_masks = 0;
     pthread_mutex_unlock(&clients->lock);
 
@@ -373,24 +362,34 @@ void mpv_wait_async_requests(mpv_handle *ctx)
     pthread_mutex_unlock(&ctx->lock);
 }
 
-void mpv_detach_destroy(mpv_handle *ctx)
+static void get_thread(void *ptr)
+{
+    *(pthread_t *)ptr = pthread_self();
+}
+
+static void mp_destroy_client(mpv_handle *ctx, bool terminate)
 {
     if (!ctx)
         return;
 
+    struct MPContext *mpctx = ctx->mpctx;
+    struct mp_client_api *clients = ctx->clients;
+
     MP_VERBOSE(ctx, "Exiting...\n");
 
+    if (terminate)
+        mpv_command(ctx, (const char*[]){"quit", NULL});
+
     // reserved_events equals the number of asynchronous requests that weren't
     // yet replied. In order to avoid that trying to reply to a removed client
     // causes a crash, block until all asynchronous requests were served.
     mpv_wait_async_requests(ctx);
 
-    osd_set_external(ctx->mpctx->osd, ctx, 0, 0, NULL);
-    mp_input_remove_sections_by_owner(ctx->mpctx->input, ctx->name);
-
-    struct mp_client_api *clients = ctx->clients;
+    osd_set_external(mpctx->osd, ctx, 0, 0, NULL);
+    mp_input_remove_sections_by_owner(mpctx->input, ctx->name);
 
     pthread_mutex_lock(&clients->lock);
+
     for (int n = 0; n < clients->num_clients; n++) {
         if (clients->clients[n] == ctx) {
             MP_TARRAY_REMOVE_AT(clients->clients, clients->num_clients, n);
@@ -409,76 +408,98 @@ void mpv_detach_destroy(mpv_handle *ctx)
             }
             talloc_free(ctx);
             ctx = NULL;
-            // shutdown_clients() sleeps to avoid wasting CPU.
-            // mp_hook_test_completion() also relies on this a bit.
-            mp_wakeup_core(clients->mpctx);
             break;
         }
     }
-    pthread_mutex_unlock(&clients->lock);
     assert(!ctx);
+
+    if (mpctx->is_cli) {
+        terminate = false;
+    } else {
+        // If the last mpv_handle got destroyed, destroy the core.
+        if (clients->num_clients == 0)
+            terminate = true;
+
+        // Reserve the right to destroy mpctx for us.
+        if (clients->have_terminator)
+            terminate = false;
+        clients->have_terminator |= terminate;
+    }
+
+    // mp_shutdown_clients() sleeps to avoid wasting CPU.
+    // mp_hook_test_completion() also relies on this a bit.
+    mp_wakeup_core(mpctx);
+
+    pthread_mutex_unlock(&clients->lock);
+
+    // Note that even if num_clients==0, having set have_terminator keeps mpctx
+    // and the core thread alive.
+    if (terminate) {
+        // Make sure the core stops playing files etc. Being able to lock the
+        // dispatch queue requires that the core thread is still active.
+        mp_dispatch_lock(mpctx->dispatch);
+        mpctx->stop_play = PT_QUIT;
+        mp_dispatch_unlock(mpctx->dispatch);
+
+        // Stop the core thread.
+        pthread_mutex_lock(&clients->lock);
+        clients->terminate_core_thread = true;
+        pthread_mutex_unlock(&clients->lock);
+        mp_wakeup_core(mpctx);
+
+        // Blocking wait for all clients and core thread to terminate.
+        pthread_t playthread;
+        mp_dispatch_run(mpctx->dispatch, get_thread, &playthread);
+
+        pthread_join(playthread, NULL);
+
+        mp_destroy(mpctx);
+    }
 }
 
-static void get_thread(void *ptr)
+void mpv_detach_destroy(mpv_handle *ctx)
 {
-    *(pthread_t *)ptr = pthread_self();
+    mp_destroy_client(ctx, false);
 }
 
 void mpv_terminate_destroy(mpv_handle *ctx)
 {
-    if (!ctx)
-        return;
+    mp_destroy_client(ctx, true);
+}
 
-    if (ctx->mpctx->initialized) {
-        mpv_command(ctx, (const char*[]){"quit", NULL});
-    } else {
-        mp_dispatch_lock(ctx->mpctx->dispatch);
-        ctx->mpctx->stop_play = PT_QUIT;
-        mp_dispatch_unlock(ctx->mpctx->dispatch);
-    }
+static bool can_terminate(struct MPContext *mpctx)
+{
+    struct mp_client_api *clients = mpctx->clients;
 
-    if (!ctx->owner) {
-        mpv_detach_destroy(ctx);
-        return;
-    }
+    pthread_mutex_lock(&clients->lock);
+    bool ok = clients->num_clients == 0 && mpctx->outstanding_async == 0 &&
+              (mpctx->is_cli || clients->terminate_core_thread);
+    pthread_mutex_unlock(&clients->lock);
 
-    mp_dispatch_lock(ctx->mpctx->dispatch);
-    assert(ctx->mpctx->autodetach);
-    ctx->mpctx->autodetach = false;
-    mp_dispatch_unlock(ctx->mpctx->dispatch);
+    return ok;
+}
 
-    pthread_t playthread;
-    mp_dispatch_run(ctx->mpctx->dispatch, get_thread, &playthread);
+// Can be called on the core thread only. Idempotent.
+void mp_shutdown_clients(struct MPContext *mpctx)
+{
+    struct mp_client_api *clients = mpctx->clients;
 
-    mpv_detach_destroy(ctx);
+    // Prevent that new clients can appear.
+    pthread_mutex_lock(&clients->lock);
+    clients->shutting_down = true;
+    pthread_mutex_unlock(&clients->lock);
 
-    // And this is also the reason why we only allow 1 thread (the owner) to
-    // call this function.
-    pthread_join(playthread, NULL);
+    while (!can_terminate(mpctx)) {
+        mp_client_broadcast_event(mpctx, MPV_EVENT_SHUTDOWN, NULL);
+        mp_wait_events(mpctx);
+    }
 }
 
-static void *core_thread(void *tag)
+static void *core_thread(void *p)
 {
-    mpthread_set_name("mpv core");
+    struct MPContext *mpctx = p;
 
-    mpv_handle *ctx = NULL;
-    struct MPContext *mpctx = mp_create();
-    if (mpctx) {
-        mpctx->autodetach = true;
-        ctx = mp_new_client(mpctx->clients, "main");
-        if (ctx) {
-            ctx->owner = true;
-            ctx->fuzzy_initialized = true;
-            m_config_set_profile(mpctx->mconfig, "libmpv", 0);
-        } else {
-            mp_destroy(mpctx);
-        }
-    }
-
-    // Let mpv_create() return, and pass it the handle.
-    mp_rendezvous(tag, (intptr_t)(void *)ctx);
-    if (!ctx)
-        return NULL;
+    mpthread_set_name("mpv core");
 
     while (!mpctx->initialized && mpctx->stop_play != PT_QUIT)
         mp_idle(mpctx);
@@ -487,24 +508,36 @@ static void *core_thread(void *tag)
         mp_play_files(mpctx);
 
     // This actually waits until all clients are gone before actually
-    // destroying mpctx.
-    mp_destroy(mpctx);
+    // destroying mpctx. Actual destruction is done by whatever destroys
+    // the last mpv_handle.
+    mp_shutdown_clients(mpctx);
 
     return NULL;
 }
 
 mpv_handle *mpv_create(void)
 {
-    char tag;
-    pthread_t thread;
-    if (pthread_create(&thread, NULL, core_thread, &tag) != 0)
+    struct MPContext *mpctx = mp_create();
+    if (!mpctx)
         return NULL;
 
-    mpv_handle *res = (void *)mp_rendezvous(&tag, 0);
-    if (!res)
-        pthread_join(thread, NULL);
+    m_config_set_profile(mpctx->mconfig, "libmpv", 0);
 
-    return res;
+    mpv_handle *ctx = mp_new_client(mpctx->clients, "main");
+    if (!ctx) {
+        mp_destroy(mpctx);
+        return NULL;
+    }
+
+    pthread_t thread;
+    if (pthread_create(&thread, NULL, core_thread, mpctx) != 0) {
+        ctx->clients->have_terminator = true; // avoid blocking
+        mpv_terminate_destroy(ctx);
+        mp_destroy(mpctx);
+        return NULL;
+    }
+
+    return ctx;
 }
 
 mpv_handle *mpv_create_client(mpv_handle *ctx, const char *name)
@@ -517,19 +550,12 @@ mpv_handle *mpv_create_client(mpv_handle *ctx, const char *name)
     return new;
 }
 
-static void doinit(void *ctx)
-{
-    void **args = ctx;
-
-    *(int *)args[1] = mp_initialize(args[0], NULL);
-}
-
 int mpv_initialize(mpv_handle *ctx)
 {
-    int res = 0;
-    void *args[2] = {ctx->mpctx, &res};
-    mp_dispatch_run(ctx->mpctx->dispatch, doinit, args);
-    return res == 0 ? 0 : MPV_ERROR_INVALID_PARAMETER;
+    lock_core(ctx);
+    int res = mp_initialize(ctx->mpctx, NULL) ? MPV_ERROR_INVALID_PARAMETER : 0;
+    unlock_core(ctx);
+    return res;
 }
 
 // set ev->data to a new copy of the original data
diff --git a/player/client.h b/player/client.h
index 118f6800f6..deec3c793b 100644
--- a/player/client.h
+++ b/player/client.h
@@ -18,9 +18,8 @@ struct mpv_global;
 #define MAX_CLIENT_NAME 64
 
 void mp_clients_init(struct MPContext *mpctx);
-void mp_client_enter_shutdown(struct MPContext *mpctx);
 void mp_clients_destroy(struct MPContext *mpctx);
-int mp_clients_num(struct MPContext *mpctx);
+void mp_shutdown_clients(struct MPContext *mpctx);
 bool mp_clients_all_initialized(struct MPContext *mpctx);
 
 bool mp_client_exists(struct MPContext *mpctx, const char *client_name);
diff --git a/player/core.h b/player/core.h
index 25c006f2e4..0eca484dc2 100644
--- a/player/core.h
+++ b/player/core.h
@@ -225,7 +225,7 @@ enum playback_status {
 
 typedef struct MPContext {
     bool initialized;
-    bool autodetach;
+    bool is_cli;
     struct mpv_global *global;
     struct MPOpts *opts;
     struct mp_log *log;
diff --git a/player/main.c b/player/main.c
index bb83accb7e..bb80591220 100644
--- a/player/main.c
+++ b/player/main.c
@@ -154,18 +154,9 @@ void mp_print_version(struct mp_log *log, int always)
     }
 }
 
-static void shutdown_clients(struct MPContext *mpctx)
-{
-    mp_client_enter_shutdown(mpctx);
-    while (mp_clients_num(mpctx) || mpctx->outstanding_async) {
-        mp_client_broadcast_event(mpctx, MPV_EVENT_SHUTDOWN, NULL);
-        mp_wait_events(mpctx);
-    }
-}
-
 void mp_destroy(struct MPContext *mpctx)
 {
-    shutdown_clients(mpctx);
+    mp_shutdown_clients(mpctx);
 
     mp_uninit_ipc(mpctx->ipc_ctx);
     mpctx->ipc_ctx = NULL;
@@ -199,9 +190,6 @@ void mp_destroy(struct MPContext *mpctx)
 
     uninit_libav(mpctx->global);
 
-    if (mpctx->autodetach)
-        pthread_detach(pthread_self());
-
     mp_msg_uninit(mpctx->global);
     pthread_mutex_destroy(&mpctx->lock);
     talloc_free(mpctx);
@@ -461,6 +449,8 @@ int mpv_main(int argc, char *argv[])
     if (!mpctx)
         return 1;
 
+    mpctx->is_cli = true;
+
     char **options = argv && argv[0] ? argv + 1 : NULL; // skips program name
     int r = mp_initialize(mpctx, options);
     if (r == 0)