[RFC] vo_libmpv/gpu-next: add Vulkan render API - #18258
Conversation
Add Vulkan image targets to the gpu-next libmpv rendering API. Support externally owned Vulkan images, caller-provided wait and signal semaphores, target layout transitions, and asynchronous GPU-to-GPU synchronization suitable for Wayland swapchain presentation. This implementation is based on Andrew Rabert's experimental Vulkan libmpv backend and has been forward-ported and extended for mpvpaper.
e2262b0 to
fd96c86
Compare
| @@ -0,0 +1,213 @@ | |||
| /* Copyright (C) 2018 the mpv developers | |||
There was a problem hiding this comment.
2018? oh this is just gonna be straight up slop isn't it
| */ | ||
|
|
||
| #include <libplacebo/config.h> | ||
| #include <libplacebo/config.h> // for PL_HAVE_OPENGL, PL_API_VER |
| #include "mpv/render_gl.h" // for mpv_opengl_init_params | ||
| #include <libplacebo/opengl.h> // for pl_opengl_destroy | ||
| #include "video/out/gpu_next/libmpv_gpu_next.h" // for libmpv_gpu_next_context | ||
| #include "video/out/gpu_next/ra.h" // for ra_pl_create, ra_pl_... |
There was a problem hiding this comment.
stop with the comments, god
| #if HAVE_GL | ||
| // Store Libplacebo OpenGL context information. | ||
| struct priv { | ||
| pl_log pl_log; | ||
| pl_opengl gl; | ||
| pl_gpu gpu; | ||
| struct ra_next *ra; | ||
|
|
||
| // Store a persistent copy of the init params to avoid a dangling pointer. | ||
| mpv_opengl_init_params gl_params; | ||
| }; |
There was a problem hiding this comment.
random opengl changes, sure, whatever, fuck me
| // Setup libplacebo logging | ||
| struct pl_log_params log_params = { | ||
| .log_level = PL_LOG_DEBUG | ||
| }; | ||
|
|
||
| // Enable verbose logging if trace is enabled | ||
| if (mp_msg_test(ctx->log, MSGL_TRACE)) { | ||
| log_params.log_cb = pl_log_cb_vk; | ||
| log_params.log_priv = ctx->log; | ||
| } | ||
|
|
||
| p->pl_log = pl_log_create(PL_API_VER, &log_params); | ||
| if (!p->pl_log) { | ||
| MP_ERR(ctx, "Failed to create libplacebo log\n"); | ||
| return MPV_ERROR_GENERIC; | ||
| } |
There was a problem hiding this comment.
yeah better duplicate all of this
| * | ||
| * Required device extensions (user must enable these): | ||
| * - VK_KHR_timeline_semaphore (core in 1.2+) | ||
| * - VK_KHR_external_memory (for hwdec interop) |
| * - User must enable required extensions when creating the device (see below) | ||
| * | ||
| * Required device extensions (user must enable these): | ||
| * - VK_KHR_timeline_semaphore (core in 1.2+) |
There was a problem hiding this comment.
Why is this required if this is core in 1.3+?
| * Recommended device extensions: | ||
| * - VK_KHR_video_decode_queue (for Vulkan video decode hwdec) | ||
| * - VK_KHR_video_decode_h264 | ||
| * - VK_KHR_video_decode_h265 |
There was a problem hiding this comment.
Only h264 and h265 is supported?
| .get_proc_addr = get_proc, | ||
| .queue_graphics = { | ||
| .index = vk_params->graphics_queue_family, | ||
| .count = 1, |
There was a problem hiding this comment.
so vulkan-queue-count option is just not used, ok cool
| * ----------------- | ||
| * | ||
| * For Vulkan hwdec to work, the user must: | ||
| * - Provide MPV_RENDER_PARAM_WL_DISPLAY (Wayland) for dmabuf import |
There was a problem hiding this comment.
Why? What's wrong with existing primitives for importing vaapi or drm images?
| * | ||
| * For Vulkan hwdec to work, the user must: | ||
| * - Provide MPV_RENDER_PARAM_WL_DISPLAY (Wayland) for dmabuf import | ||
| * - Enable the appropriate video decode extensions on the device |
There was a problem hiding this comment.
I think we should export recommended and required extentions as static arrays, same as libplacebo does.
| * The device MUST have been created with at least the features required | ||
| * by libplacebo, including: | ||
| * - hostQueryReset (Vulkan 1.2 / VK_EXT_host_query_reset) | ||
| * - timelineSemaphore (Vulkan 1.2 / VK_KHR_timeline_semaphore) |
There was a problem hiding this comment.
Don't we support only vulkan 1.3?
| if (p->ra) { | ||
| ra_pl_destroy(&p->ra); | ||
| } | ||
|
|
||
| if (p->frame_tex) | ||
| pl_tex_destroy(p->gpu, &p->frame_tex); | ||
| if (p->completion) | ||
| vkDestroySemaphore(p->vulkan->device, p->completion, NULL); | ||
| if (p->vulkan) { | ||
| pl_vulkan_destroy(&p->vulkan); | ||
| } |
There was a problem hiding this comment.
sure just use braces however you want
| // context current inside its get_proc_address callback. We can trigger | ||
| // this by calling it with a harmless, common function name. | ||
| if (gl_params && gl_params->get_proc_address) { | ||
| gl_params->get_proc_address(gl_params->get_proc_address_ctx, "glGetString"); |
There was a problem hiding this comment.
Could you explain why this is needed?
| static void pl_log_cb(void *log_priv, enum pl_log_level level, const char *msg) | ||
| { | ||
| struct mp_log *log = log_priv; | ||
| mp_msg(log, MSGL_WARN, "[gpu-next:pl] %s\n", msg); |
There was a problem hiding this comment.
You don't have to use manual prefixes, create child logger.
| static void pl_log_cb_vk(void *log_priv, enum pl_log_level level, const char *msg) | ||
| { | ||
| struct mp_log *log = log_priv; | ||
| mp_msg(log, MSGL_WARN, "[gpu-next:pl] %s\n", msg); |
There was a problem hiding this comment.
This looks very similar to previous code.
| #pragma once | ||
|
|
||
| #include <libplacebo/renderer.h> | ||
| #include "libplacebo/gpu.h" // for pl_gpu |
There was a problem hiding this comment.
I think <> is proper way for includes.
| #include "libplacebo/gpu.h" // for pl_gpu | ||
| #include "libplacebo/log.h" // for pl_log | ||
| #include "libplacebo/swapchain.h" // for pl_swapchain | ||
| #include "stdbool.h" // for bool |
There was a problem hiding this comment.
<> and put includes at the top
| @@ -0,0 +1,314 @@ | |||
| #include "libmpv_gpu_next.h" | |||
| static struct mp_image *get_image(struct render_backend *ctx, int imgfmt, | ||
| int w, int h, int stride_align, int flags) | ||
| { | ||
| return NULL; |
There was a problem hiding this comment.
What if user wants to get image?
| } | ||
|
|
||
| // Create hardware decoder devices. | ||
| ctx->hwdec_devs = hwdec_devices_create(); |
There was a problem hiding this comment.
I guess ra_hwdec_ctx_init is not cool enough to be called
| */ | ||
| static void perfdata(struct render_backend *ctx, | ||
| struct voctrl_performance_data *out) | ||
| { |
There was a problem hiding this comment.
Could you explain how this implementation collects performance data?
| @@ -0,0 +1,409 @@ | |||
| #include "video/out/gpu_next/ra.h" | |||
| ra_queue ra_next_queue_create(struct ra_next *ra) | ||
| { | ||
| return pl_queue_create(ra->gpu); | ||
| } | ||
|
|
||
| void ra_next_queue_destroy(ra_queue *queue) | ||
| { | ||
| pl_queue_destroy(queue); | ||
| } | ||
|
|
||
| void ra_next_queue_push(ra_queue queue, const struct pl_source_frame *frame) | ||
| { | ||
| pl_queue_push(queue, frame); | ||
| } | ||
|
|
||
| void ra_next_queue_update(ra_queue queue, struct pl_frame_mix *mix, const struct pl_queue_params *params) | ||
| { | ||
| pl_queue_update(queue, mix, params); | ||
| } | ||
|
|
||
| void ra_next_queue_reset(ra_queue queue) | ||
| { | ||
| pl_queue_reset(queue); | ||
| } | ||
|
|
||
| pl_tex ra_next_tex_create(struct ra_next *ra, const struct pl_tex_params *params) | ||
| { | ||
| return pl_tex_create(ra->gpu, params); | ||
| } | ||
|
|
||
| void ra_next_tex_destroy(struct ra_next *ra, pl_tex *tex) | ||
| { | ||
| pl_tex_destroy(ra->gpu, tex); | ||
| } | ||
|
|
||
| bool ra_next_tex_recreate(struct ra_next *ra, pl_tex *tex, const struct pl_tex_params *params) | ||
| { | ||
| return pl_tex_recreate(ra->gpu, tex, params); | ||
| } |
There was a problem hiding this comment.
very useful, thank you
| /* Cleanup any textures/resources attached to a pl_frame created by upload. */ | ||
| void ra_cleanup_pl_frame(struct ra_next *ra, struct pl_frame *frame); | ||
|
|
||
| /* --- New Texture Management Wrappers (prefixed to avoid conflicts) --- */ |
| @@ -0,0 +1,63 @@ | |||
| #pragma once | |||
|
|
|||
| #include "stdbool.h" // for bool | |||
There was a problem hiding this comment.
could've never guessed
| char *backend_name = get_mpv_render_param(params, MPV_RENDER_PARAM_BACKEND, ""); | ||
| const struct render_backend_fns **render_backends; | ||
|
|
||
| if (backend_name && strcmp(backend_name, "gpu-next") == 0) { | ||
| MP_VERBOSE(ctx, "Using gpu-next backend.\n"); | ||
| static const struct render_backend_fns* backends[] = { | ||
| &render_backend_gpu_next, | ||
| &render_backend_sw, | ||
| NULL | ||
| }; | ||
| render_backends = backends; | ||
| } else { | ||
| MP_VERBOSE(ctx, "Using default gpu backend.\n"); | ||
| static const struct render_backend_fns* backends[] = { | ||
| &render_backend_gpu, | ||
| &render_backend_sw, | ||
| NULL | ||
| }; | ||
| render_backends = backends; | ||
| } | ||
|
|
There was a problem hiding this comment.
???????????????????????????
| */ | ||
| bool pl_video_check_format(struct pl_video *p, int imgfmt) { | ||
| // For simplicity, we assume libplacebo can handle it. | ||
| // A more robust implementation might query libplacebo's capabilities. |
There was a problem hiding this comment.
I think we need to be robust unfortunately.
Then you should preserve history and commits of prior work. Also spliting work into smaller commits will be easier to review. Additionally, some things should be tested in isolation. For example we probably should first port the vo_gpu_next. to ra primitives, validate this and then expose this in libmpv api. Additionally it would be nice if you could implement the meson test for libmpv rendering api or a standalone app that can be used at least for build test. |
Uh in that case don't strip authorship and make one big commit that pretends you did all of it? Not really salvageable. I'll just close. |

Summary
This RFC adds a Vulkan backend to the libmpv render API, allowing embedding applications to render directly into externally owned
VkImagetargets.The implementation is based on Andrew Rabert's experimental Vulkan libmpv work, forward-ported to current mpv master. It also adds client-provided wait and signal semaphores so rendering and presentation can remain GPU-synchronized instead of blocking the CPU after every frame.
Existing OpenGL and software render paths remain unchanged.
Motivation
This was developed for mpvpaper, which owns its Wayland surfaces and presentation loop.
Standalone mpv can select Vulkan through normal GPU options, but those options cannot replace the render API and target supplied by an embedding application. A Vulkan-capable client needs to provide its own instance, device, queue, image target, layouts and synchronization objects.
The immediate use case is rendering through a selected GPU in a multi-GPU Wayland setup.
API
The RFC introduces a Vulkan render API and public
render_vk.hheader. The client supplies:VkImageandVkImageViewThe client retains ownership of all Vulkan resources.
The backend imports the device into libplacebo, renders into the supplied image and transitions it to the requested final layout. With a client signal semaphore, the image can be passed directly to
vkQueuePresentKHR()without a per-frame CPU wait.Origin
The initial backend is based on work from the
andrewrabert/mpvfork. This PR forward-ports it to:The synchronization, final-layout handling and client integration were adjusted while testing it with mpvpaper. I am not claiming sole authorship of the original backend.
Testing
Built successfully with GCC 16.1.1, Meson 1.11.1, libplacebo 7.360.1 and Vulkan 1.4.350.
A patched mpvpaper was built against the resulting
libmpv.so.2.5.0and tested on Wayland/Hyprland with an RTX 4070 driving the compositor and a GTX 1660 SUPER selected for rendering.4K H.264 videos played and looped correctly using Vulkan hardware decoding.
nvidia-smiconfirmed that mpvpaper was running on the selected GPU.Status
This is an RFC because the public API, ownership model and semaphore interface need upstream review. The corresponding
client-api-changes.rstentry can be added once the API shape is agreed upon.AI assistance disclosure
AI was used to prepare parts of the patch, comments in the code and description.