[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