[Bug 703486] Implement GstNavigation interface in glimagesink

[GStreamer (bugzilla.gnome.org)]

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

Matthew Waters <ystreet00> changed:

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

--- Comment #35 from Matthew Waters <ystreet00 at gmail.com> 2014-08-05 14:50:14 UTC ---
Review of attachment 282444:
 --> (https://bugzilla.gnome.org/review?bug=703486&attachment=282444)

Just a couple of little things :)

The GST_VIDEO_SINK_WIDTH/HEIGHT usage looks fine.

::: ext/gl/gstglimagesink.c
@@ +210,3 @@
+  if (width != GST_VIDEO_SINK_WIDTH (sink) &&
+      gst_structure_get_double (structure, "pointer_x", &x)) {
+    xscale = (gdouble) GST_VIDEO_SINK_WIDTH (sink) / width;

potential divide by zero.

@@ +216,3 @@
+  if (height != GST_VIDEO_SINK_HEIGHT (sink) &&
+      gst_structure_get_double (structure, "pointer_y", &y)) {
+    yscale = (gdouble) GST_VIDEO_SINK_HEIGHT (sink) / height;

potential divide by zero.

::: gst-libs/gst/gl/gstglwindow.c
@@ +603,3 @@

+/**
+ * gst_gl_window_get_surface_dimensions

missing : at the end

@@ +606,3 @@
+ * @window: a #GstGLWindow
+ * @width: place holder for width of surface 
+ * @height: place holder for height of surface 

what about?
@width: (out): resulting surface width
@height: (out): resulting surface height

@@ +607,3 @@
+ * @width: place holder for width of surface 
+ * @height: place holder for height of surface 
+ * Returns: nothing

"Returns:" Not needed for void functions

-- 
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.