[Bug 743248] Add new control binding GstAppControlBinding
GStreamer (bugzilla.gnome.org)
bugzilla at gnome.org
Tue Jan 20 08:00:11 PST 2015
https://bugzilla.gnome.org/show_bug.cgi?id=743248
GStreamer | gstreamer (core) | unspecified
Sebastian Dröge (slomo) <slomo> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #295004|none |needs-work
status| |
--- Comment #1 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2015-01-20 16:00:07 UTC ---
Review of attachment 295004:
--> (https://bugzilla.gnome.org/review?bug=743248&attachment=295004)
Looks mostly good to me, just some minor comments :)
::: libs/gst/controller/gstappcontrolbinding.c
@@ +90,3 @@
+DEFINE_STORE_G_VALUE (float);
+DEFINE_STORE_G_VALUE (double);
+DEFINE_STORE_G_VALUE (boolean);
Hmm, I think for this we could use something from GTypeValueTable. That should
be lcopy_value, which is used e.g. by g_object_get().
@@ +122,3 @@
+DEFINE_ARE_EQUAL (float);
+DEFINE_ARE_EQUAL (double);
+DEFINE_ARE_EQUAL (boolean);
You could use gst_value_compare() here
@@ +169,3 @@
+ */
+ signals[SIGNAL_UPDATE_APP_VALUE] =
+ g_signal_new ("update-value", G_TYPE_FROM_CLASS (klass),
Signal enum and property name are out of sync :)
@@ +310,3 @@
+ if (G_LIKELY (app_val)) {
+
+ // Always update the first sync, then only if value changed
No C99 comments :)
@@ +317,3 @@
+
+ if (!self->last_value) {
+
empty newline
@@ +336,3 @@
+
+ return TRUE;
+
and here
@@ +434,3 @@
+ if (app_val) {
+ g_value_init (&values[i], type);
+ g_value_copy (app_val, &values[i]);
Why do you copy here instead of just memcpy(&values[i], app_val, sizeof
(GValue)); g_free(app_val);?
@@ +499,3 @@
+
+ // TODO: could allow app to return anything that is convertible to the
+ // expected type and try to convert here. For the moment, we are strict
C99 comment
::: libs/gst/controller/gstappcontrolbinding.h
@@ +84,3 @@
+ GValue * last_value;
+ GstAppControlBindingStoreValue store_func;
+ GstAppControlBindingValuesAreEqual equality_func;
I think these should all go into instance private data, together with their
function pointer typdefs
--
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