After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 734482 - Make OpenGL plugins more accessible for application programmers
Make OpenGL plugins more accessible for application programmers
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
unspecified
Other All
: Normal enhancement
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-08-08 13:43 UTC by Lubosz Sarnecki
Modified: 2014-09-04 23:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Implement introspection for the gl plugins (2.96 KB, patch)
2014-08-08 13:43 UTC, Lubosz Sarnecki
reviewed Details | Review
Expose the OpenGL handle for the shader object (1.39 KB, patch)
2014-08-08 13:45 UTC, Lubosz Sarnecki
committed Details | Review
Add opengl context to draw callback (3.37 KB, patch)
2014-08-08 13:46 UTC, Lubosz Sarnecki
committed Details | Review
Fixes for the gltransformation Plugin (6.25 KB, patch)
2014-08-08 13:48 UTC, Lubosz Sarnecki
needs-work Details | Review
Fix examples build with gtk+ 3.14 (917 bytes, patch)
2014-08-08 13:48 UTC, Lubosz Sarnecki
committed Details | Review
Implement introspection for the gl plugins (3.00 KB, patch)
2014-08-20 19:31 UTC, Lubosz Sarnecki
committed Details | Review
Fixes for the gltransformation Plugin (6.27 KB, patch)
2014-08-20 19:32 UTC, Lubosz Sarnecki
needs-work Details | Review
Fixes for the gltransformation Plugin 2 (6.29 KB, patch)
2014-09-04 10:02 UTC, Lubosz Sarnecki
committed Details | Review

Description Lubosz Sarnecki 2014-08-08 13:43:41 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.
Comment 1 Lubosz Sarnecki 2014-08-08 13:45:51 UTC
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.
Comment 2 Lubosz Sarnecki 2014-08-08 13:46:35 UTC
Created attachment 282911 [details] [review]
Add opengl context to draw callback

For using shaders in the callback the opengl context needs to be exposed
Comment 3 Lubosz Sarnecki 2014-08-08 13:48:16 UTC
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.
Comment 4 Lubosz Sarnecki 2014-08-08 13:48:47 UTC
Created attachment 282913 [details] [review]
Fix examples build with gtk+ 3.14
Comment 5 Sebastian Dröge (slomo) 2014-08-11 08:12:18 UTC
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?
Comment 6 Sebastian Dröge (slomo) 2014-08-11 08:12:57 UTC
Review of attachment 282910 [details] [review]:

Why? The problem I see here is that it's only usable from inside the GL thread
Comment 7 Sebastian Dröge (slomo) 2014-08-11 08:13:31 UTC
Review of attachment 282911 [details] [review]:

Looks good
Comment 8 Sebastian Dröge (slomo) 2014-08-11 08:16:00 UTC
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 9 Sebastian Dröge (slomo) 2014-08-11 08:17:03 UTC
Comment on attachment 282913 [details] [review]
Fix examples build with gtk+ 3.14

This is probably needed in all the other examples too then
Comment 10 Lubosz Sarnecki 2014-08-18 09:16:31 UTC
== 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.
Comment 11 Matthew Waters (ystreet00) 2014-08-18 12:06:47 UTC
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.
Comment 12 Lubosz Sarnecki 2014-08-20 19:31:50 UTC
Created attachment 283996 [details] [review]
Implement introspection for the gl plugins

Add gst-video pkg
Comment 13 Lubosz Sarnecki 2014-08-20 19:32:32 UTC
Created attachment 283997 [details] [review]
Fixes for the gltransformation Plugin

Add check for NULL pointers
Comment 14 Matthew Waters (ystreet00) 2014-08-21 07:52:40 UTC
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)
Comment 15 Matthew Waters (ystreet00) 2014-08-21 08:43:03 UTC
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
Comment 16 Lubosz Sarnecki 2014-09-04 10:02:14 UTC
Created attachment 285327 [details] [review]
Fixes for the gltransformation Plugin 2

Checks for NULL in g_value_get_boxed(value)
Comment 17 Matthew Waters (ystreet00) 2014-09-04 23:41:15 UTC
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