[Bug 709747] [wayland_egl] Add support for GstVideoOverlay

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=709747
  GStreamer | gst-plugins-gl | git

--- Comment #4 from Sebastian Dröge (slomo) <slomo at circular-chaos.org> 2013-10-10 12:41:30 UTC ---
(In reply to comment #3)
> (In reply to comment #2)
> > Review of attachment 256826 [details] [details]:
> > 
> > Does all this still work without setting a window handle/display, and then
> > creates a new window?
> 
> For wayland_egl, and about the display: yes, if you go back to ready state
> first. No if you do that in paused or playing.
> For wayland_egl surface, yes in paused or playing state too.

Sounds good :)

> > 
> > ::: configure.ac
> > @@ +878,3 @@
> > +  else
> > +    PKG_CHECK_MODULES(GTK2, gtk+-2.0, HAVE_GTK_20=yes, HAVE_GTK_20=no)
> > +    if test "x$HAVE_GTK_20" = "xyes"; then
> > 
> > Just remove GTK2 support, nobody needs that :)
> 
> WHy not keep it as it's done in my patch, then in a near future remove GTK2
> support. No ?
> Also it could be interesting for comparisons.

Or remove it later, just makes the configure check a bit more complicated for
no good reason ;)

> > ::: gst-libs/gst/gl/gstgldisplay.h
> > @@ +68,2 @@
> >  GstGLAPI       gst_gl_display_get_gl_api             (GstGLDisplay * display);
> > +gpointer       gst_gl_display_get_platform_display   (GstGLDisplay * display);
> > 
> > Would this be an EGLDisplay with EGL, a X11 display connection with X11, etc.?
> 
> With wayland_egl, it's a wl_display, not a EGLDisplay. EGLDisplay is created
> using gst_gl_window_get_display which return this wl_display too.
> Later one is available when the window is already "open".
> 
> Note sure about glx/X11 and egl/X11 display, because in gst-gl 0.10 it was not
> necessary  that glimagesink and the application share the same instance of X11
> Display. (same with win32, wgl, egl)
> 
> Maybe it's only related to wayland.

So for Wayland there is a EGLDisplay and a separate wl_display? Ugly :/ I think
this should somehow be made more clear then in the API, and more strict
distinction between the EGLDisplay-like things and the wl_display.

> > Make it's usage consistent :) I think that would make sense in general, and
> > then also connected with GstContext usage
> > 
> > ::: gst-libs/gst/gl/wayland/wayland_event_source.c
> > @@ +113,2 @@
> >    if (source->pfd.revents) {
> > +    gst_gl_wl_display_roundtrip_queue (source->display, source->queue);
> > 
> > Why not the Wayland function?
> 
> Interesting point, initially I replaced the wl_display_dispatch (which is there
> in master) by wl_display_dispatch_queue. But that's not enough when a surface
> and its child does not share the same queue.
> I found this morning this discussion
> https://bugs.launchpad.net/glmark2/+bug/1211076 . The comments have also an
> interest. It's not the exact same problem but it seems that the common point is
> that there is a difference between events and requests.
> 
> So I needed something like wl_display_roundtrip but for just a queue, and
> wl_display_roundtrip_queue does not exist. So I created
> gst_gl_wl_display_roundtrip_queue. See wl_display_roundtrip in
> http://cgit.freedesktop.org/wayland/wayland/tree/src/wayland-client.c

Makes sense, maybe add a comment with all this explanation to the code :)

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.