GNOME Bugzilla – Bug 734482
Make OpenGL plugins more accessible for application programmers
Last modified: 2014-09-04 23:42:04 UTC
Created attachment 282909 [details] [review] Implement introspection for the gl plugins This series of patches implements introspection support for the OpenGL plugins and expose the OpenGL context in the draw callback of the glimagesink, needed for modern pipeline rendering in this callback. The patches are needed for the transfromation interface I wrote for GSoC this year.
Created attachment 282910 [details] [review] Expose the OpenGL handle for the shader object This is needed for uploading data like matrices with other OpenGL bindings like pyopengl.
Created attachment 282911 [details] [review] Add opengl context to draw callback For using shaders in the callback the opengl context needs to be exposed
Created attachment 282912 [details] [review] Fixes for the gltransformation Plugin The transformation order was wrong. Also renamed the fovy property to fov (field of view) and added the mvp matrix to be readable.
Created attachment 282913 [details] [review] Fix examples build with gtk+ 3.14
Review of attachment 282909 [details] [review]: ::: gst-libs/gst/gl/Makefile.am @@ +143,3 @@ + --pkg gstreamer-@GST_API_VERSION@ \ + --pkg gstreamer-base-@GST_API_VERSION@ \ + --pkg-export gstreamer-gl-@GST_API_VERSION@ \ Doesn't it also need gstreamer-video / GstVideo for the GstVideoInfo and other things in the API?
Review of attachment 282910 [details] [review]: Why? The problem I see here is that it's only usable from inside the GL thread
Review of attachment 282911 [details] [review]: Looks good
Review of attachment 282912 [details] [review]: ::: ext/gl/gstgltransformation.c @@ +308,3 @@ break; + case PROP_MVP: + filter->mvp_matrix = *((graphene_matrix_t *) g_value_get_boxed (value)); Storing boxed types by pointer is a bit weird... but ok :) Nonetheless you need to check for NULL here, which should probably be handled like the identity matrix
Comment on attachment 282913 [details] [review] Fix examples build with gtk+ 3.14 This is probably needed in all the other examples too then
== Introspection == I don't get any import errors and could use GstVideo for the video overlay in my Python example. How can I test if additional gir packages are required as dependency? The gir scanner creates the most warnings about undefined OpenGL types. == Exposure of shader handle == The shader handle, which is not a pointer but a OpenGL id, can be used in the drawing callback in the sink, since it used the same OpenGL context. It is not the same thread per se, but the same context. For using the OpenGL plugins in other languages, or using OpenGL functions not wrapped by GStreamer, this handle is required to be exposed. In my use case I need it to upload Python typed data to the GPU. The pyopengl functions work with this handle and are able to upload numpy types very fine. https://github.com/lubosz/gst-gl-tests/blob/master/gst_opengl_editor/opengl.py#L104 To make this possible without exposing the handle, I would need to wrap functions with numpy types for glUniformMatrix4fv in GStreamer. Additionally other things could be done with this handle that the GStGL api does not expose. == boxed type == After experimenting this was the best way to achieve this. I am even able to get the correct introspected type in python from this property. I will implement NULL checking after the review is finished. == gtk+ deprecation error == I just tried to build the -bad plugins with this gtk version afaik. One could expect this to happen in other places. GTK+ 3.14 is not released yet though, just tried the git version.
These all look fine. Just add the gstreamer-video pkg to the gi patch and the NULL check mentioned by Sebastian. I'm ok with exposing the shader handle as it is useful shared state that an application might need.
Created attachment 283996 [details] [review] Implement introspection for the gl plugins Add gst-video pkg
Created attachment 283997 [details] [review] Fixes for the gltransformation Plugin Add check for NULL pointers
Review of attachment 283997 [details] [review]: One issue :). ::: ext/gl/gstgltransformation.c @@ +309,3 @@ + case PROP_MVP: + if (value != NULL) + filter->mvp_matrix = *((graphene_matrix_t *) g_value_get_boxed (value)); You need to check if the value returned by g_value_get_boxed is not NULL (not value != NULL)
commit 5ae3d6859096eb253515cbd81ac9b97b1da617ff Author: Lubosz Sarnecki <lubosz@gmail.com> Date: Wed Jul 2 12:49:44 2014 +0200 gstopengl: add introspection support https://bugzilla.gnome.org/show_bug.cgi?id=734482 commit 57a96ce3e16854922749ec09953015ee4d9e2b22 Author: Lubosz Sarnecki <lubosz@gmail.com> Date: Tue Aug 5 12:07:08 2014 +0200 examples: fix gtk+ 3.14 deprecation error https://developer.gnome.org/gtk3/3.13/GtkWidget.html#gtk-widget-set-double-buffered https://bugzilla.gnome.org/show_bug.cgi?id=734482 commit ddaaff33766999c97e161bc207695d05bce82550 Author: Lubosz Sarnecki <lubosz@gmail.com> Date: Mon Jul 7 10:52:06 2014 +0200 glimagesink: expose context * expose context in draw / reshape callbacks * add context property https://bugzilla.gnome.org/show_bug.cgi?id=734482 commit d2cac06eebea98bc16b140e155af7f11ccce2397 Author: Lubosz Sarnecki <lubosz@gmail.com> Date: Mon Jul 7 10:51:28 2014 +0200 glshader: expose opengl handle in getter https://bugzilla.gnome.org/show_bug.cgi?id=734482
Created attachment 285327 [details] [review] Fixes for the gltransformation Plugin 2 Checks for NULL in g_value_get_boxed(value)
commit f1b026c4801ccde3a9200d0d6bc824031a47e4f5 Author: Lubosz Sarnecki <lubosz@gmail.com> Date: Mon Jul 7 10:52:57 2014 +0200 gltransformation: fix issues and expose mvp matrix * aspect should not be 0 on init * rename fovy to fov * add mvp to properties as boxed graphene type * fix transformation order. scale first * clear color with 1.0 alpha https://bugzilla.gnome.org/show_bug.cgi?id=734223