[Bug 655719] wayland video sink: initial implementation

[GStreamer (bugzilla.gnome.org)]

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

Stefan Sauer (gstreamer, gtkdoc dev) <ensonic> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #200519|none                        |needs-work
             status|                            |

--- Comment #26 from Stefan Sauer (gstreamer, gtkdoc dev) <ensonic at sonicpulse.de> 2012-03-04 19:36:21 UTC ---
Review of attachment 200519:
 --> (https://bugzilla.gnome.org/review?bug=655719&attachment=200519)

::: ext/wayland/gstwaylandsink.c
@@ +3,3 @@
+ *
+ * Copyright: Intel Corporation
+ * Copyright: Sreerenj Balachandran <sreerenj.balachandran at intel.com>

Please add year like in header file.

@@ +21,3 @@
+
+
+/* The waylandsink is currently just a prototype . It creates its own window
and render the decoded video frames to that.*/

this is a good place to have the standard gtk-doc blob (even if the element is
not part of the docs yet).

@@ +84,3 @@
+
+/*Fixme: Add more interfaces */
+GST_BOILERPLATE (GstWayLandSink, gst_wayland_sink, GstVideoSink,

according to http://wayland.freedesktop.org/ it is "Wayland" and not "WayLand",
"WayLand" would also imply "way_land".

@@ +131,3 @@
+      "wayland video sink", "Sink/Video",
+      "Output to wayland surface",
+      "Sreerenj Balachandran <sreerenj.balachandran at intel.com>,");

drop the ',' at the end

@@ +163,3 @@
+      g_param_spec_pointer ("wayland-display", "WayLand Display",
+          "WayLand  Display id created by the application ",
+          G_PARAM_READWRITE));

G_PARAM_READWRITE|G_PARAM_STATIC_STRINGS
The display id is a pointer? Call it "display handle" maybe?

@@ +165,3 @@
+          G_PARAM_READWRITE));
+
+  /*Fixme: not using now */

Please use a #if 0 then, to keep it compiled out (together with a little
comment telling why you'd like to keep it)

@@ +227,3 @@
+gst_wayland_sink_dispose (GObject * object)
+{
+  GstWayLandSink *sink = GST_WAYLAND_SINK (object);

don't add an empty dispose method.

@@ +237,3 @@
+  GstWayLandSink *sink = GST_WAYLAND_SINK (object);
+
+  gst_caps_replace (&sink->caps, NULL);

Or move the gst_caps_replace to dispose, its an unref op and those should in
theory be in dispose, so that dispose could be called multiple times to break
ref-cycles (won't be the case here). Up to you :)

@@ +385,3 @@
+
+  /* We intersect those caps with our template to make sure they are correct
*/
+  intersection = gst_caps_intersect (allowed_caps, caps);

if you just want to know that the intersection is non-empty use
gst_caps_can_intersect()

@@ +543,3 @@
+    guint len = GST_BUFFER_SIZE (buffer) / sink->height;
+
+    /*for (i = 0; i < sink->height; i++) {

remove?

@@ +584,3 @@
+    GST_VERSION_MINOR,
+    "waylandsink",
+    "WayLand Video Sink", plugin_init, VERSION, "LGPL", "gst-wayland", "")

"gst-wayland" -> GST_PACKAGE_ORIGIN

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