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 58ee0b8..73330b9 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 44c29bd..ab12b7b 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 755485a..46de6c5 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 c3b4c1c..5eaf0a7 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 22cdc22..7bb02a6 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 345639d..c0b7098 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); -- 2.7.4