mirror of
https://github.com/mpv-player/mpv.git
synced 2024-09-20 20:03:10 +02:00
4d43c79e4c
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).
38 lines
1.9 KiB
C
38 lines
1.9 KiB
C
#pragma once
|
|
|
|
// This is a helper for implementing thread-safety for DR callbacks. These need
|
|
// to allocate GPU buffers on the GPU thread (e.g. OpenGL with its forced TLS),
|
|
// and the buffers also need to be freed on the GPU thread.
|
|
// This is not a helpful "Dr.", rather it represents Satan in form of C code.
|
|
struct dr_helper;
|
|
|
|
struct mp_image;
|
|
struct mp_dispatch_queue;
|
|
|
|
// dr_helper_get_image() calls will use the dispatch queue to run get_image on
|
|
// a target thread, which processes the dispatch queue.
|
|
// Note: the dispatch queue must process outstanding async. work before the
|
|
// dr_helper instance can be destroyed.
|
|
struct dr_helper *dr_helper_create(struct mp_dispatch_queue *dispatch,
|
|
struct mp_image *(*get_image)(void *ctx, int imgfmt, int w, int h,
|
|
int stride_align),
|
|
void *get_image_ctx);
|
|
|
|
// Make DR release calls (freeing images) reentrant if they are called on this
|
|
// (pthread_self()) thread. That means any free call will directly release the
|
|
// image as allocated with get_image().
|
|
// Only 1 thread can use this at a time. Note that it would make no sense to
|
|
// call this on more than 1 thread, as get_image is assumed not thread-safe.
|
|
void dr_helper_acquire_thread(struct dr_helper *dr);
|
|
|
|
// This _must_ be called on the same thread as dr_helper_acquire_thread() was
|
|
// called. Every release call must be paired with an acquire call.
|
|
void dr_helper_release_thread(struct dr_helper *dr);
|
|
|
|
// Allocate an image by running the get_image callback on the target thread.
|
|
// Always blocks on dispatch queue processing. This implies there is no way to
|
|
// allocate a DR'ed image on the render thread (at least not in a way which
|
|
// actually works if you want foreign threads to be able to free them).
|
|
struct mp_image *dr_helper_get_image(struct dr_helper *dr, int imgfmt,
|
|
int w, int h, int stride_align);
|