GNOME Bugzilla – Bug 746632
dispmanx: surfaceless EGL context support broken
Last modified: 2015-03-24 11:15:37 UTC
If the glimagesink delegates surfaces rendering to the application (via the client-draw signal) the EGL context needs to create a PBuffer surface instead of a Window surface. This is currently not working because the dispmanx glwindow unconditionally creates a native window.
Created attachment 300117 [details] [review] gl/dispmanx: surfaceless EGL context support Show the DispmanX window only if there's no shared external GL context set up. The native window management is now split between the window_show and window_resize functions.
Review of attachment 300117 [details] [review]: Looks almost good (some empty newlines too many in window_resize) ::: gst-libs/gst/gl/egl/gstglcontext_egl.c @@ +399,1 @@ window_handle = Why only if there is no external context? I assume you could have an external context but still want to show the window ;)
(In reply to Sebastian Dröge (slomo) from comment #2) > Review of attachment 300117 [details] [review] [review]: > > Looks almost good (some empty newlines too many in window_resize) > > ::: gst-libs/gst/gl/egl/gstglcontext_egl.c > @@ +399,1 @@ > window_handle = > > Why only if there is no external context? I assume you could have an > external context but still want to show the window ;) Hum ok, so I need a different condition then. Basically I don't want to show the window if glimagesink has a signal handler connected to the client-draw signal, but that's outside of the context scope. Any suggestions? :) Thanks for the review, I'll remove the empty lines.
Not sure, do you provide a custom window? Or maybe the context needs some kind of no-window setting.
Well in my case it's the WebKit wayland compositor that manages the DispmanX window. So yeah perhaps some new API for the context would be the way to go. I can file a new bug for this if needed.
After discussion on IRC the plan is to follow this: http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/gst-libs/gst/gl/egl/gstglcontext_egl.c#n383 I.e if other_context is NULL then we create the window. Otherwise we assume it is offscreen. (All offscreen follows createContext(other not null), onscreen is the most downstream or from app created like this: createContext(other is null)) So please in gst_gl_window_dispmanx_egl add the function: "gst_gl_window_dispmanx_egl_create_window" as it is done x11 and wind32. In this function you create the dispmanx element unvisible (I hope the unvisible is possible) Invisible means that get_window_handle still returns a valide handle. Then gst_gl_window_dispmanx_egl_draw does not make it visible. Only gst_gl_window_dispmanx_egl_show must make it visible. Then add a dispmanx case in the line pointed above. That should cover all the cases.
Created attachment 300143 [details] [review] dispmanx/gl: surfaceless EGL context support Show the DispmanX window only if there's no shared external GL context set up. When a window is required by the context a transparent DispmanX element is created and later on made visible by the ::show method.
Review of attachment 300143 [details] [review]: Thx for the patch, it looks good, I just put a few remarks. ::: gst-libs/gst/gl/dispmanx/gstglwindow_dispmanx_egl.c @@ +40,3 @@ +#define ELEMENT_CHANGE_TRANSFORM (1<<5) +#endif + I guess it was not working to include vc_vchi_dispmanx.h ? @@ +370,2 @@ } Just a detail :) : You can remove this "if () window_resize" block as it is done in show. And to be close as what gstglwindow_x11.c does.
Created attachment 300146 [details] [review] dispmanx/gl: surfaceless EGL context support Show the DispmanX window only if there's no shared external GL context set up. When a window is required by the context a transparent DispmanX element is created and later on made visible by the ::show method.
(In reply to Julien Isorce from comment #8) > Review of attachment 300143 [details] [review] [review]: > > Thx for the patch, it looks good, I just put a few remarks. > > ::: gst-libs/gst/gl/dispmanx/gstglwindow_dispmanx_egl.c > @@ +40,3 @@ > +#define ELEMENT_CHANGE_TRANSFORM (1<<5) > +#endif > + > > I guess it was not working to include vc_vchi_dispmanx.h ? > No, here at least the vc_vchi_dispmanx_common.h header is pulled in but not found in my sysroot.
Review of attachment 300146 [details] [review]: Looks good, I'll push it soon.
Comment on attachment 300146 [details] [review] dispmanx/gl: surfaceless EGL context support commit 14ac40f106d5e742267f4a1ee6ce3af587a15b2e Author: Philippe Normand <philn@igalia.com> Date: Mon Mar 23 16:43:01 2015 +0100 gl/dispmanx: surfaceless EGL context support Show the DispmanX window only if there's no shared external GL context set up. When a window is required by the context a transparent DispmanX element is created and later on made visible by the ::show method. https://bugzilla.gnome.org/show_bug.cgi?id=746632