From 7b8a30fc8132203bc93d35ac887682d2044ad5a9 Mon Sep 17 00:00:00 2001 From: Dudemanguy Date: Sun, 1 Oct 2023 01:19:40 -0500 Subject: [PATCH] vo_dmabuf_wayland: eliminate an extra frame copy When implementing vo_dmabuf_wayland, it always did a copy of the image from the current frame and worked with that. The reason was because mpv's core held onto the frame and caused some timing issues and rendering glitches depending on when it freed the image. This is pretty easy to fix: just make vo_dmabuf_wayland manage the the frames. In vo.h, we add a boolean that a VO can set to make them manage freeing frames directly. After doing this, change the buffers in vo_dmabuf_wayland to store the whole vo_frame instead of just the image. Then, just modify some things a bit so frame is freed instead of the image. Now, we should truly have zero-copy playback. Well as long as you don't use libass to render anything (that's still a copy from system memory). --- video/out/vo.c | 5 ++-- video/out/vo.h | 6 +++- video/out/vo_dmabuf_wayland.c | 54 ++++++++++++++++++----------------- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/video/out/vo.c b/video/out/vo.c index 84a900af5b..420b5b44c9 100644 --- a/video/out/vo.c +++ b/video/out/vo.c @@ -1026,7 +1026,8 @@ static bool render_frame(struct vo *vo) pthread_cond_broadcast(&in->wakeup); // for vo_wait_frame() done: - talloc_free(frame); + if (!vo->driver->frame_owner) + talloc_free(frame); if (in->wakeup_on_done && !still_displaying(vo)) { in->wakeup_on_done = false; wakeup_core(vo); @@ -1064,7 +1065,7 @@ static void do_redraw(struct vo *vo) vo->driver->draw_frame(vo, frame); vo->driver->flip_page(vo); - if (frame != &dummy) + if (frame != &dummy && !vo->driver->frame_owner) talloc_free(frame); } diff --git a/video/out/vo.h b/video/out/vo.h index e11c23a60a..a6c6a3ce6e 100644 --- a/video/out/vo.h +++ b/video/out/vo.h @@ -305,6 +305,9 @@ struct vo_driver { // Disable video timing, push frames as quickly as possible, never redraw. bool untimed; + // The VO is responsible for freeing frames. + bool frame_owner; + const char *name; const char *description; @@ -381,7 +384,8 @@ struct vo_driver { /* Render the given frame. Note that this is also called when repeating * or redrawing frames. * - * frame is freed by the caller, but the callee can still modify the + * frame is freed by the caller if the callee did not assume ownership + * of the frames, but in any case the callee can still modify the * contained data and references. */ void (*draw_frame)(struct vo *vo, struct vo_frame *frame); diff --git a/video/out/vo_dmabuf_wayland.c b/video/out/vo_dmabuf_wayland.c index 8290bad263..4ba58781ab 100644 --- a/video/out/vo_dmabuf_wayland.c +++ b/video/out/vo_dmabuf_wayland.c @@ -62,7 +62,7 @@ struct buffer { struct vo *vo; struct wl_buffer *buffer; struct wl_list link; - struct mp_image *image; + struct vo_frame *frame; uint32_t drm_format; uintptr_t id; @@ -111,9 +111,9 @@ struct priv { static void buffer_handle_release(void *data, struct wl_buffer *wl_buffer) { struct buffer *buf = data; - if (buf->image) { - mp_image_unrefp(&buf->image); - buf->image = NULL; + if (buf->frame) { + talloc_free(buf->frame); + buf->frame = NULL; } } @@ -296,7 +296,7 @@ static bool drm_format_check(struct vo *vo, struct mp_image *src) return false; } -static struct buffer *buffer_check(struct vo *vo, struct mp_image *src) +static struct buffer *buffer_check(struct vo *vo, struct vo_frame *frame) { struct priv *p = vo->priv; @@ -304,13 +304,13 @@ static struct buffer *buffer_check(struct vo *vo, struct mp_image *src) if (wl_list_length(&p->buffer_list) < WL_BUFFERS_WANTED) goto done; - uintptr_t id = surface_id(vo, src); + uintptr_t id = surface_id(vo, frame->current); struct buffer *buf; wl_list_for_each(buf, &p->buffer_list, link) { if (buf->id == id) { - if (buf->image) - mp_image_unrefp(&buf->image); - buf->image = src; + if (buf->frame) + talloc_free(buf->frame); + buf->frame = frame; return buf; } } @@ -319,34 +319,35 @@ done: return NULL; } -static struct buffer *buffer_create(struct vo *vo, struct mp_image *src) +static struct buffer *buffer_create(struct vo *vo, struct vo_frame *frame) { struct vo_wayland_state *wl = vo->wl; struct priv *p = vo->priv; struct buffer *buf = talloc_zero(vo, struct buffer); buf->vo = vo; - buf->image = src; + buf->frame = frame; + struct mp_image *image = buf->frame->current; struct zwp_linux_buffer_params_v1 *params = zwp_linux_dmabuf_v1_create_params(wl->dmabuf); switch(p->hwdec_type) { case HWDEC_VAAPI: - vaapi_dmabuf_importer(buf, src, params); + vaapi_dmabuf_importer(buf, image, params); break; case HWDEC_DRMPRIME: - drmprime_dmabuf_importer(buf, src, params); + drmprime_dmabuf_importer(buf, image, params); break; } if (!buf->drm_format) { - mp_image_unrefp(&buf->image); + talloc_free(buf->frame); talloc_free(buf); zwp_linux_buffer_params_v1_destroy(params); return NULL; } - buf->buffer = zwp_linux_buffer_params_v1_create_immed(params, src->params.w, src->params.h, + buf->buffer = zwp_linux_buffer_params_v1_create_immed(params, image->params.w, image->params.h, buf->drm_format, 0); zwp_linux_buffer_params_v1_destroy(params); wl_buffer_add_listener(buf->buffer, &buffer_listener, buf); @@ -354,14 +355,14 @@ static struct buffer *buffer_create(struct vo *vo, struct mp_image *src) return buf; } -static struct buffer *buffer_get(struct vo *vo, struct mp_image *src) +static struct buffer *buffer_get(struct vo *vo, struct vo_frame *frame) { /* Reuse existing buffer if possible. */ - struct buffer *buf = buffer_check(vo, src); + struct buffer *buf = buffer_check(vo, frame); if (buf) { return buf; } else { - return buffer_create(vo, src); + return buffer_create(vo, frame); } } @@ -372,9 +373,9 @@ static void destroy_buffers(struct vo *vo) p->destroy_buffers = false; wl_list_for_each_safe(buf, tmp, &p->buffer_list, link) { wl_list_remove(&buf->link); - if (buf->image) { - mp_image_unrefp(&buf->image); - buf->image = NULL; + if (buf->frame) { + talloc_free(buf->frame); + buf->frame = NULL; } if (buf->buffer) { wl_buffer_destroy(buf->buffer); @@ -604,13 +605,13 @@ static void draw_frame(struct vo *vo, struct vo_frame *frame) pts = frame->current ? frame->current->pts : 0; if (frame->current) { - struct mp_image *src = mp_image_new_ref(frame->current); - buf = buffer_get(vo, src); + buf = buffer_get(vo, frame); - if (buf && buf->image) { + if (buf && buf->frame) { + struct mp_image *image = buf->frame->current; wl_surface_attach(wl->video_surface, buf->buffer, 0, 0); - wl_surface_damage_buffer(wl->video_surface, 0, 0, buf->image->w, - buf->image->h); + wl_surface_damage_buffer(wl->video_surface, 0, 0, image->w, + image->h); } } @@ -851,6 +852,7 @@ const struct vo_driver video_out_dmabuf_wayland = { .description = "Wayland dmabuf video output", .name = "dmabuf-wayland", .caps = VO_CAP_ROTATE90, + .frame_owner = true, .preinit = preinit, .query_format = query_format, .reconfig2 = reconfig,