[Bug 766206] plugins: negotiate GstGLContext during the GstContext sharing

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766206

Matthew Waters (ystreet00) <ystreet00 at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #331372|none                        |reviewed
             status|                            |

--- Comment #5 from Matthew Waters (ystreet00) <ystreet00 at gmail.com> ---
Review of attachment 331372:
 --> (https://bugzilla.gnome.org/review?bug=766206&attachment=331372)

Looks mostly ok from the libgstgl side.

::: gst/vaapi/gstvaapivideocontext.c
@@ +27,3 @@
 #include "gstvaapivideocontext.h"
+#if USE_GST_GL_HELPERS
+# include <gst/gl/gstglcontext.h>

Any of the object specific headers aren't guarenteed to be straight includable.
Use #include <gst/gl/gl.h>

@@ +266,3 @@
+  if (gl_context && gl_context_ptr) {
+    *gl_context_ptr = gl_context;
+    return TRUE;

This may not do what the function name implies.

One should probably assert that gl_context_ptr is non-NULL on function entry
(otherwise there's no point calling the function) and always return TRUE when a
gl_context is found.

::: gst/vaapi/gstvaapivideocontext.h
@@ +59,3 @@
+G_GNUC_INTERNAL
+gboolean
+gst_gl_find_local_context (GstElement * element, GstObject ** gl_context_ptr);

Creating a function in the gst_gl namespace isn't a good idea at all.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.