[gstreamer-bugs] [Bug 610249] [xoverlay] add resize() and set_render_rect() methods

GStreamer (bugzilla.gnome.org) bugzilla at gnome.org
Thu Feb 18 03:02:32 PST 2010


https://bugzilla.gnome.org/show_bug.cgi?id=610249
  GStreamer | gst-plugins-base | unspecified

Tim-Philipp Müller <t.i.m> changed:

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

--- Comment #4 from Tim-Philipp Müller <t.i.m at zen.co.uk> 2010-02-18 11:02:27 UTC ---
Review of attachment 154042:
 --> (https://bugzilla.gnome.org/review?bug=610249&attachment=154042)

Bunch of nitpicks:

- the commit message could be improved:
    - the summary should say what vmethods you are adding
    - please write 'first' and 'second' if you don't just enumerate
      or list.
    - more about the *why* would be good, I think
    - 'windowless toolkits' - an example would be nice

::: gst-libs/gst/interfaces/Makefile.am
@@ +91,3 @@
         -I$(top_builddir)/gst-libs \
         --add-include-path=`$(PKG_CONFIG) --variable=libdir
gstreamer-0.10`/gst \
+        --add-include-path=$(builddir)/../video \

really builddir?

@@ +113,2 @@
 %.typelib: %.gir $(INTROSPECTION_COMPILER)
+    $(INTROSPECTION_COMPILER) --includedir=$(srcdir) --includedir=$(builddir)
--includedir=`$(PKG_CONFIG) --variable=libdir gstreamer-0.10`/gst
--includedir=$(builddir)/../video $(INTROSPECTION_COMPILER_OPTS) $< -o $(@F)

here too: really builddir?

::: gst-libs/gst/interfaces/xoverlay.c
@@ +465,3 @@
+ * in the drawable even if the pipeline is PAUSED. It is needed if the sink is
+ * not able to detect resizes.
+ */

- in what cases would the sink not be able to detect resizes for example?

- Since: markers

@@ +467,3 @@
+ */
+void
+gst_x_overlay_resize (GstXOverlay * overlay)

Wonder if there's a better name for this that makes it clearer that the
function is to notify the sink of the resize after the fact and not to be used
to resize/set the size.

@@ +482,3 @@
+
+/**
+ * gst_x_overlay_set_render_rect:

Can we make this _set_render_rectangle()?

@@ +487,3 @@
+ *
+ * Configure a subregion as a video target within the window set by
+ * gst_x_overlay_set_xwindow_id().

I'm not really sure what this means (configure as a target) - would be nice to
flesh this out a little bit more :)

::: gst-libs/gst/interfaces/xoverlay.h
@@ +76,3 @@
+  
+  void (* resize)              (GstXOverlay *overlay);
+  

Same comments re. naming as for the methods.

@@ +79,2 @@
   /*< private >*/
+  gpointer                 _gst_reserved[GST_PADDING - 3];

I think we can get rid of the padding block, since this is an interface, no?

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



More information about the Gstreamer-bugs mailing list