[vlc-devel] [PATCH 1/2] vout: add a resize acknowledgement callback

Rémi Denis-Courmont remi at remlab.net
Thu Feb 11 07:54:32 UTC 2021


Hi,

I'm calling on the TC to approve this patch. It was approved by the only other Wayland dev (Alexandre) and objections by Steve make no sense, aside from documentation (which is a preexisting problem of the code).

I obviously vote for the patch.

Le 9 février 2021 19:04:42 GMT+02:00, remi at remlab.net a écrit :
>From: Rémi Denis-Courmont <remi at remlab.net>
>
>For perfect rendering synchronisation, the change of window size must
>be acknowledged after the last picture is rendered in the old size,
>and before the first picture is rendered in the new size.
>
>As of c9d6d95f291b1e2c0e4f374f87790af091282c1e, the window resize is
>processed synchronously. This provides for the "after" requirement. But
>it failed to address the "before" requirement: the video output thread
>can end render a picture so soon after the resize so the window
>callback thread has not had time to perform acknowledgement procedures.
>
>This adds a callback to the window resize event which is called before
>the display lock (or equivalent) is released. This is guaranteed to be
>invoked before the video output thread has the opportunity to render
>anything.
>
>Other options were considered:
>
>1) Invoking a callback to the window provider from the video output
>   thread code code.
>   -> This can delay the callback unreasonably.
>   -> Taking the window lock on the video output thread is undesirable
>      if at all possible.
>
>2) Invoking a windowing system type-specific callback to the window
>   provider from every display of that windowing system.
>   -> Same delay and locking problems as 1.
>   -> Requires patching several display plugins.
>
>3) Exposing the display lock to the window plugin, via new callbacks.
>   -> This leaks internal implementation of the core to window
>      providers.
>   -> This gets really confusing as some callbacks would need explicit
>      locking and others not.
>
>In comparison, this patch seems like the least of evils.
>
>The callback is passed a parameter rather than as a property of the
>window to distinguish it from the existing window callbacks which are
>not reentrant.
>
>Refs #25112.
>---
> include/vlc_vout_window.h        |  7 +++++--
> modules/video_output/splitter.c  |  6 +++++-
> src/video_output/opengl.c        |  6 +++++-
> src/video_output/video_output.c  |  6 +++++-
> src/video_output/vout_internal.h |  3 ++-
> src/video_output/window.c        | 21 +++++++++++++++++++--
> 6 files changed, 41 insertions(+), 8 deletions(-)
>
>diff --git a/include/vlc_vout_window.h b/include/vlc_vout_window.h
>index ea16d4b414..7d0db591e5 100644
>--- a/include/vlc_vout_window.h
>+++ b/include/vlc_vout_window.h
>@@ -173,6 +173,8 @@ typedef struct vout_window_cfg_t {
> 
> } vout_window_cfg_t;
> 
>+typedef void (*vout_window_ack_cb)(struct vout_window_t *, void *);
>+
> /**
>  * Window event callbacks structure.
>  *
>@@ -200,7 +202,8 @@ struct vout_window_callbacks {
>* of external events from the windowing system, or deferred processing
>of
>      * a size change request.
>      */
>-    void (*resized)(struct vout_window_t *, unsigned width, unsigned
>height);
>+    void (*resized)(struct vout_window_t *, unsigned width, unsigned
>height,
>+                    vout_window_ack_cb cb, void *opaque);
> 
>     /**
>      * Callback for window closing.
>@@ -543,7 +546,7 @@ void vout_window_Disable(vout_window_t *window);
> static inline void vout_window_ReportSize(vout_window_t *window,
>                                       unsigned width, unsigned height)
> {
>-    window->owner.cbs->resized(window, width, height);
>+    window->owner.cbs->resized(window, width, height, NULL, NULL);
> }
> 
> /**
>diff --git a/modules/video_output/splitter.c
>b/modules/video_output/splitter.c
>index 6ca82e34c1..9a2a1544e7 100644
>--- a/modules/video_output/splitter.c
>+++ b/modules/video_output/splitter.c
>@@ -135,7 +135,8 @@ static void vlc_vidsplit_Close(vout_display_t *vd)
> }
> 
> static void vlc_vidsplit_window_Resized(vout_window_t *wnd,
>-                                        unsigned width, unsigned
>height)
>+                                        unsigned width, unsigned
>height,
>+                                        vout_window_ack_cb cb, void
>*opaque)
> {
>     struct vlc_vidsplit_part *part = wnd->owner.sys;
> 
>@@ -145,6 +146,9 @@ static void
>vlc_vidsplit_window_Resized(vout_window_t *wnd,
> 
>     if (part->display != NULL)
>         vout_display_SetSize(part->display, width, height);
>+
>+    if (cb != NULL)
>+        cb(wnd, opaque);
>     vlc_sem_post(&part->lock);
> }
> 
>diff --git a/src/video_output/opengl.c b/src/video_output/opengl.c
>index 8355c16135..bb10ca6302 100644
>--- a/src/video_output/opengl.c
>+++ b/src/video_output/opengl.c
>@@ -124,7 +124,8 @@ typedef struct vlc_gl_surface
> } vlc_gl_surface_t;
> 
> static void vlc_gl_surface_ResizeNotify(vout_window_t *surface,
>-                                        unsigned width, unsigned
>height)
>+                                        unsigned width, unsigned
>height,
>+                                        vout_window_ack_cb cb, void
>*opaque)
> {
>     vlc_gl_surface_t *sys = surface->owner.sys;
> 
>@@ -133,6 +134,9 @@ static void
>vlc_gl_surface_ResizeNotify(vout_window_t *surface,
>     vlc_mutex_lock(&sys->lock);
>     sys->width = width;
>     sys->height = height;
>+
>+    if (cb != NULL)
>+        cb(surface, opaque);
>     vlc_mutex_unlock(&sys->lock);
> }
> 
>diff --git a/src/video_output/video_output.c
>b/src/video_output/video_output.c
>index d04d3f4d11..fb7f54e385 100644
>--- a/src/video_output/video_output.c
>+++ b/src/video_output/video_output.c
>@@ -543,7 +543,8 @@ void vout_ChangeWindowState(vout_thread_t *vout,
>unsigned st)
> }
> 
> void vout_ChangeDisplaySize(vout_thread_t *vout,
>-                            unsigned width, unsigned height)
>+                            unsigned width, unsigned height,
>+                            void (*cb)(void *), void *opaque)
> {
>     vout_thread_sys_t *sys = VOUT_THREAD_TO_SYS(vout);
> 
>@@ -557,6 +558,9 @@ void vout_ChangeDisplaySize(vout_thread_t *vout,
> 
>     if (sys->display != NULL)
>         vout_display_SetSize(sys->display, width, height);
>+
>+    if (cb != NULL)
>+        cb(opaque);
>     vlc_mutex_unlock(&sys->display_lock);
> }
> 
>diff --git a/src/video_output/vout_internal.h
>b/src/video_output/vout_internal.h
>index fb24b51ee8..95274f2409 100644
>--- a/src/video_output/vout_internal.h
>+++ b/src/video_output/vout_internal.h
>@@ -150,7 +150,8 @@ bool GetAspectRatio(const char *ar_str, unsigned
>*num, unsigned *den);
> void vout_ChangeFullscreen(vout_thread_t *, const char *id);
> void vout_ChangeWindowed(vout_thread_t *);
> void vout_ChangeWindowState(vout_thread_t *, unsigned state);
>-void vout_ChangeDisplaySize(vout_thread_t *, unsigned width, unsigned
>height);
>+void vout_ChangeDisplaySize(vout_thread_t *, unsigned width, unsigned
>height,
>+                            void (*ack_cb)(void *), void *opaque);
> void vout_ChangeDisplayFilled(vout_thread_t *, bool is_filled);
> void vout_ChangeZoom(vout_thread_t *, unsigned num, unsigned den);
>void vout_ChangeDisplayAspectRatio(vout_thread_t *, unsigned num,
>unsigned den);
>diff --git a/src/video_output/window.c b/src/video_output/window.c
>index 9c2df5c1e6..c866fe7af4 100644
>--- a/src/video_output/window.c
>+++ b/src/video_output/window.c
>@@ -198,6 +198,20 @@ void vout_window_ReportFullscreen(vout_window_t
>*window, const char *id)
>         window->owner.cbs->fullscreened(window, id);
> }
> 
>+struct vout_window_ack_data {
>+    vout_window_t *window;
>+    vout_window_ack_cb callback;
>+    void *opaque;
>+};
>+
>+static void vout_window_Ack(void *data)
>+{
>+    struct vout_window_ack_data *cb_data = data;
>+
>+    if (cb_data->callback != NULL)
>+        cb_data->callback(cb_data->window, cb_data->opaque);
>+}
>+
> /* Video output display integration */
> #include <vlc_vout.h>
> #include <vlc_vout_display.h>
>@@ -214,13 +228,16 @@ typedef struct vout_display_window
> } vout_display_window_t;
> 
> static void vout_display_window_ResizeNotify(vout_window_t *window,
>-                                             unsigned width, unsigned
>height)
>+                                             unsigned width, unsigned
>height,
>+                                             vout_window_ack_cb cb,
>+                                             void *opaque)
> {
>     vout_display_window_t *state = window->owner.sys;
>     vout_thread_t *vout = state->vout;
>+    struct vout_window_ack_data data = { window, cb, opaque };
> 
>     msg_Dbg(window, "resized to %ux%u", width, height);
>-    vout_ChangeDisplaySize(vout, width, height);
>+    vout_ChangeDisplaySize(vout, width, height, vout_window_Ack,
>&data);
> }
> 
> static void vout_display_window_CloseNotify(vout_window_t *window)
>-- 
>2.30.0
>
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20210211/1229beec/attachment.html>


More information about the vlc-devel mailing list