summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwm4 <wm4@nowhere>2019-09-20 16:04:18 +0200
committerwm4 <wm4@nowhere>2019-09-20 16:04:18 +0200
commit3ae728532dbd8f4baa77a1e8ad74066719613e92 (patch)
tree4956f9fe50f188ef88df870bf59e2970ff8021f7
parentd12264acc03d1ef92b7c0d0c0245e1654747434e (diff)
downloadmpv-3ae728532dbd8f4baa77a1e8ad74066719613e92.tar.bz2
mpv-3ae728532dbd8f4baa77a1e8ad74066719613e92.tar.xz
client API: document unfortunate render API threading requirement
This is because dr_helper.c calls pthread_self(). It's used to avoid deadlocks. This was not a problem internal to mpv (dr_helper.c was first created for vo_gpu.c), but it accidentally leaked as an unintended consequence of an implementation detail to libmpv. This problem existed for MPV_RENDER_PARAM_ADVANCED_CONTROL users, ever since it was introduced. Maybe this could be done differently, but it's certainly very tricky. the pthread_t is used to avoid deadlocks when the caller is on the same thread as the one which needs to handle the calls. The critical code is in free_dr_buffer_on_dr_thread(). It's called when a DR image is free'd. Freeing a DR image requires access to the GL context, i.e. it just has to run on the "GL" thread. In libmpv's case, this is done by calling the API user's wakeup call, and the user will eventually call mpv_render_context_update() on the "GL" thread, which in turn calls mp_dispatch_queue_process() on the dispatch queue that was passed to dr_helper_create(), which then calls av_buffer_unref(), which calls gl_video_dr_free_buffer(). (God who came up with this rube goldberg shit.) The above case will typically happen when e.g. vd_lavc.c (internal mpv thread) frees images. Allocation works similarly; deallocation is just trickier because calls to free images are everywhere. Now consider if mpv_render_context_render() releases a DR image. This function is always called on the "GL" thread. Going through the dispatch queue would obviously deadlock, because according to the render API rules, the user's wakeup callback must not mpv_render_context_update(), instead it has to signal the "GL" thread, which must wait until mpv_render_context_render() returns. To avoid this deadlock, dr_helper.c checks whether the calling thread is the "GL" thread, and if so, allows a reentrant call (basically gl_video_dr_free_buffer() is called directly). While with GL, you usually will stay on the same thread, API users were in theory allowed to e.g. move the GL context to a different thread. But this dr_helper issue means this is not possible. Moreover, other APIs will not have the same thread-locality problems as GL. FUCK THIS SHIT In theory, libmpv could provide an API to "move" the context to a separate thread, but let's not start with even more broken crap like this. I'm not sure yet whether there is an easy solution. Maybe all mpv_render() function could be guarded by entry/exit functions, which set/unset a flag that dr_helper.c should use reentrant calls. libmpv users which do not set MPV_RENDER_PARAM_ADVANCED_CONTROL were never affected.
-rw-r--r--libmpv/render.h6
1 files changed, 6 insertions, 0 deletions
diff --git a/libmpv/render.h b/libmpv/render.h
index 14933674ca..086b42d1b5 100644
--- a/libmpv/render.h
+++ b/libmpv/render.h
@@ -79,6 +79,12 @@ extern "C" {
* is logged. If you set MPV_RENDER_PARAM_ADVANCED_CONTROL, you promise that
* this won't happen, and must absolutely guarantee it, or a real deadlock
* will freeze the mpv core thread forever.
+ * - if MPV_RENDER_PARAM_ADVANCED_CONTROL is used, you currently must call all
+ * mpv_render*() API functions from the same thread on which the
+ * mpv_render_context was returned by mpv_render_context_create(). This
+ * requirement always existed. Not honoring it will lead to UB (deadlocks,
+ * use of invalid pthread_t handles). This requirement might be removed in
+ * the future, but will require some considerable work internal to libmpv.
*
* libmpv functions which are safe to call from a render thread are:
* - functions marked with "Safe to be called from mpv render API threads."