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

[erwan.tulou at gmail.com]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20171213/c4533ed8/attachment.html>