clip: tidy handling of child priorities

Handle the child priorities in a way that keeps the container children
list sorted by priority at all times. Also, no longer rely on the
control_mode of the container, since we have less control over its value,
compared to private variables.

Also fixed the changing of priorities in set_top_effect_index:
previously *all* children whose priority was above or below the new
priority were shifted, when we should have been only shifting priorities
for the children whose priority lied *between* the old and the new
priority of the effect. E.g.
  effect:   A   B   C   D   E   F
  index:    0   1   2   3   4   5
After moving effect E to index 1, previously, we would get
  effect:   A   B   C   D   E   F
  index:    0   2   3   4   1   6
(this would have also shifted the priority for the core children as
well!). Whereas now, we have the correct:
  effect:   A   B   C   D   E   F
  index:    0   2   3   4   1   5
This commit is contained in:
Henry Wilkes 2020-03-02 13:25:21 +00:00
parent 91b5a5804a
commit d1276ed29b

View file

@ -97,6 +97,8 @@ struct _GESClipPrivate
GList *copied_track_elements; GList *copied_track_elements;
GESLayer *copied_layer; GESLayer *copied_layer;
gboolean prevent_priority_offset_update;
gboolean prevent_resort;
/* The formats supported by this Clip */ /* The formats supported by this Clip */
GESTrackType supportedformats; GESTrackType supportedformats;
@ -128,37 +130,52 @@ G_DEFINE_ABSTRACT_TYPE_WITH_PRIVATE (GESClip, ges_clip, GES_TYPE_CONTAINER);
* @max_priority: The absolute maximum priority a child of @container should have * @max_priority: The absolute maximum priority a child of @container should have
*/ */
static void static void
_get_priority_range (GESContainer * container, guint32 * min_priority, _get_priority_range_full (GESContainer * container, guint32 * min_priority,
guint32 * max_priority) guint32 * max_priority, guint32 priority_base)
{ {
GESLayer *layer = GES_CLIP (container)->priv->layer; GESLayer *layer = GES_CLIP (container)->priv->layer;
if (layer) { if (layer) {
*min_priority = _PRIORITY (container) + layer->min_nle_priority; *min_priority = priority_base + layer->min_nle_priority;
*max_priority = layer->max_nle_priority; *max_priority = layer->max_nle_priority;
} else { } else {
*min_priority = _PRIORITY (container) + MIN_NLE_PRIO; *min_priority = priority_base + MIN_NLE_PRIO;
*max_priority = G_MAXUINT32; *max_priority = G_MAXUINT32;
} }
} }
static void
_get_priority_range (GESContainer * container, guint32 * min_priority,
guint32 * max_priority)
{
_get_priority_range_full (container, min_priority, max_priority,
_PRIORITY (container));
}
static void static void
_child_priority_changed_cb (GESTimelineElement * child, _child_priority_changed_cb (GESTimelineElement * child,
GParamSpec * arg G_GNUC_UNUSED, GESContainer * container) GParamSpec * arg G_GNUC_UNUSED, GESContainer * container)
{ {
/* we do not change the rest of the clip in response to a change in
* the child priority */
guint32 min_prio, max_prio; guint32 min_prio, max_prio;
GESClipPrivate *priv = GES_CLIP (container)->priv;
GST_DEBUG_OBJECT (container, "TimelineElement %p priority changed to %i", GST_DEBUG_OBJECT (container, "TimelineElement %" GES_FORMAT
child, _PRIORITY (child)); " priority changed to %u", GES_ARGS (child), _PRIORITY (child));
if (container->children_control_mode == GES_CHILDREN_IGNORE_NOTIFIES) if (!priv->prevent_priority_offset_update) {
return; /* Update mapping */
_get_priority_range (container, &min_prio, &max_prio);
/* Update mapping */ _ges_container_set_priority_offset (container, child,
_get_priority_range (container, &min_prio, &max_prio); min_prio - _PRIORITY (child));
}
_ges_container_set_priority_offset (container, child, if (!priv->prevent_resort) {
min_prio - _PRIORITY (child)); _ges_container_sort_children (container);
_compute_height (container);
}
} }
/***************************************************** /*****************************************************
@ -246,20 +263,24 @@ _set_max_duration (GESTimelineElement * element, GstClockTime maxduration)
static gboolean static gboolean
_set_priority (GESTimelineElement * element, guint32 priority) _set_priority (GESTimelineElement * element, guint32 priority)
{ {
GESClipPrivate *priv = GES_CLIP (element)->priv;
GList *tmp; GList *tmp;
guint32 min_prio, max_prio; guint32 min_prio, max_prio;
GESContainer *container = GES_CONTAINER (element); GESContainer *container = GES_CONTAINER (element);
_get_priority_range (container, &min_prio, &max_prio); /* send the new 'priority' to determine what the new 'min_prio' should
* be for the clip */
_get_priority_range_full (container, &min_prio, &max_prio, priority);
container->children_control_mode = GES_CHILDREN_IGNORE_NOTIFIES; /* offsets will remain constant for the children */
priv->prevent_resort = TRUE;
priv->prevent_priority_offset_update = TRUE;
for (tmp = container->children; tmp; tmp = g_list_next (tmp)) { for (tmp = container->children; tmp; tmp = g_list_next (tmp)) {
guint32 track_element_prio; guint32 track_element_prio;
GESTimelineElement *child = (GESTimelineElement *) tmp->data; GESTimelineElement *child = (GESTimelineElement *) tmp->data;
gint off = _ges_container_get_priority_offset (container, child); gint off = _ges_container_get_priority_offset (container, child);
if (off >= LAYER_HEIGHT) { if (off >= LAYER_HEIGHT) {
GST_ERROR ("%s child %s as a priority offset %d >= LAYER_HEIGHT %d" GST_ERROR ("%s child %s as a priority offset %d >= LAYER_HEIGHT %d"
" ==> clamping it to 0", GES_TIMELINE_ELEMENT_NAME (element), " ==> clamping it to 0", GES_TIMELINE_ELEMENT_NAME (element),
@ -267,24 +288,25 @@ _set_priority (GESTimelineElement * element, guint32 priority)
off = 0; off = 0;
} }
/* We need to remove our current priority from @min_prio /* Want the priority offset 'off' of each child to stay the same with
* as it is the absolute minimum priority @child could have * the new priority. The offset is calculated from
* before we set @container to @priority. * offset = min_priority - child_priority
*/ */
track_element_prio = min_prio - _PRIORITY (container) + priority - off; track_element_prio = min_prio - off;
if (track_element_prio > max_prio) { if (track_element_prio > max_prio) {
GST_WARNING ("%p priority of %i, is outside of the its containing " GST_WARNING ("%p priority of %i, is outside of the its containing "
"layer space. (%d/%d) setting it to the maximum it can be", "layer space. (%d/%d) setting it to the maximum it can be",
container, priority, min_prio - _PRIORITY (container) + priority, container, priority, min_prio, max_prio);
max_prio);
track_element_prio = max_prio; track_element_prio = max_prio;
} }
_set_priority0 (child, track_element_prio); _set_priority0 (child, track_element_prio);
} }
container->children_control_mode = GES_CHILDREN_UPDATE; /* no need to re-sort the container since we maintained the relative
_compute_height (container); * offsets. As such, the height remains the same as well. */
priv->prevent_resort = FALSE;
priv->prevent_priority_offset_update = FALSE;
return TRUE; return TRUE;
} }
@ -341,8 +363,6 @@ _add_child (GESContainer * container, GESTimelineElement * element)
g_return_val_if_fail (GES_IS_TRACK_ELEMENT (element), FALSE); g_return_val_if_fail (GES_IS_TRACK_ELEMENT (element), FALSE);
/* First make sure we work with a sorted list of GESTimelineElement-s */
_ges_container_sort_children (container);
_get_priority_range (container, &min_prio, &max_prio); _get_priority_range (container, &min_prio, &max_prio);
if (ELEMENT_FLAG_IS_SET (element, GES_TRACK_ELEMENT_IS_CORE)) { if (ELEMENT_FLAG_IS_SET (element, GES_TRACK_ELEMENT_IS_CORE)) {
/* NOTE: we are assuming that the core element is **our** core element /* NOTE: we are assuming that the core element is **our** core element
@ -369,6 +389,7 @@ _add_child (GESContainer * container, GESTimelineElement * element)
min_prio + priv->nb_effects); min_prio + priv->nb_effects);
/* changing priorities, and updating their offset */ /* changing priorities, and updating their offset */
priv->prevent_resort = TRUE;
tmp = g_list_nth (GES_CONTAINER_CHILDREN (container), priv->nb_effects); tmp = g_list_nth (GES_CONTAINER_CHILDREN (container), priv->nb_effects);
for (; tmp; tmp = tmp->next) for (; tmp; tmp = tmp->next)
ges_timeline_element_set_priority (GES_TIMELINE_ELEMENT (tmp->data), ges_timeline_element_set_priority (GES_TIMELINE_ELEMENT (tmp->data),
@ -376,6 +397,7 @@ _add_child (GESContainer * container, GESTimelineElement * element)
_set_priority0 (element, min_prio + priv->nb_effects); _set_priority0 (element, min_prio + priv->nb_effects);
priv->nb_effects++; priv->nb_effects++;
priv->prevent_resort = FALSE;
/* no need to call _ges_container_sort_children (container) since /* no need to call _ges_container_sort_children (container) since
* there is no change to the ordering yet (this happens after the * there is no change to the ordering yet (this happens after the
* child is actually added) */ * child is actually added) */
@ -435,39 +457,36 @@ _child_added (GESContainer * container, GESTimelineElement * element)
G_CALLBACK (_child_priority_changed_cb), container); G_CALLBACK (_child_priority_changed_cb), container);
_child_priority_changed_cb (element, NULL, container); _child_priority_changed_cb (element, NULL, container);
_compute_height (container);
} }
static void static void
_child_removed (GESContainer * container, GESTimelineElement * element) _child_removed (GESContainer * container, GESTimelineElement * element)
{ {
GESClipPrivate *priv = GES_CLIP (element)->priv;
g_signal_handlers_disconnect_by_func (element, _child_priority_changed_cb, g_signal_handlers_disconnect_by_func (element, _child_priority_changed_cb,
container); container);
if (!ELEMENT_FLAG_IS_SET (element, GES_TRACK_ELEMENT_IS_CORE) if (!ELEMENT_FLAG_IS_SET (element, GES_TRACK_ELEMENT_IS_CORE)
&& GES_IS_BASE_EFFECT (element)) { && GES_IS_BASE_EFFECT (element)) {
GList *tmp; GList *tmp;
guint32 priority;
GESChildrenControlMode mode = container->children_control_mode;
GST_DEBUG_OBJECT (container, "Resyncing effects priority."); GST_DEBUG_OBJECT (container, "Resyncing effects priority.");
container->children_control_mode = GES_CHILDREN_UPDATE_OFFSETS; /* changing priorities, so preventing a re-sort */
tmp = GES_CONTAINER_CHILDREN (container); priv->prevent_resort = TRUE;
priority = for (tmp = GES_CONTAINER_CHILDREN (container); tmp; tmp = tmp->next) {
ges_timeline_element_get_priority (GES_TIMELINE_ELEMENT (element)); guint32 sibling_prio = GES_TIMELINE_ELEMENT_PRIORITY (tmp->data);
for (; tmp; tmp = tmp->next) { if (sibling_prio > element->priority)
if (ges_timeline_element_get_priority (GES_TIMELINE_ELEMENT (tmp->data)) >
priority) {
ges_timeline_element_set_priority (GES_TIMELINE_ELEMENT (tmp->data), ges_timeline_element_set_priority (GES_TIMELINE_ELEMENT (tmp->data),
GES_TIMELINE_ELEMENT_PRIORITY (tmp->data) - 1); sibling_prio - 1);
}
} }
priv->prevent_resort = FALSE;
container->children_control_mode = mode; /* no need to re-sort the children since the rest keep the same
* relative priorities */
/* height may have changed */
_compute_height (container);
} }
_compute_height (container);
} }
static void static void
@ -1298,7 +1317,9 @@ ges_clip_get_top_effects (GESClip * clip)
ret = g_list_append (ret, gst_object_ref (child)); ret = g_list_append (ret, gst_object_ref (child));
} }
return g_list_sort (ret, (GCompareFunc) element_start_compare); /* list is already sorted by index because the list of children is
* sorted by priority */
return ret;
} }
static gboolean static gboolean
@ -1411,15 +1432,16 @@ ges_clip_set_top_effect_index (GESClip * clip, GESBaseEffect * effect,
return FALSE; return FALSE;
} }
_ges_container_sort_children (GES_CONTAINER (clip)); GST_DEBUG_OBJECT (clip, "Setting top effect %" GST_PTR_FORMAT "priority: %i",
if (_PRIORITY (track_element) < newindex) effect, newindex);
if (current_prio < newindex)
inc = -1; inc = -1;
else else
inc = +1; inc = +1;
GST_DEBUG_OBJECT (clip, "Setting top effect %" GST_PTR_FORMAT "priority: %i", /* prevent a re-sort of the list whilst we are traversing it! */
effect, newindex); clip->priv->prevent_resort = TRUE;
for (tmp = GES_CONTAINER_CHILDREN (clip); tmp; tmp = tmp->next) { for (tmp = GES_CONTAINER_CHILDREN (clip); tmp; tmp = tmp->next) {
GESTrackElement *tmpo = GES_TRACK_ELEMENT (tmp->data); GESTrackElement *tmpo = GES_TRACK_ELEMENT (tmp->data);
guint tck_priority = _PRIORITY (tmpo); guint tck_priority = _PRIORITY (tmpo);
@ -1427,13 +1449,20 @@ ges_clip_set_top_effect_index (GESClip * clip, GESBaseEffect * effect,
if (tmpo == track_element) if (tmpo == track_element)
continue; continue;
if ((inc == +1 && tck_priority >= newindex) || /* only need to change the priority for those between the new and old
(inc == -1 && tck_priority <= newindex)) { * index */
if ((inc == +1 && tck_priority >= newindex && tck_priority < current_prio)
|| (inc == -1 && tck_priority <= newindex
&& tck_priority > current_prio)) {
_set_priority0 (GES_TIMELINE_ELEMENT (tmpo), tck_priority + inc); _set_priority0 (GES_TIMELINE_ELEMENT (tmpo), tck_priority + inc);
} }
} }
_set_priority0 (GES_TIMELINE_ELEMENT (track_element), newindex); _set_priority0 (GES_TIMELINE_ELEMENT (track_element), newindex);
clip->priv->prevent_resort = FALSE;
_ges_container_sort_children (GES_CONTAINER (clip));
/* height should have stayed the same */
return TRUE; return TRUE;
} }