timeline-element: simplify check for being edited

It should be sufficient to set the edit flag only on the toplevel, which
allows all of its children to know they are being edited and should not
move in response.

Also, removed some unnecessary setting/checking of this.

Also, supplied the ges_timeline_element_peak_toplevel, which unlike
ges_timeline_element_get_toplevel_parent, does not add a reference to
the toplevel. Some corresponding leaks in auto-transition have been
fixed by using this instead.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-editing-services/-/merge_requests/169>
This commit is contained in:
Henry Wilkes 2020-04-20 13:13:48 +01:00
parent cda1cfa0df
commit bac0df294e
7 changed files with 85 additions and 113 deletions

View file

@ -44,24 +44,22 @@ neighbour_changed_cb (GESClip * clip, GParamSpec * arg G_GNUC_UNUSED,
{
gint64 new_duration;
GESTimelineElement *parent =
ges_timeline_element_get_toplevel_parent (GES_TIMELINE_ELEMENT (clip));
ges_timeline_element_peak_toplevel (GES_TIMELINE_ELEMENT (clip));
if (ELEMENT_FLAG_IS_SET (parent, GES_TIMELINE_ELEMENT_SET_SIMPLE)) {
if (GES_TIMELINE_ELEMENT_BEING_EDITED (parent))
return;
}
if (parent) {
GESTimelineElement *prev_topparent =
ges_timeline_element_get_toplevel_parent (GES_TIMELINE_ELEMENT
ges_timeline_element_peak_toplevel (GES_TIMELINE_ELEMENT
(self->next_source));
GESTimelineElement *next_topparent =
ges_timeline_element_get_toplevel_parent (GES_TIMELINE_ELEMENT
ges_timeline_element_peak_toplevel (GES_TIMELINE_ELEMENT
(self->previous_source));
if (ELEMENT_FLAG_IS_SET (prev_topparent, GES_TIMELINE_ELEMENT_SET_SIMPLE) ||
ELEMENT_FLAG_IS_SET (next_topparent, GES_TIMELINE_ELEMENT_SET_SIMPLE)) {
if (GES_TIMELINE_ELEMENT_BEING_EDITED (prev_topparent) ||
GES_TIMELINE_ELEMENT_BEING_EDITED (next_topparent))
return;
}
if (parent == prev_topparent && parent == next_topparent) {
GST_DEBUG_OBJECT (self,
@ -91,11 +89,11 @@ neighbour_changed_cb (GESClip * clip, GParamSpec * arg G_GNUC_UNUSED,
}
self->positioning = TRUE;
ELEMENT_SET_FLAG (self->transition_clip, GES_TIMELINE_ELEMENT_SET_SIMPLE);
GES_TIMELINE_ELEMENT_SET_BEING_EDITED (self->transition_clip);
_set_start0 (GES_TIMELINE_ELEMENT (self->transition_clip),
_START (self->next_source));
_set_duration0 (GES_TIMELINE_ELEMENT (self->transition_clip), new_duration);
ELEMENT_UNSET_FLAG (self->transition_clip, GES_TIMELINE_ELEMENT_SET_SIMPLE);
GES_TIMELINE_ELEMENT_UNSET_BEING_EDITED (self->transition_clip);
self->positioning = FALSE;
}

View file

@ -297,7 +297,7 @@ _update_duration_limit (GESClip * self)
GST_TIME_FORMAT, GST_TIME_ARGS (duration_limit));
if (_CLOCK_TIME_IS_LESS (duration_limit, element->duration)
&& !ELEMENT_FLAG_IS_SET (self, GES_TIMELINE_ELEMENT_SET_SIMPLE)) {
&& !GES_TIMELINE_ELEMENT_BEING_EDITED (self)) {
gboolean res;
GST_INFO_OBJECT (self, "Automatically reducing duration to %"
@ -667,11 +667,8 @@ _set_duration (GESTimelineElement * element, GstClockTime duration)
for (tmp = container->children; tmp; tmp = g_list_next (tmp)) {
GESTimelineElement *child = (GESTimelineElement *) tmp->data;
if (child != container->initiated_move) {
ELEMENT_SET_FLAG (child, GES_TIMELINE_ELEMENT_SET_SIMPLE);
if (child != container->initiated_move)
_set_duration0 (GES_TIMELINE_ELEMENT (child), duration);
ELEMENT_UNSET_FLAG (child, GES_TIMELINE_ELEMENT_SET_SIMPLE);
}
}
container->children_control_mode = GES_CHILDREN_UPDATE;
g_list_free_full (children, gst_object_unref);
@ -1880,8 +1877,7 @@ ges_clip_move_to_layer (GESClip * clip, GESLayer * layer)
return FALSE;
}
if (layer->timeline
&& !ELEMENT_FLAG_IS_SET (clip, GES_TIMELINE_ELEMENT_SET_SIMPLE)) {
if (layer->timeline && !GES_TIMELINE_ELEMENT_BEING_EDITED (clip)) {
/* move to new layer, also checks moving of toplevel */
return timeline_tree_move (timeline_get_tree (layer->timeline),
element, (gint64) ges_layer_get_priority (current_layer) -
@ -2308,10 +2304,9 @@ ges_clip_split (GESClip * clip, guint64 position)
gst_object_ref (track));
}
ELEMENT_SET_FLAG (clip, GES_TIMELINE_ELEMENT_SET_SIMPLE);
GES_TIMELINE_ELEMENT_SET_BEING_EDITED (clip);
_set_duration0 (GES_TIMELINE_ELEMENT (clip), old_duration);
ELEMENT_UNSET_FLAG (clip, GES_TIMELINE_ELEMENT_SET_SIMPLE);
g_object_notify (G_OBJECT (clip), "duration");
GES_TIMELINE_ELEMENT_UNSET_BEING_EDITED (clip);
/* add to the track after the duration change so we don't overlap! */
for (tmp = GES_CONTAINER_CHILDREN (new_object); tmp; tmp = tmp->next) {

View file

@ -554,8 +554,7 @@ _update_start_duration (GESContainer * container, GESTimelineElement * child)
{
GList *tmp;
GstClockTime duration, end = 0, start = G_MAXUINT64;
gboolean was_setting_simple =
ELEMENT_FLAG_IS_SET (container, GES_TIMELINE_ELEMENT_SET_SIMPLE);
gboolean was_being_edited = GES_TIMELINE_ELEMENT_BEING_EDITED (container);
if (!container->children) {
/* If we are now empty, keep the same duration and start. This works
@ -565,7 +564,7 @@ _update_start_duration (GESContainer * container, GESTimelineElement * child)
return;
}
ELEMENT_SET_FLAG (container, GES_TIMELINE_ELEMENT_SET_SIMPLE);
GES_TIMELINE_ELEMENT_SET_BEING_EDITED (container);
for (tmp = container->children; tmp; tmp = tmp->next) {
start = MIN (start, _START (tmp->data));
@ -592,8 +591,8 @@ _update_start_duration (GESContainer * container, GESTimelineElement * child)
if (prev_dur != duration)
g_object_notify (G_OBJECT (container), "duration");
}
if (!was_setting_simple)
ELEMENT_UNSET_FLAG (container, GES_TIMELINE_ELEMENT_SET_SIMPLE);
if (!was_being_edited)
GES_TIMELINE_ELEMENT_UNSET_BEING_EDITED (container);
g_hash_table_foreach (container->priv->mappings,
(GHFunc) _resync_position_offsets, container);
@ -607,24 +606,24 @@ _child_start_changed_cb (GESTimelineElement * child,
GESContainerPrivate *priv = container->priv;
GESTimelineElement *element = GES_TIMELINE_ELEMENT (container);
GESChildrenControlMode pmode = container->children_control_mode;
GESChildrenControlMode mode = container->children_control_mode;
if (mode == GES_CHILDREN_IGNORE_NOTIFIES)
return;
if (GES_TIMELINE_ELEMENT_BEING_EDITED (child))
mode = GES_CHILDREN_UPDATE_ALL_VALUES;
map = g_hash_table_lookup (priv->mappings, child);
g_assert (map);
if (ELEMENT_FLAG_IS_SET (child, GES_TIMELINE_ELEMENT_SET_SIMPLE))
container->children_control_mode = GES_CHILDREN_UPDATE_ALL_VALUES;
switch (container->children_control_mode) {
case GES_CHILDREN_IGNORE_NOTIFIES:
break;
switch (mode) {
case GES_CHILDREN_UPDATE_ALL_VALUES:
_update_start_duration (container, child);
break;
case GES_CHILDREN_UPDATE_OFFSETS:
map->start_offset = _START (container) - _START (child);
break;
case GES_CHILDREN_UPDATE:
/* We update all the children calling our set_start method */
container->initiated_move = child;
@ -634,8 +633,6 @@ _child_start_changed_cb (GESTimelineElement * child,
default:
break;
}
container->children_control_mode = pmode;
}
static void
@ -646,20 +643,18 @@ _child_duration_changed_cb (GESTimelineElement * child,
GESContainerPrivate *priv = container->priv;
GESTimelineElement *element = GES_TIMELINE_ELEMENT (container);
GESChildrenControlMode pmode = container->children_control_mode;
GESChildrenControlMode mode = container->children_control_mode;
if (pmode == GES_CHILDREN_IGNORE_NOTIFIES)
if (mode == GES_CHILDREN_IGNORE_NOTIFIES)
return;
if (ELEMENT_FLAG_IS_SET (child, GES_TIMELINE_ELEMENT_SET_SIMPLE))
container->children_control_mode = GES_CHILDREN_UPDATE_ALL_VALUES;
if (GES_TIMELINE_ELEMENT_BEING_EDITED (child))
mode = GES_CHILDREN_UPDATE_ALL_VALUES;
map = g_hash_table_lookup (priv->mappings, child);
g_assert (map);
switch (container->children_control_mode) {
case GES_CHILDREN_IGNORE_NOTIFIES:
break;
switch (mode) {
case GES_CHILDREN_UPDATE_ALL_VALUES:
_update_start_duration (container, child);
break;
@ -678,8 +673,6 @@ _child_duration_changed_cb (GESTimelineElement * child,
default:
break;
}
container->children_control_mode = pmode;
}
/****************************************************

View file

@ -203,7 +203,7 @@ _child_clip_changed_layer_cb (GESTimelineElement * clip,
/* sigids takes ownership of new_layer, we take ownership of old_layer */
sigids->layer = new_layer;
if (ELEMENT_FLAG_IS_SET (clip, GES_TIMELINE_ELEMENT_SET_SIMPLE)) {
if (GES_TIMELINE_ELEMENT_BEING_EDITED (clip)) {
GST_DEBUG_OBJECT (container, "Not moving with change in priority of "
"clip child %" GES_FORMAT, GES_ARGS (clip));
_update_our_values (GES_GROUP (clip->parent));
@ -262,7 +262,8 @@ _child_group_priority_changed (GESTimelineElement * child,
return;
}
/* if a child group is simply updating its values we will do the same */
if (child_group->priv->updating_priority) {
if (child_group->priv->updating_priority ||
GES_TIMELINE_ELEMENT_BEING_EDITED (child)) {
GST_DEBUG_OBJECT (container, "Not moving with change in priority of "
"group child %" GES_FORMAT " because it is not moving itself",
GES_ARGS (child_group));
@ -318,8 +319,6 @@ _set_priority (GESTimelineElement * element, guint32 priority)
if (GES_GROUP (element)->priv->updating_priority == TRUE)
return TRUE;
container->children_control_mode = GES_CHILDREN_IGNORE_NOTIFIES;
layers = GES_TIMELINE_ELEMENT_TIMELINE (element) ?
GES_TIMELINE_ELEMENT_TIMELINE (element)->layers : NULL;
if (layers == NULL) {
@ -332,11 +331,15 @@ _set_priority (GESTimelineElement * element, guint32 priority)
/* FIXME: why are we not shifting ->max_layer_prio? */
if (GES_TIMELINE_ELEMENT_BEING_EDITED (element))
return TRUE;
container->children_control_mode = GES_CHILDREN_IGNORE_NOTIFIES;
for (tmp = GES_CONTAINER_CHILDREN (element); tmp; tmp = tmp->next) {
GESTimelineElement *child = tmp->data;
if (child != container->initiated_move
|| ELEMENT_FLAG_IS_SET (container, GES_TIMELINE_ELEMENT_SET_SIMPLE)) {
if (child != container->initiated_move) {
if (GES_IS_CLIP (child)) {
guint32 layer_prio = GES_TIMELINE_ELEMENT_LAYER_PRIORITY (child) + diff;
GESLayer *layer =
@ -370,33 +373,20 @@ _set_start (GESTimelineElement * element, GstClockTime start)
{
GList *tmp, *children;
gint64 diff = start - _START (element);
GESTimeline *timeline;
GESContainer *container = GES_CONTAINER (element);
GESTimelineElement *toplevel =
ges_timeline_element_get_toplevel_parent (element);
gst_object_unref (toplevel);
if (GES_GROUP (element)->priv->setting_value == TRUE)
/* Let GESContainer update itself */
return GES_TIMELINE_ELEMENT_CLASS (parent_class)->set_start (element,
start);
if (ELEMENT_FLAG_IS_SET (element, GES_TIMELINE_ELEMENT_SET_SIMPLE) ||
ELEMENT_FLAG_IS_SET (toplevel, GES_TIMELINE_ELEMENT_SET_SIMPLE)) {
/* get copy of children, since GESContainer may resort the group */
children = ges_container_get_children (container, FALSE);
container->children_control_mode = GES_CHILDREN_IGNORE_NOTIFIES;
for (tmp = children; tmp; tmp = tmp->next)
_set_start0 (tmp->data, _START (tmp->data) + diff);
container->children_control_mode = GES_CHILDREN_UPDATE;
g_list_free_full (children, gst_object_unref);
return TRUE;
}
timeline = GES_TIMELINE_ELEMENT_TIMELINE (element);
if (timeline)
return ges_timeline_move_object_simple (timeline, element, NULL,
GES_EDGE_NONE, start);
/* get copy of children, since GESContainer may resort the group */
children = ges_container_get_children (container, FALSE);
container->children_control_mode = GES_CHILDREN_IGNORE_NOTIFIES;
for (tmp = children; tmp; tmp = tmp->next)
_set_start0 (tmp->data, _START (tmp->data) + diff);
container->children_control_mode = GES_CHILDREN_UPDATE;
g_list_free_full (children, gst_object_unref);
return TRUE;
}

View file

@ -86,6 +86,21 @@ GstDebugCategory * _ges_debug (void);
#define GES_TRACK_ELEMENT_IS_CORE(child) \
(ges_track_element_get_creator_asset (GES_TRACK_ELEMENT (child)) != NULL)
#define GES_TIMELINE_ELEMENT_SET_BEING_EDITED(element) \
ELEMENT_SET_FLAG ( \
ges_timeline_element_peak_toplevel (GES_TIMELINE_ELEMENT (element)), \
GES_TIMELINE_ELEMENT_SET_SIMPLE)
#define GES_TIMELINE_ELEMENT_UNSET_BEING_EDITED(element) \
ELEMENT_UNSET_FLAG ( \
ges_timeline_element_peak_toplevel (GES_TIMELINE_ELEMENT (element)), \
GES_TIMELINE_ELEMENT_SET_SIMPLE)
#define GES_TIMELINE_ELEMENT_BEING_EDITED(element) \
ELEMENT_FLAG_IS_SET ( \
ges_timeline_element_peak_toplevel (GES_TIMELINE_ELEMENT (element)), \
GES_TIMELINE_ELEMENT_SET_SIMPLE)
#define SUPRESS_UNUSED_WARNING(a) (void)a
@ -443,6 +458,7 @@ typedef enum
GES_TIMELINE_ELEMENT_SET_SIMPLE = (1 << 1),
} GESTimelineElementFlags;
G_GNUC_INTERNAL GESTimelineElement * ges_timeline_element_peak_toplevel (GESTimelineElement * self);
G_GNUC_INTERNAL gdouble ges_timeline_element_get_media_duration_factor(GESTimelineElement *self);
G_GNUC_INTERNAL GESTimelineElement * ges_timeline_element_get_copied_from (GESTimelineElement *self);
G_GNUC_INTERNAL GESTimelineElementFlags ges_timeline_element_flags (GESTimelineElement *self);

View file

@ -673,6 +673,18 @@ _set_name (GESTimelineElement * self, const gchar * wanted_name)
/*********************************************
* Internal and private helpers *
*********************************************/
GESTimelineElement *
ges_timeline_element_peak_toplevel (GESTimelineElement * self)
{
GESTimelineElement *toplevel = self;
while (toplevel->parent)
toplevel = toplevel->parent;
return toplevel;
}
gdouble
ges_timeline_element_get_media_duration_factor (GESTimelineElement * self)
{
@ -1072,18 +1084,11 @@ ges_timeline_element_set_start (GESTimelineElement * self, GstClockTime start)
" new start: %" GST_TIME_FORMAT,
GST_TIME_ARGS (GES_TIMELINE_ELEMENT_START (self)), GST_TIME_ARGS (start));
toplevel_container = ges_timeline_element_get_toplevel_parent (self);
if (self->timeline
&& !ELEMENT_FLAG_IS_SET (self, GES_TIMELINE_ELEMENT_SET_SIMPLE)
&& !ELEMENT_FLAG_IS_SET (toplevel_container,
GES_TIMELINE_ELEMENT_SET_SIMPLE)) {
gst_object_unref (toplevel_container);
if (self->timeline && !GES_TIMELINE_ELEMENT_BEING_EDITED (self))
return ges_timeline_element_edit (self, NULL, -1, GES_EDIT_MODE_NORMAL,
GES_EDGE_NONE, start);
}
toplevel_container = ges_timeline_element_peak_toplevel (self);
parent = self->parent;
/* FIXME This should not belong to GESTimelineElement */
@ -1097,11 +1102,9 @@ ges_timeline_element_set_start (GESTimelineElement * self, GstClockTime start)
"Can not move the object as it would imply its "
"container to have a negative start value");
gst_object_unref (toplevel_container);
return FALSE;
}
gst_object_unref (toplevel_container);
klass = GES_TIMELINE_ELEMENT_GET_CLASS (self);
if (klass->set_start) {
gint res = klass->set_start (self, start);
@ -1256,22 +1259,15 @@ ges_timeline_element_set_duration (GESTimelineElement * self,
GstClockTime duration)
{
GESTimelineElementClass *klass;
GESTimelineElement *toplevel;
g_return_val_if_fail (GES_IS_TIMELINE_ELEMENT (self), FALSE);
if (duration == self->duration)
return TRUE;
toplevel = ges_timeline_element_get_toplevel_parent (self);
if (self->timeline &&
!ELEMENT_FLAG_IS_SET (self, GES_TIMELINE_ELEMENT_SET_SIMPLE) &&
!ELEMENT_FLAG_IS_SET (toplevel, GES_TIMELINE_ELEMENT_SET_SIMPLE)) {
gst_object_unref (toplevel);
if (self->timeline && !GES_TIMELINE_ELEMENT_BEING_EDITED (self))
return ges_timeline_element_edit (self, NULL, -1, GES_EDIT_MODE_TRIM,
GES_EDGE_END, self->start + duration);
}
gst_object_unref (toplevel);
GST_DEBUG_OBJECT (self, "current duration: %" GST_TIME_FORMAT
" new duration: %" GST_TIME_FORMAT,
@ -1707,12 +1703,11 @@ G_GNUC_END_IGNORE_DEPRECATIONS; /* End ignoring GParameter deprecation */
GESTimelineElement *
ges_timeline_element_get_toplevel_parent (GESTimelineElement * self)
{
GESTimelineElement *toplevel = self;
GESTimelineElement *toplevel;
g_return_val_if_fail (GES_IS_TIMELINE_ELEMENT (self), NULL);
while (GES_TIMELINE_ELEMENT_PARENT (toplevel))
toplevel = GES_TIMELINE_ELEMENT_PARENT (toplevel);
toplevel = ges_timeline_element_peak_toplevel (self);
return gst_object_ref (toplevel);
}

View file

@ -198,18 +198,6 @@ timeline_tree_debug (GNode * root)
(GNodeTraverseFunc) print_node, NULL);
}
static inline GESTimelineElement *
get_toplevel_container (gpointer element)
{
GESTimelineElement *ret =
ges_timeline_element_get_toplevel_parent ((GESTimelineElement
*) (element));
/* We own a ref to the elements ourself */
gst_object_unref (ret);
return ret;
}
static GNode *
find_node (GNode * root, gpointer element)
{
@ -246,7 +234,7 @@ timeline_tree_track_element (GNode * root, GESTimelineElement * element)
g_signal_connect (element, "notify::parent",
G_CALLBACK (timeline_element_parent_cb), root);
toplevel = get_toplevel_container (element);
toplevel = ges_timeline_element_peak_toplevel (element);
if (toplevel == element) {
GST_DEBUG ("Tracking toplevel element %" GES_FORMAT, GES_ARGS (element));
@ -1462,7 +1450,6 @@ static gboolean
perform_element_edit (GESTimelineElement * element, EditData * edit)
{
gboolean ret = FALSE;
GESTimelineElement *toplevel = get_toplevel_container (element);
guint32 layer_prio = ges_timeline_element_get_layer_priority (element);
switch (edit->mode) {
@ -1496,8 +1483,7 @@ perform_element_edit (GESTimelineElement * element, EditData * edit)
return FALSE;
}
ELEMENT_SET_FLAG (element, GES_TIMELINE_ELEMENT_SET_SIMPLE);
ELEMENT_SET_FLAG (toplevel, GES_TIMELINE_ELEMENT_SET_SIMPLE);
GES_TIMELINE_ELEMENT_SET_BEING_EDITED (element);
if (GST_CLOCK_TIME_IS_VALID (edit->start)) {
if (!ges_timeline_element_set_start (element, edit->start)) {
GST_ERROR_OBJECT (element, "Failed to set the start");
@ -1541,8 +1527,7 @@ perform_element_edit (GESTimelineElement * element, EditData * edit)
ret = TRUE;
done:
ELEMENT_UNSET_FLAG (element, GES_TIMELINE_ELEMENT_SET_SIMPLE);
ELEMENT_UNSET_FLAG (toplevel, GES_TIMELINE_ELEMENT_SET_SIMPLE);
GES_TIMELINE_ELEMENT_UNSET_BEING_EDITED (element);
return ret;
}
@ -1594,7 +1579,7 @@ timeline_tree_ripple (GNode * root, GESTimelineElement * element,
_REPLACE_TRACK_ELEMENT_WITH_PARENT (element);
ripple_toplevel = get_toplevel_container (element);
ripple_toplevel = ges_timeline_element_peak_toplevel (element);
/* if EDGE_END:
* TRIM_END the element, and MOVE all toplevels whose start is after
@ -1797,7 +1782,7 @@ timeline_tree_move (GNode * root, GESTimelineElement * element,
GST_INFO_OBJECT (element, "Moving with toplevel with offset %"
G_GINT64_FORMAT " and layer offset %" G_GINT64_FORMAT, offset,
layer_priority_offset);
element = get_toplevel_container (element);
element = ges_timeline_element_peak_toplevel (element);
mode = EDIT_MOVE;
break;
default: