When the current file changes (or rather, when starting/finishing playback of a playlist entry), clients tend to have the problem that it's hard to tell whether a property change notification (via mpv_observe_property() and mechanisms layered on top of it) is from the previous or new playlist entry. The previous commit probably helps, but all the asynchronity is still a bit unhelpful. Try to make this better by adding new hooks, that are run before/after playback init/deinit. This is similar to the existing hooks, except they're outside of "initialized" playback, which excludes that you might accidentally get an overlap between the current and the previous/next playlist entry. That still doesn't seem quite enough, since normally, property change notifications come after the hook event. So basically a client would have to explicitly "drain" the event queue within the hook, and make the hook continue only after that is done. Knowing when property notifications are done is another asynchronous nightmare (how exactly it works keeps changing within client.c, and an API user probably can't tell anymore when all pending properties are truly done). So introduce another guarantee: properties that were changed before the hook happens will be returned before the hook event is returned. That means the client will have received all pending property notifications from the previous playlist entry (or whatever) before the hook is entered. As another minor complication, we shouldn't just keep the hook pending until _all_ property notifications are done, since the client's hook could produce new ones. (Or just consider things like the demuxer thread hammering the client with cache update events, while the "on_preloaded" hook is run.) So there is some extra untested, fragile logic in client.c to handle this (the waiting_for_hook flag). This probably works, but was barely tested. Not sure if this helps anyone, but I think it's fine for my own purposes. (I really hated this aspect of the API whenever I used it myself.)
Unified · Split
| @@ -1253,6 +1253,12 @@ the player freeze randomly. Basically, nobody should use this API. · 1252 unmodified lines | ||
| 1253 | 1253 | The C API is described in the header files. The Lua API is described in the |
| 1254 | 1254 | Lua section. |
| 1255 | 1255 | |
| 1256 | +Before a hook is actually invoked on an API clients, it will attempt to return | |
| 1257 | +new values for all observed properties that were changed before the hook. This | |
| 1258 | +may make it easier for an application to set defined "barriers" between property | |
| 1259 | +change notifications by registering hooks. (That means these hooks will have an | |
| 1260 | +effect, even if you do nothing and make them continue immediately.) | |
| 1261 | + | |
| 1256 | 1262 | The following hooks are currently defined: |
| 1257 | 1263 | |
| 1258 | 1264 | ``on_load`` |
| @@ -1286,6 +1292,16 @@ The following hooks are currently defined: · 27 unmodified lines | ||
| 1286 | 1292 | Run before closing a file, and before actually uninitializing |
| 1287 | 1293 | everything. It's not possible to resume playback in this state. |
| 1288 | 1294 | |
| 1295 | +``on_before_start_file`` | |
| 1296 | + Run before a ``start-file`` event is sent. (If any client changes the | |
| 1297 | + current playlist entry, or sends a quit command to the player, the | |
| 1298 | + corresponding event will not actually happen after the hook returns.) | |
| 1299 | + Useful to drain property changes before a new file is loaded. | |
| 1300 | + | |
| 1301 | +``on_after_end_file`` | |
| 1302 | + Run after an ``end-file`` event. Useful to drain property changes after a | |
| 1303 | + file has finished. | |
| 1304 | + | |
| 1289 | 1305 | Input Command Prefixes |
| 1290 | 1306 | ---------------------- |
| 1291 | 1307 | |
| @@ -0,0 +1,32 @@ | ||
| 1 | +local utils = require("mp.utils") | |
| 2 | + | |
| 3 | +function hardsleep() | |
| 4 | + os.execute("sleep 1s") | |
| 5 | +end | |
| 6 | + | |
| 7 | +local hooks = {"on_before_start_file", "on_load", "on_load_fail", | |
| 8 | + "on_preloaded", "on_unload", "on_after_end_file"} | |
| 9 | + | |
| 10 | +for _, name in ipairs(hooks) do | |
| 11 | + mp.add_hook(name, 0, function() | |
| 12 | + print("--- hook: " .. name) | |
| 13 | + hardsleep() | |
| 14 | + print(" ... continue") | |
| 15 | + end) | |
| 16 | +end | |
| 17 | + | |
| 18 | +local events = {"start-file", "end-file", "file-loaded", "seek", | |
| 19 | + "playback-restart", "idle", "shutdown"} | |
| 20 | +for _, name in ipairs(events) do | |
| 21 | + mp.register_event(name, function() | |
| 22 | + print("--- event: " .. name) | |
| 23 | + end) | |
| 24 | +end | |
| 25 | + | |
| 26 | +local props = {"path", "metadata"} | |
| 27 | +for _, name in ipairs(props) do | |
| 28 | + mp.observe_property(name, "native", function(name, val) | |
| 29 | + print("property '" .. name .. "' changed to '" .. | |
| 30 | + utils.to_string(val) .. "'") | |
| 31 | + end) | |
| 32 | +end | |
| @@ -102,6 +102,7 @@ struct observe_property { · 101 unmodified lines | ||
| 102 | 102 | union m_option_value value; |
| 103 | 103 | uint64_t value_ret_ts; // logical timestamp of value returned to user |
| 104 | 104 | union m_option_value value_ret; |
| 105 | + bool waiting_for_hook; // flag for draining old property changes on a hook | |
| 105 | 106 | }; |
| 106 | 107 | |
| 107 | 108 | struct mpv_handle { |
| @@ -140,6 +141,7 @@ struct mpv_handle { · 32 unmodified lines | ||
| 140 | 141 | size_t async_counter; // pending other async events |
| 141 | 142 | bool choked; // recovering from queue overflow |
| 142 | 143 | bool destroying; // pending destruction; no API accesses allowed |
| 144 | + bool hook_pending; // hook events are returned after draining properties | |
| 143 | 145 | |
| 144 | 146 | struct observe_property **properties; |
| 145 | 147 | int num_properties; |
| @@ -847,6 +849,27 @@ int mpv_request_event(mpv_handle *ctx, mpv_event_id event, int enable) · 701 unmodified lines | ||
| 847 | 849 | return 0; |
| 848 | 850 | } |
| 849 | 851 | |
| 852 | +// Set waiting_for_hook==true for all possibly pending properties. | |
| 853 | +static void set_wait_for_hook_flags(mpv_handle *ctx) | |
| 854 | +{ | |
| 855 | + for (int n = 0; n < ctx->num_properties; n++) { | |
| 856 | + struct observe_property *prop = ctx->properties[n]; | |
| 857 | + | |
| 858 | + if (prop->value_ret_ts != prop->change_ts) | |
| 859 | + prop->waiting_for_hook = true; | |
| 860 | + } | |
| 861 | +} | |
| 862 | + | |
| 863 | +// Return whether any property still has waiting_for_hook set. | |
| 864 | +static bool check_for_for_hook_flags(mpv_handle *ctx) | |
| 865 | +{ | |
| 866 | + for (int n = 0; n < ctx->num_properties; n++) { | |
| 867 | + if (ctx->properties[n]->waiting_for_hook) | |
| 868 | + return true; | |
| 869 | + } | |
| 870 | + return false; | |
| 871 | +} | |
| 872 | + | |
| 850 | 873 | mpv_event *mpv_wait_event(mpv_handle *ctx, double timeout) |
| 851 | 874 | { |
| 852 | 875 | mpv_event *event = ctx->cur_event; |
| @@ -874,8 +897,24 @@ mpv_event *mpv_wait_event(mpv_handle *ctx, double timeout) · 21 unmodified lines | ||
| 874 | 897 | event->event_id = MPV_EVENT_QUEUE_OVERFLOW; |
| 875 | 898 | break; |
| 876 | 899 | } |
| 877 | − if (ctx->num_events) { | |
| 878 | − *event = ctx->events[ctx->first_event]; | |
| 900 | + struct mpv_event *ev = | |
| 901 | + ctx->num_events ? &ctx->events[ctx->first_event] : NULL; | |
| 902 | + if (ev && ev->event_id == MPV_EVENT_HOOK) { | |
| 903 | + // Give old property notifications priority over hooks. This is a | |
| 904 | + // guarantee given to clients to simplify their logic. New property | |
| 905 | + // changes after this are treated normally, so | |
| 906 | + if (!ctx->hook_pending) { | |
| 907 | + ctx->hook_pending = true; | |
| 908 | + set_wait_for_hook_flags(ctx); | |
| 909 | + } | |
| 910 | + if (check_for_for_hook_flags(ctx)) { | |
| 911 | + ev = NULL; // delay | |
| 912 | + } else { | |
| 913 | + ctx->hook_pending = false; | |
| 914 | + } | |
| 915 | + } | |
| 916 | + if (ev) { | |
| 917 | + *event = *ev; | |
| 879 | 918 | ctx->first_event = (ctx->first_event + 1) % ctx->max_events; |
| 880 | 919 | ctx->num_events--; |
| 881 | 920 | talloc_steal(event, event->data); |
| @@ -1608,7 +1647,7 @@ static void send_client_property_changes(struct mpv_handle *ctx) · 726 unmodified lines | ||
| 1608 | 1647 | ctx->async_counter -= 1; |
| 1609 | 1648 | prop_unref(prop); |
| 1610 | 1649 | |
| 1611 | − // Set of observed properties was changed or something similar | |
| 1650 | + // Set if observed properties was changed or something similar | |
| 1612 | 1651 | // => start over, retry next time. |
| 1613 | 1652 | if (cur_ts != ctx->properties_change_ts || ctx->destroying) { |
| 1614 | 1653 | m_option_free(type, &val); |
| @@ -1638,10 +1677,14 @@ static void send_client_property_changes(struct mpv_handle *ctx) · 23 unmodified lines | ||
| 1638 | 1677 | changed = true; |
| 1639 | 1678 | } |
| 1640 | 1679 | |
| 1680 | + if (prop->waiting_for_hook) | |
| 1681 | + ctx->new_property_events = true; // make sure to wakeup | |
| 1682 | + | |
| 1641 | 1683 | // Avoid retriggering the change event if the property didn't change, |
| 1642 | 1684 | // and the previous value was actually returned to the client. |
| 1643 | 1685 | if (!changed && prop->value_ret_ts == prop->value_ts) { |
| 1644 | 1686 | prop->value_ret_ts = prop->change_ts; // no change => no event |
| 1687 | + prop->waiting_for_hook = false; | |
| 1645 | 1688 | } else { |
| 1646 | 1689 | ctx->new_property_events = true; |
| 1647 | 1690 | } |
| @@ -1704,6 +1747,7 @@ static bool gen_property_change_event(struct mpv_handle *ctx) · 56 unmodified lines | ||
| 1704 | 1747 | prop->value_ret_ts != prop->value_ts) // other value than last time? |
| 1705 | 1748 | { |
| 1706 | 1749 | prop->value_ret_ts = prop->value_ts; |
| 1750 | + prop->waiting_for_hook = false; | |
| 1707 | 1751 | prop_unref(ctx->cur_property); |
| 1708 | 1752 | ctx->cur_property = prop; |
| 1709 | 1753 | prop->refcount += 1; |
| @@ -931,7 +931,8 @@ static void process_hooks(struct MPContext *mpctx, char *name) · 930 unmodified lines | ||
| 931 | 931 | while (!mp_hook_test_completion(mpctx, name)) { |
| 932 | 932 | mp_idle(mpctx); |
| 933 | 933 | |
| 934 | − // We have no idea what blocks a hook, so just do a full abort. | |
| 934 | + // We have no idea what blocks a hook, so just do a full abort. This | |
| 935 | + // does nothing for hooks that happen outside of playback. | |
| 935 | 936 | if (mpctx->stop_play) |
| 936 | 937 | mp_abort_playback_async(mpctx); |
| 937 | 938 | } |
| @@ -1374,13 +1375,17 @@ static void play_current_file(struct MPContext *mpctx) · 436 unmodified lines | ||
| 1374 | 1375 | double playback_start = -1e100; |
| 1375 | 1376 | |
| 1376 | 1377 | assert(mpctx->stop_play); |
| 1378 | + mpctx->stop_play = 0; | |
| 1379 | + | |
| 1380 | + process_hooks(mpctx, "on_before_start_file"); | |
| 1381 | + if (mpctx->stop_play) | |
| 1382 | + return; | |
| 1377 | 1383 | |
| 1378 | 1384 | mp_notify(mpctx, MPV_EVENT_START_FILE, NULL); |
| 1379 | 1385 | |
| 1380 | 1386 | mp_cancel_reset(mpctx->playback_abort); |
| 1381 | 1387 | |
| 1382 | 1388 | mpctx->error_playing = MPV_ERROR_LOADING_FAILED; |
| 1383 | − mpctx->stop_play = 0; | |
| 1384 | 1389 | mpctx->filename = NULL; |
| 1385 | 1390 | mpctx->shown_aframes = 0; |
| 1386 | 1391 | mpctx->shown_vframes = 0; |
| @@ -1701,6 +1706,8 @@ terminate_playback: · 314 unmodified lines | ||
| 1701 | 1706 | } |
| 1702 | 1707 | |
| 1703 | 1708 | assert(mpctx->stop_play); |
| 1709 | + | |
| 1710 | + process_hooks(mpctx, "on_after_end_file"); | |
| 1704 | 1711 | } |
| 1705 | 1712 | |
| 1706 | 1713 | // Determine the next file to play. Note that if this function returns non-NULL, |