[vlc-devel] [PATCH] fix some Wayland key issues when playing out videos

[Rémi Denis-Courmont]

Le 13 décembre 2017 02:39:17 GMT+02:00, "erwan.tulou at gmail.com" <erwan.tulou at gmail.com> a écrit :
>Hello,
>
>On 11/12/2017 18:19, Rémi Denis-Courmont wrote:
>> 	Hello,
>>
>> The second patch does not look correct at all to me. I don't see why
>we should
>> assume that the window expands solely right and down. glViewport()
>manages the
>> mapping of OpenGL coordinates to window coordinates.
>The (x,y) parameters required in wl_egl_window_resize() are those used 
>in wl_surface_attach(), and they are a bit misleading
>
>Doc says : "The x and y arguments specify the location of the new 
>pending buffer's upper left corner, relative to the current buffer's 
>upper left corner, in surface-local coordinates. In other words, the x 
>and y, combined with the new surface size define in which directions
>the 
>surface's size changes."
>
>Reality is : it should read : "In other words, the x and y, combined 
>with the new surface size define in which directions _the surface
>moves_."
>
>Actually, when you provide the (x,y) parameters via 
>wl_egl_window_resize(), you're requesting the EGL library to move the 
>original display by (x,y) during their next wl_surface_attach(). This
>is 
>wrong as the original display belongs to the qt interface with its 
>internal placement strategy and must not be tampered with. The EGL 
>library also needs the size of the original display to render the right
>
>buffer size.  Unlike X, Wayland doesn't offer a means to query/to 
>register the server to get updated about the size of an existing
>window. 
>This is the reason why wl_egl_window_resize() is needed for Wayland but
>
>not for X.
>
>Please, just give a try to the patches. You'll see that without the 
>patches, the video widget moves around a lot (because of these (x,y) 
>parameters). With the patch, everything stays in place and is fully OK 
>(resize, crop, aspect ratio, ...). You even benefit from a clean video 
>while resizing, which was impossible in Opengl with an X server. A
>first 
>benefit that was a promise of Wayland !
>
>As a side note, this misuse of the (x,y) parameter also exists in 
>wl_surface_attach()  in the wl vout display. You cannot use them to 
>place the video inside a given window. With the qt interface, you just 
>end up moving the video widget outside the vlc interface where it 
>belongs. But a fix for the wl vout display is more complicated to do. I
>
>guess the only way to solve it is via an additional subsurface, that
>you 
>can position wrt to the original display. But Wayland is more basic
>(too 
>basic?) than X with these things.
>
>>
>> And the third patch looks like a kludge rather than a fix, TBH.
>I do agree. This patch was added just in case somebody wanted to test 
>the result of the other two patches right away.
>
>Rgds
>
>
>
>
>
>---
>L'absence de virus dans ce courrier électronique a été vérifiée par le
>logiciel antivirus Avast.
>https://www.avast.com/antivirus

Hi,

I know what the coordinates mean. They are currently working consistently between two vouts and the reference implementation of a vout window (naked Kwin backend).

Of course the scheme does not work in all scenarii. Neither does your proposed scheme, AFAIK. Wayland will not work properly in 3.0 anyway. J-B postponed the Wayland embedding ticket before I(or anyone else) could implement it. But that is not the only fatal issue - input handling comes to mind. Not to speak of "non-fatal" issues such as screen capture  and skin engine. At this point, we should revert to giving X11 higher priority inside Qt and make it clear that Wayland is unfinished and experimental in NEWS.

As for 4.0, I don't think that your change works. It solved some cases but breaks others and breaks internal consistency. In all likelihood, the interface between render and window needs to change a bit. The whole point of not adding LibVLC support is that we can still change it...
-- 
Remi Denis-Courmont