From b2006eeb74f909c6e23b0b46907ada42b892f10a Mon Sep 17 00:00:00 2001
From: wm4 <wm4@nowhere>
Date: Sat, 16 Nov 2019 17:38:59 +0100
Subject: client API: remove sync. property notification code again

It's too easy to introduce unintended circular lock dependencies. Just
now we found that the (old) cocoa vo_gpu backend is also affected by
this, because it waits on the Cocoa main thread, which in turn uses
libmpv API for OSX... stuff.

Also fix a missing initial property update after observe.

This leaves me unhappy, because it just leads to a stupid thread ping
pong. Will probably rewrite this later.
---
 player/client.c | 55 ++++++++++++++-----------------------------------------
 1 file changed, 14 insertions(+), 41 deletions(-)

(limited to 'player')

diff --git a/player/client.c b/player/client.c
index a22b56a188..3a1d204ce8 100644
--- a/player/client.c
+++ b/player/client.c
@@ -66,8 +66,6 @@ struct mp_client_api {
 
     pthread_mutex_t lock;
 
-    atomic_bool uses_vo_libmpv;
-
     // -- protected by lock
 
     struct mpv_handle **clients;
@@ -95,7 +93,6 @@ struct observe_property {
     bool dead;              // property unobserved while retrieving value
     bool value_valid;
     union m_option_value value;
-    // Only if async. update is used.
     uint64_t async_change_ts;   // logical timestamp incremented on each change
     uint64_t async_value_ts;    // logical timestamp for async_value contents
     bool async_updating;        // if true, updating async_value_ts to change_ts
@@ -150,7 +147,7 @@ struct mpv_handle {
 };
 
 static bool gen_log_message_event(struct mpv_handle *ctx);
-static bool gen_property_change_event(struct mpv_handle *ctx, bool *unlocked);
+static bool gen_property_change_event(struct mpv_handle *ctx);
 static void notify_property_events(struct mpv_handle *ctx, uint64_t event_mask);
 
 void mp_clients_init(struct MPContext *mpctx)
@@ -845,15 +842,12 @@ mpv_event *mpv_wait_event(mpv_handle *ctx, double timeout)
             talloc_steal(event, event->data);
             break;
         }
-        bool unlocked = false;
         // If there's a changed property, generate change event (never queued).
-        if (gen_property_change_event(ctx, &unlocked))
+        if (gen_property_change_event(ctx))
             break;
         // Pop item from message queue, and return as event.
         if (gen_log_message_event(ctx))
             break;
-        if (unlocked)
-            continue;
         int r = wait_wakeup(ctx, deadline);
         if (r == ETIMEDOUT)
             break;
@@ -1456,6 +1450,7 @@ int mpv_observe_property(mpv_handle *ctx, uint64_t userdata,
         .format = format,
         .type = type,
         .changed = true,
+        .async_change_ts = 1,
     };
     MP_TARRAY_APPEND(ctx, ctx->properties, ctx->num_properties, prop);
     ctx->property_event_masks |= prop->event_mask;
@@ -1575,39 +1570,19 @@ static bool update_prop(struct mpv_handle *ctx, struct observe_property *prop)
     if (!prop->type)
         return true;
 
-    union m_option_value val = {0};
-    bool val_valid = false;
-
-    // With vo_libmpv, we can't lock the core for stupid reasons.
-    // Yes, that's FUCKING HORRIBLE. On the other hand, might be useful for
-    // true async. properties in the future.
-    if (atomic_load_explicit(&ctx->clients->uses_vo_libmpv, memory_order_relaxed)) {
-        if (prop->async_change_ts > prop->async_value_ts) {
-            if (!prop->async_updating) {
-                prop->async_updating = true;
-                ctx->async_counter += 1;
-                mp_dispatch_enqueue(ctx->mpctx->dispatch, update_prop_async, prop);
-            }
-            return false; // re-update later when the changed value comes in
+    if (prop->async_change_ts > prop->async_value_ts) {
+        if (!prop->async_updating) {
+            prop->async_updating = true;
+            ctx->async_counter += 1;
+            mp_dispatch_enqueue(ctx->mpctx->dispatch, update_prop_async, prop);
         }
-
-        m_option_copy(prop->type, &val, &prop->async_value);
-        val_valid = prop->async_value_valid;
-    } else {
-        pthread_mutex_unlock(&ctx->lock);
-
-        struct getproperty_request req = {
-            .mpctx = ctx->mpctx,
-            .name = prop->name,
-            .format = prop->format,
-            .data = &val,
-        };
-        run_locked(ctx, getproperty_fn, &req);
-        val_valid = req.status >= 0;
-
-        pthread_mutex_lock(&ctx->lock);
+        return false; // re-update later when the changed value comes in
     }
 
+    union m_option_value val = {0};
+    bool val_valid = prop->async_value_valid;
+    m_option_copy(prop->type, &val, &prop->async_value);
+
     bool changed = prop->value_valid != val_valid;
     if (prop->value_valid && val_valid)
         changed = !equal_mpv_value(&prop->value, &val, prop->format);
@@ -1630,7 +1605,7 @@ static bool update_prop(struct mpv_handle *ctx, struct observe_property *prop)
 
 // Set ctx->cur_event to a generated property change event, if there is any
 // outstanding property.
-static bool gen_property_change_event(struct mpv_handle *ctx, bool *unlocked)
+static bool gen_property_change_event(struct mpv_handle *ctx)
 {
     if (!ctx->mpctx->initialized)
         return false;
@@ -1651,7 +1626,6 @@ static bool gen_property_change_event(struct mpv_handle *ctx, bool *unlocked)
         if (prop->changed && !prop->dead) {
             prop->changed = false;
             updated = update_prop(ctx, prop);
-            *unlocked = true; // not always; but good enough
         }
 
         if (prop->dead) {
@@ -1915,7 +1889,6 @@ bool mp_set_main_render_context(struct mp_client_api *client_api,
     if (res)
         client_api->render_context = active ? ctx : NULL;
     pthread_mutex_unlock(&client_api->lock);
-    atomic_store(&client_api->uses_vo_libmpv, active);
     return res;
 }
 
-- 
cgit v1.2.3