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