[Bug 745107] glimagesink: implement GstVideoOverlayCompositionMeta

[GStreamer (GNOME Bugzilla)]

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

Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> changed:

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

--- Comment #50 from Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> ---
Review of attachment 306511:
 --> (https://bugzilla.gnome.org/review?bug=745107&attachment=306511)

::: gst-libs/gst/gl/gstglcompositionoverlay.c
@@ +34,3 @@
+
+#define DEBUG_INIT \
+  GST_DEBUG_CATEGORY_INIT (gst_gl_composition_overlay_debug,
"glcompositionoverlay", 0, "compositionoverlay");

I suspect this is larger then 80 characters.

@@ +41,3 @@
+
+void
+gst_gl_composition_overlay_add_transformation (GstGLCompositionOverlay *

Missing static, put the type on same line unless the gst-indent says otherwise.

@@ +279,3 @@
+      GST_MAP_READ);
+
+  if (raw_overlay_data != NULL) {

I would rather check the return value of gst_video_meta_map().

@@ +312,3 @@
+  }
+
+  gst_video_meta_unmap (vmeta, 0, &info);

Only unmap if map return TRUE.

@@ +323,3 @@
+  if (overlay->texture_id != -1)
+    gl->BindTexture (GL_TEXTURE_2D, overlay->texture_id);
+  gl->DrawElements (GL_TRIANGLES, 6, GL_UNSIGNED_SHORT, 0);

If there is no texture to draw to, isn't it a better idea to simpy not do
anything ?

::: gst-libs/gst/gl/gstglcompositionoverlay.h
@@ +26,3 @@
+#include <gst/gl/gstgl_fwd.h>
+
+G_BEGIN_DECLS GType gst_gl_composition_overlay_get_type (void);

This is uncommon syntax. I prefer when G_BEGIN_DECLS is on separate line.

@@ +40,3 @@
+ */
+struct _GstGLCompositionOverlay
+{

For documentation you need /*< private >*/ here for opaque type.

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