summaryrefslogtreecommitdiffstats
path: root/player
diff options
context:
space:
mode:
authorwm4 <wm4@nowhere>2019-10-25 01:57:51 +0200
committerwm4 <wm4@nowhere>2019-10-25 01:57:51 +0200
commitd66eb93e5d45892cc0cdd72a579907e251d9953b (patch)
tree961adefdd013590256a6e4622c58ce36efae7536 /player
parent767c35c8839359593f7b0af14bcfe1cb92c41162 (diff)
downloadmpv-d66eb93e5d45892cc0cdd72a579907e251d9953b.tar.bz2
mpv-d66eb93e5d45892cc0cdd72a579907e251d9953b.tar.xz
client API: add async path; fix deadlock for vo_libmpv users
In commit 065c307e8e7db, I broke everything. It seemed like a nice idea, but it explicitly broke an assumption vo_libmpv were explicitly allowed to make: that observing properties does not lock the core. The commit did just that and locked the core for property updates. This made for example mpv's own OSX backend freeze (it uses vo_libmpv for convenience to make up for Apple's incredibly broken OpenGL shit). I don't want to revert that commit just because vo_libmpv's design is horrible. So instead add an optional asynchronous path, that is only used if vo_libmpv is in use (best idea ever?). Interestingly, this isn't so hard. It adds about 90 lines of code, which are only run on OSX and libmpv users, so I don't have to care about the crashes and weird behavior this might cause. It even worked on the first try except for a quickly fixed memory leak. The code path can be tested anywhere by just turning the uses_vo_libmpv condition into always true. The atomic is out of laziness. Saves some thinking how to get around the locking order.
Diffstat (limited to 'player')
-rw-r--r--player/client.c104
1 files changed, 95 insertions, 9 deletions
diff --git a/player/client.c b/player/client.c
index 8df4b26b99..6d95faae48 100644
--- a/player/client.c
+++ b/player/client.c
@@ -39,6 +39,7 @@
#include "options/m_property.h"
#include "options/path.h"
#include "options/parse_configfile.h"
+#include "osdep/atomic.h"
#include "osdep/threads.h"
#include "osdep/timer.h"
#include "osdep/io.h"
@@ -65,6 +66,8 @@ struct mp_client_api {
pthread_mutex_t lock;
+ atomic_bool uses_vo_libmpv;
+
// -- protected by lock
struct mpv_handle **clients;
@@ -81,6 +84,7 @@ struct mp_client_api {
};
struct observe_property {
+ struct mpv_handle *owner;
char *name;
int id; // ==mp_get_property_id(name)
uint64_t event_mask; // ==mp_get_property_event_mask(name)
@@ -91,6 +95,12 @@ 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
+ bool async_value_valid;
+ union m_option_value async_value;
};
struct mpv_handle {
@@ -126,6 +136,7 @@ struct mpv_handle {
int first_event; // events[first_event] is the first readable event
int num_events; // number of readable events
int reserved_events; // number of entries reserved for replies
+ size_t async_counter; // pending other async events
bool choked; // recovering from queue overflow
struct observe_property **properties;
@@ -355,7 +366,7 @@ static void unlock_core(mpv_handle *ctx)
void mpv_wait_async_requests(mpv_handle *ctx)
{
pthread_mutex_lock(&ctx->lock);
- while (ctx->reserved_events)
+ while (ctx->reserved_events || ctx->async_counter)
wait_wakeup(ctx, INT64_MAX);
pthread_mutex_unlock(&ctx->lock);
}
@@ -1416,8 +1427,13 @@ int mpv_get_property_async(mpv_handle *ctx, uint64_t ud, const char *name,
static void property_free(void *p)
{
struct observe_property *prop = p;
- if (prop->type)
+
+ assert(!prop->async_updating);
+
+ if (prop->type) {
m_option_free(prop->type, &prop->value);
+ m_option_free(prop->type, &prop->async_value);
+ }
}
int mpv_observe_property(mpv_handle *ctx, uint64_t userdata,
@@ -1434,6 +1450,7 @@ int mpv_observe_property(mpv_handle *ctx, uint64_t userdata,
struct observe_property *prop = talloc_ptrtype(ctx, prop);
talloc_set_destructor(prop, property_free);
*prop = (struct observe_property){
+ .owner = ctx,
.name = talloc_strdup(prop, name),
.id = mp_get_property_id(ctx->mpctx, name),
.event_mask = mp_get_property_event_mask(name),
@@ -1453,6 +1470,7 @@ static void mark_property_changed(struct mpv_handle *client, int index)
{
struct observe_property *prop = client->properties[index];
prop->changed = true;
+ prop->async_change_ts += 1;
client->lowest_changed = MPMIN(client->lowest_changed, index);
}
@@ -1509,13 +1527,18 @@ static void notify_property_events(struct mpv_handle *ctx, uint64_t event_mask)
wakeup_client(ctx);
}
-static bool update_prop(struct mpv_handle *ctx, struct observe_property *prop)
+static void update_prop_async(void *p)
{
- if (!prop->type)
- return true;
+ struct observe_property *prop = p;
+ struct mpv_handle *ctx = prop->owner;
union m_option_value val = {0};
+ bool val_valid = false;
+ uint64_t value_ts;
+ pthread_mutex_lock(&ctx->lock);
+ value_ts = prop->async_change_ts;
+ assert(prop->async_updating);
pthread_mutex_unlock(&ctx->lock);
struct getproperty_request req = {
@@ -1524,11 +1547,71 @@ static bool update_prop(struct mpv_handle *ctx, struct observe_property *prop)
.format = prop->format,
.data = &val,
};
- run_locked(ctx, getproperty_fn, &req);
+ getproperty_fn(&req);
+ val_valid = req.status >= 0;
pthread_mutex_lock(&ctx->lock);
- bool val_valid = req.status >= 0;
+ assert(prop->async_updating);
+
+ // Move to prop->async_value
+ m_option_free(prop->type, &prop->async_value);
+ memcpy(&prop->async_value, &val, prop->type->type->size);
+ prop->async_value_valid = val_valid;
+
+ prop->async_value_ts = value_ts;
+ prop->async_updating = false;
+
+ // Cause it to re-check the property.
+ prop->changed = true;
+ ctx->lowest_changed = 0;
+
+ ctx->async_counter -= 1;
+ wakeup_client(ctx);
+
+ pthread_mutex_unlock(&ctx->lock);
+}
+
+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
+ }
+
+ // Move to val
+ memcpy(&val, &prop->async_value, prop->type->type->size);
+ val_valid = prop->async_value_valid;
+ prop->async_value = (union m_option_value){0};
+ prop->async_value_valid = false;
+ } 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);
+ }
bool changed = prop->value_valid != val_valid;
if (prop->value_valid && val_valid)
@@ -1603,8 +1686,10 @@ static bool gen_property_change_event(struct mpv_handle *ctx, bool *unlocked)
for (int n = ctx->num_properties - 1; n >= 0; n--) {
struct observe_property *prop = ctx->properties[n];
if (prop->dead) {
- MP_TARRAY_REMOVE_AT(ctx->properties, ctx->num_properties, n);
- talloc_free(prop);
+ if (!prop->async_updating) {
+ MP_TARRAY_REMOVE_AT(ctx->properties, ctx->num_properties, n);
+ talloc_free(prop);
+ }
} else {
ctx->property_event_masks |= prop->event_mask;
}
@@ -1836,6 +1921,7 @@ 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;
}