client API: fix potential deadlock problems by throwing more shit at it

The render API (vo_libmpv) had potential deadlock problems with
MPV_RENDER_PARAM_ADVANCED_CONTROL. This required vd-lavc-dr to be
enabled (the default). I never observed these deadlocks in the wild
(doesn't mean they didn't happen), although I could specifically provoke
them with some code changes.

The problem was mostly about DR (direct rendering, letting the video
decoder write to OpenGL buffer memory). Allocating/freeing a DR image
needs to be done on the OpenGL thread, even though _lots_ of threads are
involved with handling images. Freeing a DR image is a special case that
can happen any time. dr_helper.c does most of the evil magic of
achieving this. Unfortunately, there was a (sort of) circular lock
dependency: freeing an image while certain internal locks are held would
trigger the user's context update callback, which in turn would call
mpv_render_context_update(), which processed all pending free requests,
and then acquire an internal lock - which the caller might not release
until a further DR image could be freed.

"Solve" this by making freeing DR images asynchronous. This is slightly
risky, but actually not much. The DR images will be free'd eventually.
The biggest disadvantage is probably that debugging might get trickier.

Any solution to this problem will probably add images to free to some
sort of queue, and then process it later. I considered making this more
explicit (so there'd be a point where the caller forcibly waits for all
queued items to be free'd), but discarded these ideas as this probably
would only increase complexity.

Another consequence is that freeing DR images on the GL thread is not
synchronous anymore. Instead, it mpv_render_context_update() will do it
with a delay. This seems roundabout, but doesn't actually change
anything, and avoids additional code.

This also fixes that the render API required the render API user to
remain on the same thread, even though this wasn't documented. As such,
it was a bug. OpenGL essentially forces you to do all GL usage on a
single thread, but in theory the API user could for example move the GL
context to another thread.

The API bump is because I think you can't make enough noise about this.
Since we don't backport fixes to old versions, I'm specifically stating
that old versions are broken, and I'm supplying workarounds.

Internally, dr_helper_create() does not use pthread_self() anymore, thus
the vo.c change. I think it's better to make binding to the current
thread as explicit as possible.

Of course it's not sure that this fixes all deadlocks (probably not).

1 files changed, 12 insertions, 10 deletions

diff --git a/libmpv/render.h b/libmpv/render.h
index 6d594e0c06..afea7c2e10 100644
--- a/libmpv/render.h
+++ b/libmpv/render.h
@@ -79,16 +79,6 @@ 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.
- *    You can avoid this issue by setting "vd-lavc-dr" to "no".
- *  - MPV_RENDER_PARAM_ADVANCED_CONTROL has some other libmpv-internal problems,
- *    which may result in random deadlocks (see top of vo_libmpv.c).
- *    You can probably avoid this issue by setting "vd-lavc-dr" to "no".
  *
  * libmpv functions which are safe to call from a render thread are:
  *  - functions marked with "Safe to be called from mpv render API threads."
@@ -104,6 +94,18 @@ extern "C" {
  *  - if the mpv_handle parameter refers to a different mpv core than the one
  *    you're rendering for (very obscure, but allowed)
  *
+ * Note about old libmpv version:
+ *
+ *      Before API version 1.105 (basically in mpv 0.29.x), simply enabling
+ *      MPV_RENDER_PARAM_ADVANCED_CONTROL could cause deadlock issues. This can
+ *      be worked around by setting the "vd-lavc-dr" option to "no".
+ *      In addition, you were required to call all mpv_render*() API functions
+ *      from the same thread on which mpv_render_context_create() was originally
+ *      run (for the same the mpv_render_context). Not honoring it led to UB
+ *      (deadlocks, use of invalid pthread_t handles), even if you moved your GL
+ *      context to a different thread correctly.
+ *      These problems were addressed in API version 1.105 (mpv 0.30.0).
+ *
  * Context and handle lifecycle
  * ----------------------------
  *