[Bug 731722] OpenGL: Add an element for transforming video geometry

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=731722
  GStreamer | gst-plugins-bad | git

Mathieu Duponchelle <mathieu.duponchelle> changed:

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

--- Comment #1 from Mathieu Duponchelle <mathieu.duponchelle at epitech.eu> 2014-06-16 16:52:30 UTC ---
Review of attachment 278540:
 --> (https://bugzilla.gnome.org/review?bug=731722&attachment=278540)

Very clean patch in my opinion, I did not review the OpenGL related code as I
have no knowledge of OpenGL, but only had very minor nitpicks.

::: ext/gl/gstgltransformation.c
@@ +121,3 @@
+  gobject_class->set_property = gst_gl_transformation_set_property;
+  gobject_class->get_property = gst_gl_transformation_get_property;
+

Unneeded new line

@@ +138,3 @@
+          G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));
+
+  // Rotation

C++ style comment, not sure what the policy is but I guess it might be better
to stick to "/* */"

@@ +179,3 @@
+  g_object_class_install_property (gobject_class, PROP_XSCALE,
+      g_param_spec_float ("xscale", "X Scale",
+          "Scales the video at the X-Axis in times.",

Might be clearer to say "Scale multiplier for the X-axis" ?

@@ +299,3 @@
+  GstGLTransformation *transformation = GST_GL_TRANSFORMATION (filter);
+
+  if (transformation->aspect == 0)

This means you won't let the aspect change if the geometry of the input stream
changes right ?

@@ +312,3 @@
+  GstGLTransformation *transformation = GST_GL_TRANSFORMATION (filter);
+
+  /* blocking call, wait the opengl thread has destroyed the shader */

wait until* :)

@@ +324,3 @@
+
+  if (gst_gl_context_get_gl_api (filter->context)) {
+    /* blocking call, wait the opengl thread has compiled the shader */

wait until

@@ +412,3 @@
+  if (transformation->ortho) {
+    graphene_matrix_init_ortho (&projection_matrix,
+        -1 * transformation->aspect, 1 * transformation->aspect,

No need to multiply transformation->aspect by 1 in the third argument I guess
:)

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.