summaryrefslogtreecommitdiffstats
path: root/player/client.c
diff options
context:
space:
mode:
authorwm4 <wm4@nowhere>2020-03-06 23:59:21 +0100
committerwm4 <wm4@nowhere>2020-03-06 23:59:21 +0100
commit8a4f812b76befa9529591e274cefdd2f234fb450 (patch)
tree957968e219fcb448fb515e24db90aa59d1da3b01 /player/client.c
parent575197ff8b0a0d8cd14f7ee78461c1d61d045d72 (diff)
downloadmpv-8a4f812b76befa9529591e274cefdd2f234fb450.tar.bz2
mpv-8a4f812b76befa9529591e274cefdd2f234fb450.tar.xz
client API: avoid returning stale value on property notifications
This could happen if a property was flagged as changed, then updated, then flagged again, but gen_property_change_event() was called before the value was updated a second time. Then the function simply returned the old value, and would later trigger a new change event again. This was considered acceptable before, since property notifications are asynchronous anyway (so they may always be "outdated", it just mattered whether the most recent value was eventually delivered). But consider ordering with events. It seems desirable that specific important events (e.g. MPV_EVENT_START_FILE) should not be followed by property updates that happened before it, because that would make application logic really a mess, and property notification near-useless in certain cases. Avoid this by never returning a value if it was marked changed, but not updated yet. Unfortunately, this could lead to clients never receiving a value (or receiving it with a high random delay), if they're too slow to read it (or the property simply updates too often). Note that this is done for _all_ property notifications, not just returned events. Hopefully not a problem in practice. If it turns out to be one, this mechanism could be restricted to actually returned events, for which this really matters.
Diffstat (limited to 'player/client.c')
-rw-r--r--player/client.c12
1 files changed, 8 insertions, 4 deletions
diff --git a/player/client.c b/player/client.c
index ffb0665c40..815c64c005 100644
--- a/player/client.c
+++ b/player/client.c
@@ -1638,10 +1638,12 @@ static void send_client_property_changes(struct mpv_handle *ctx)
changed = true;
}
- if (changed) {
- ctx->new_property_events = true;
- } else if (prop->value_ret_ts == prop->value_ts) {
+ // Avoid retriggering the change event if the property didn't change,
+ // and the previous value was actually returned to the client.
+ if (!changed && prop->value_ret_ts == prop->value_ts) {
prop->value_ret_ts = prop->change_ts; // no change => no event
+ } else {
+ ctx->new_property_events = true;
}
prop->value_ts = prop->change_ts;
@@ -1698,7 +1700,9 @@ static bool gen_property_change_event(struct mpv_handle *ctx)
struct observe_property *prop = ctx->properties[ctx->cur_property_index++];
- if (prop->value_ret_ts != prop->value_ts) {
+ if (prop->value_ts == prop->change_ts && // not a stale value?
+ prop->value_ret_ts != prop->value_ts) // other value than last time?
+ {
prop->value_ret_ts = prop->value_ts;
prop_unref(ctx->cur_property);
ctx->cur_property = prop;