From d87578c843bba4c5bce8a36f5b86c51327bcd33b Mon Sep 17 00:00:00 2001 From: Thibault Saunier Date: Thu, 2 May 2019 11:41:10 -0400 Subject: [PATCH] element: Make return value of setters mean something Setters return values should return %FALSE **only** when the value could not be set, not when unchanged or when the subclass handled it itself! This patches makes it so the return value is meaningul by allowing subclasses return anything different than `TRUE` or `FALSE` (convention is -1) to let the subclass now that it took care of everything and no signal should be emited. --- ges/ges-clip.c | 2 +- ges/ges-group.c | 2 +- ges/ges-source-clip.c | 7 ++++--- ges/ges-timeline-element.c | 15 ++++++++------- ges/ges-timeline-element.h | 12 +++++++++--- ges/ges-track-element.c | 7 +++---- 6 files changed, 26 insertions(+), 19 deletions(-) diff --git a/ges/ges-clip.c b/ges/ges-clip.c index 58ee0b8f83..73330b9ca7 100644 --- a/ges/ges-clip.c +++ b/ges/ges-clip.c @@ -149,7 +149,7 @@ _set_start (GESTimelineElement * element, GstClockTime start) } container->children_control_mode = GES_CHILDREN_UPDATE; - return FALSE; + return -1; } static gboolean diff --git a/ges/ges-group.c b/ges/ges-group.c index 44c29bd8a7..ab12b7b32e 100644 --- a/ges/ges-group.c +++ b/ges/ges-group.c @@ -411,7 +411,7 @@ _set_duration (GESTimelineElement * element, GstClockTime duration) _set_duration0 (element, last_child_end - _START (element)); priv->setting_value = FALSE; - return FALSE; + return -1; } /**************************************************** diff --git a/ges/ges-source-clip.c b/ges/ges-source-clip.c index 755485a40d..46de6c5c74 100644 --- a/ges/ges-source-clip.c +++ b/ges/ges-source-clip.c @@ -56,9 +56,10 @@ _set_start (GESTimelineElement * element, GstClockTime start) if (element->timeline && !ELEMENT_FLAG_IS_SET (element, GES_TIMELINE_ELEMENT_SET_SIMPLE) && !ELEMENT_FLAG_IS_SET (toplevel, GES_TIMELINE_ELEMENT_SET_SIMPLE)) { - ges_timeline_move_object_simple (element->timeline, element, NULL, - GES_EDGE_NONE, start); - return FALSE; + if (!ges_timeline_move_object_simple (element->timeline, element, NULL, + GES_EDGE_NONE, start)) + return FALSE; + return -1; } return diff --git a/ges/ges-timeline-element.c b/ges/ges-timeline-element.c index c3b4c1c52f..5eaf0a7f37 100644 --- a/ges/ges-timeline-element.c +++ b/ges/ges-timeline-element.c @@ -721,15 +721,16 @@ ges_timeline_element_set_start (GESTimelineElement * self, GstClockTime start) gst_object_unref (toplevel_container); if (klass->set_start) { - gboolean res = klass->set_start (self, start); - if (res) { + gint res = klass->set_start (self, start); + if (res == TRUE) { self->start = start; g_object_notify_by_pspec (G_OBJECT (self), properties[PROP_START]); } GST_DEBUG_OBJECT (self, "New start: %" GST_TIME_FORMAT, GST_TIME_ARGS (GES_TIMELINE_ELEMENT_START (self))); - return res; + + return ! !res; } GST_WARNING_OBJECT (self, "No set_start virtual method implementation" @@ -764,12 +765,12 @@ ges_timeline_element_set_inpoint (GESTimelineElement * self, if (klass->set_inpoint) { gboolean res = klass->set_inpoint (self, inpoint); - if (res) { + if (res == TRUE) { self->inpoint = inpoint; g_object_notify_by_pspec (G_OBJECT (self), properties[PROP_INPOINT]); } - return res; + return ! !res; } GST_DEBUG_OBJECT (self, "No set_inpoint virtual method implementation" @@ -842,12 +843,12 @@ ges_timeline_element_set_duration (GESTimelineElement * self, if (klass->set_duration) { gboolean res = klass->set_duration (self, duration); - if (res) { + if (res == TRUE) { self->duration = duration; g_object_notify_by_pspec (G_OBJECT (self), properties[PROP_DURATION]); } - return res; + return ! !res; } GST_WARNING_OBJECT (self, "No set_duration virtual method implementation" diff --git a/ges/ges-timeline-element.h b/ges/ges-timeline-element.h index 22cdc228d8..7bb02a6b94 100644 --- a/ges/ges-timeline-element.h +++ b/ges/ges-timeline-element.h @@ -163,9 +163,15 @@ struct _GESTimelineElement /** * GESTimelineElementClass: * @set_parent: method to set the parent of a #GESTimelineElement. - * @set_start: method to set the start of a #GESTimelineElement - * @set_duration: method to set the duration of a #GESTimelineElement - * @set_inpoint: method to set the inpoint of a #GESTimelineElement + * @set_start: method to set the start of a #GESTimelineElement, + * -1 means that the subclass handled emiting the notify signal and + * the base class should return TRUE. + * @set_duration: method to set the duration of a #GESTimelineElement, + * -1 means that the subclass handled emiting the notify signal and + * the base class should return TRUE. + * @set_inpoint: method to set the inpoint of a #GESTimelineElement, + * -1 means that the subclass handled emiting the notify signal and + * the base class should return TRUE. * @set_max_duration: method to set the maximun duration of a #GESTimelineElement * @set_priority: method to set the priority of a #GESTimelineElement * @ripple: method to ripple an object diff --git a/ges/ges-track-element.c b/ges/ges-track-element.c index 345639da55..c0b7098136 100644 --- a/ges/ges-track-element.c +++ b/ges/ges-track-element.c @@ -480,7 +480,7 @@ _set_start (GESTimelineElement * element, GstClockTime start) g_return_val_if_fail (object->priv->nleobject, FALSE); if (G_UNLIKELY (start == _START (object))) - return FALSE; + return -1; g_object_set (object->priv->nleobject, "start", start, NULL); @@ -495,8 +495,7 @@ _set_inpoint (GESTimelineElement * element, GstClockTime inpoint) g_return_val_if_fail (object->priv->nleobject, FALSE); if (G_UNLIKELY (inpoint == _INPOINT (object))) - - return FALSE; + return -1; g_object_set (object->priv->nleobject, "inpoint", inpoint, NULL); _update_control_bindings (element, inpoint, GST_CLOCK_TIME_NONE); @@ -517,7 +516,7 @@ _set_duration (GESTimelineElement * element, GstClockTime duration) duration = _MAXDURATION (element) - _INPOINT (object); if (G_UNLIKELY (duration == _DURATION (object))) - return FALSE; + return -1; g_object_set (priv->nleobject, "duration", duration, NULL);