formatter: Serialize Transition border and invert properties

Marking them as children properties and properly allow serializing
clips children properties.

This doesn't handle several TrackElement of a same type with
different property values but this require more worked already
marked as fixme to allow specifying full path of elements in the
children properties API.

See https://gitlab.gnome.org/GNOME/pitivi/issues/1687

6 files changed, 102 insertions, 20 deletions

diff --git a/ges/ges-base-xml-formatter.c b/ges/ges-base-xml-formatter.c
index 1172dfb3..22986bad 100644
--- a/ges/ges-base-xml-formatter.c
+++ b/ges/ges-base-xml-formatter.c
@@ -77,6 +77,7 @@ typedef struct PendingClip
   GESLayer *layer;
 
   GstStructure *properties;
+  GstStructure *children_properties;
   gchar *metadatas;
 
   GList *effects;
@@ -503,26 +504,26 @@ _loading_done_cb (GESFormatter * self)
 
 static gboolean
 _set_child_property (GQuark field_id, const GValue * value,
-    GESTrackElement * effect)
+    GESTimelineElement * tlelement)
 {
   GParamSpec *pspec;
-  GstElement *element;
+  GObject *object;
 
   /* FIXME: error handling? */
-  if (!ges_track_element_lookup_child (effect,
-          g_quark_to_string (field_id), &element, &pspec)) {
+  if (!ges_timeline_element_lookup_child (tlelement,
+          g_quark_to_string (field_id), &object, &pspec)) {
 #ifndef GST_DISABLE_GST_DEBUG
     gchar *tmp = gst_value_serialize (value);
-    GST_ERROR_OBJECT (effect, "Could not set %s=%s",
+    GST_ERROR_OBJECT (tlelement, "Could not set %s=%s",
         g_quark_to_string (field_id), tmp);
     g_free (tmp);
 #endif
     return TRUE;
   }
 
-  g_object_set_property (G_OBJECT (element), pspec->name, value);
+  g_object_set_property (G_OBJECT (object), pspec->name, value);
   g_param_spec_unref (pspec);
-  gst_object_unref (element);
+  gst_object_unref (object);
   return TRUE;
 }
 
@@ -538,7 +539,7 @@ _add_object_to_layer (GESBaseXmlFormatterPrivate * priv, const gchar * id,
     GESLayer * layer, GESAsset * asset, GstClockTime start,
     GstClockTime inpoint, GstClockTime duration,
     GESTrackType track_types, const gchar * metadatas,
-    GstStructure * properties)
+    GstStructure * properties, GstStructure * children_properties)
 {
   GESClip *clip = ges_layer_add_asset (layer,
       asset, start, inpoint, duration, track_types);
@@ -558,6 +559,10 @@ _add_object_to_layer (GESBaseXmlFormatterPrivate * priv, const gchar * id,
     gst_structure_foreach (properties,
         (GstStructureForeachFunc) set_property_foreach, clip);
 
+  if (children_properties)
+    gst_structure_foreach (children_properties,
+        (GstStructureForeachFunc) _set_child_property, clip);
+
   g_hash_table_insert (priv->containers, g_strdup (id), gst_object_ref (clip));
   return clip;
 }
@@ -758,7 +763,7 @@ new_asset_cb (GESAsset * source, GAsyncResult * res, PendingAsset * passet)
     clip =
         _add_object_to_layer (priv, pend->id, pend->layer, asset,
         pend->start, pend->inpoint, pend->duration, pend->track_types,
-        pend->metadatas, pend->properties);
+        pend->metadatas, pend->properties, pend->children_properties);
 
     if (clip == NULL)
       continue;
@@ -944,6 +949,7 @@ ges_base_xml_formatter_add_clip (GESBaseXmlFormatter * self,
     const gchar * id, const char *asset_id, GType type, GstClockTime start,
     GstClockTime inpoint, GstClockTime duration,
     guint layer_prio, GESTrackType track_types, GstStructure * properties,
+    GstStructure * children_properties,
     const gchar * metadatas, GError ** error)
 {
   GESAsset *asset;
@@ -999,6 +1005,8 @@ ges_base_xml_formatter_add_clip (GESBaseXmlFormatter * self,
     pclip->layer = gst_object_ref (entry->layer);
 
     pclip->properties = properties ? gst_structure_copy (properties) : NULL;
+    pclip->children_properties =
+        properties ? gst_structure_copy (children_properties) : NULL;
     pclip->metadatas = g_strdup (metadatas);
 
     /* Add the new pending object to the hashtable */
@@ -1013,7 +1021,8 @@ ges_base_xml_formatter_add_clip (GESBaseXmlFormatter * self,
   }
 
   nclip = _add_object_to_layer (priv, id, entry->layer,
-      asset, start, inpoint, duration, track_types, metadatas, properties);
+      asset, start, inpoint, duration, track_types, metadatas, properties,
+      children_properties);
 
   if (!nclip)
     return;
diff --git a/ges/ges-container.c b/ges/ges-container.c
index 543b03d3..a048a002 100644
--- a/ges/ges-container.c
+++ b/ges/ges-container.c
@@ -281,9 +281,8 @@ _lookup_child (GESTimelineElement * self, const gchar * prop_name,
 {
   GList *tmp;
 
-  /* FIXME Implement a synthax to precisely get properties by path */
+  /* FIXME Implement a syntax to precisely get properties by path */
   for (tmp = GES_CONTAINER_CHILDREN (self); tmp; tmp = tmp->next) {
-
     if (ges_timeline_element_lookup_child (tmp->data, prop_name, child, pspec))
       return TRUE;
   }
diff --git a/ges/ges-internal.h b/ges/ges-internal.h
index 502b11ba..6a363821 100644
--- a/ges/ges-internal.h
+++ b/ges/ges-internal.h
@@ -232,6 +232,7 @@ G_GNUC_INTERNAL void ges_base_xml_formatter_add_clip (GESBaseXmlFormatter * self
                                                                  guint layer_prio,
                                                                  GESTrackType track_types,
                                                                  GstStructure *properties,
+                                                                 GstStructure * children_properties,
                                                                  const gchar *metadatas,
                                                                  GError **error);
 G_GNUC_INTERNAL void ges_base_xml_formatter_add_asset        (GESBaseXmlFormatter * self,
diff --git a/ges/ges-transition-clip.c b/ges/ges-transition-clip.c
index ef4754ee..b92e60da 100644
--- a/ges/ges-transition-clip.c
+++ b/ges/ges-transition-clip.c
@@ -227,6 +227,22 @@ ges_transition_clip_set_property (GObject * object,
   }
 }
 
+static gboolean
+_lookup_child (GESTimelineElement * self, const gchar * prop_name,
+    GObject ** child, GParamSpec ** pspec)
+{
+  GESTimelineElementClass *element_klass =
+      g_type_class_peek (GES_TYPE_TIMELINE_ELEMENT);
+
+  /* Bypass the container implementation as we handle children properties directly */
+  /* FIXME Implement a syntax to precisely get properties by path */
+  if (element_klass->lookup_child (self, prop_name, child, pspec))
+    return TRUE;
+
+  return FALSE;
+}
+
+
 static void
 ges_transition_clip_class_init (GESTransitionClipClass * klass)
 {
@@ -251,6 +267,7 @@ ges_transition_clip_class_init (GESTransitionClipClass * klass)
           GES_VIDEO_STANDARD_TRANSITION_TYPE_CROSSFADE,
           G_PARAM_READWRITE | G_PARAM_CONSTRUCT));
 
+  GES_TIMELINE_ELEMENT_CLASS (klass)->lookup_child = _lookup_child;
   container_class->child_added = _child_added;
   container_class->child_removed = _child_removed;
 
@@ -287,9 +304,16 @@ _child_added (GESContainer * container, GESTimelineElement * element)
   GESTransitionClipPrivate *priv = GES_TRANSITION_CLIP (container)->priv;
 
   if (GES_IS_VIDEO_TRANSITION (element)) {
+    GObjectClass *eklass = G_OBJECT_GET_CLASS (element);
+
     GST_DEBUG_OBJECT (container, "%" GST_PTR_FORMAT " added", element);
     priv->video_transitions =
         g_slist_prepend (priv->video_transitions, gst_object_ref (element));
+
+    ges_timeline_element_add_child_property (GES_TIMELINE_ELEMENT (container),
+        g_object_class_find_property (eklass, "invert"), G_OBJECT (element));
+    ges_timeline_element_add_child_property (GES_TIMELINE_ELEMENT (container),
+        g_object_class_find_property (eklass, "border"), G_OBJECT (element));
   }
 }
 
diff --git a/ges/ges-xml-formatter.c b/ges/ges-xml-formatter.c
index ea54b722..5533e772 100644
--- a/ges/ges-xml-formatter.c
+++ b/ges/ges-xml-formatter.c
@@ -465,13 +465,13 @@ _parse_clip (GMarkupParseContext * context,
     const gchar ** attribute_values, GESXmlFormatter * self, GError ** error)
 {
   GType type;
-  GstStructure *props = NULL;
+  GstStructure *props = NULL, *children_props = NULL;
   GESTrackType track_types;
   GstClockTime start, inpoint = 0, duration, layer_prio;
 
   const gchar *strid, *asset_id, *strstart, *strin, *strduration, *strrate,
       *strtrack_types, *strtype, *metadatas = NULL, *properties =
-      NULL, *strlayer_prio;
+      NULL, *children_properties = NULL, *strlayer_prio;
 
   if (!g_markup_collect_attributes (element_name, attribute_names,
           attribute_values, error,
@@ -483,6 +483,7 @@ _parse_clip (GMarkupParseContext * context,
           G_MARKUP_COLLECT_STRING, "track-types", &strtrack_types,
           G_MARKUP_COLLECT_STRING, "layer-priority", &strlayer_prio,
           COLLECT_STR_OPT, "properties", &properties,
+          COLLECT_STR_OPT, "children-properties", &children_properties,
           COLLECT_STR_OPT, "metadatas", &metadatas,
           COLLECT_STR_OPT, "rate", &strrate,
           COLLECT_STR_OPT, "inpoint", &strin, G_MARKUP_COLLECT_INVALID)) {
@@ -521,9 +522,15 @@ _parse_clip (GMarkupParseContext * context,
       goto wrong_properties;
   }
 
+  if (children_properties) {
+    children_props = gst_structure_from_string (children_properties, NULL);
+    if (children_props == NULL)
+      goto wrong_children_properties;
+  }
+
   ges_base_xml_formatter_add_clip (GES_BASE_XML_FORMATTER (self),
       strid, asset_id, type, start, inpoint, duration, layer_prio,
-      track_types, props, metadatas, error);
+      track_types, props, children_props, metadatas, error);
   if (props)
     gst_structure_free (props);
 
@@ -536,6 +543,13 @@ wrong_properties:
       element_name, asset_id, properties);
   return;
 
+wrong_children_properties:
+  g_set_error (error, G_MARKUP_ERROR,
+      G_MARKUP_ERROR_INVALID_CONTENT,
+      "element '%s', Clip %s children properties '%s', could no be deserialized",
+      element_name, asset_id, children_properties);
+  return;
+
 convertion_failed:
   g_set_error (error, G_MARKUP_ERROR,
       G_MARKUP_ERROR_INVALID_CONTENT,
@@ -1001,14 +1015,14 @@ _save_tracks (GESXmlFormatter * self, GString * str, GESTimeline * timeline)
 }
 
 static inline void
-_save_children_properties (GString * str, GESTrackElement * trackelement)
+_save_children_properties (GString * str, GESTimelineElement * element)
 {
   GstStructure *structure;
   GParamSpec **pspecs, *spec;
   guint i, n_props;
   gchar *struct_str;
 
-  pspecs = ges_track_element_list_children_properties (trackelement, &n_props);
+  pspecs = ges_timeline_element_list_children_properties (element, &n_props);
 
   structure = gst_structure_new_empty ("properties");
   for (i = 0; i < n_props; i++) {
@@ -1021,7 +1035,7 @@ _save_children_properties (GString * str, GESTrackElement * trackelement)
           spec->name);
 
       _init_value_from_spec_for_serialization (&val, spec);
-      ges_track_element_get_child_property_by_pspec (trackelement, spec, &val);
+      ges_timeline_element_get_child_property_by_pspec (element, spec, &val);
       gst_structure_set_value (structure, spec_name, &val);
 
       g_free (spec_name);
@@ -1146,7 +1160,7 @@ _save_effect (GString * str, guint clip_id, GESTrackElement * trackelement,
   g_free (properties);
   g_free (metas);
 
-  _save_children_properties (str, trackelement);
+  _save_children_properties (str, GES_TIMELINE_ELEMENT (trackelement));
   append_escaped (str, g_markup_printf_escaped (">\n"));
 
   _save_keyframes (str, trackelement, -1);
@@ -1203,17 +1217,23 @@ _save_layers (GESXmlFormatter * self, GString * str, GESTimeline * timeline)
           g_markup_printf_escaped ("        <clip id='%i' asset-id='%s'"
               " type-name='%s' layer-priority='%i' track-types='%i' start='%"
               G_GUINT64_FORMAT "' duration='%" G_GUINT64_FORMAT "' inpoint='%"
-              G_GUINT64_FORMAT "' rate='%d' properties='%s' >\n",
+              G_GUINT64_FORMAT "' rate='%d' properties='%s' ",
               priv->nbelements, extractable_id,
               g_type_name (G_OBJECT_TYPE (clip)), priority,
               ges_clip_get_supported_formats (clip), _START (clip),
               _DURATION (clip), _INPOINT (clip), 0, properties));
+
+      if (GES_IS_TRANSITION_CLIP (clip))
+        _save_children_properties (str, GES_TIMELINE_ELEMENT (clip));
+      g_string_append (str, ">\n");
+
       g_free (extractable_id);
       g_free (properties);
 
       g_hash_table_insert (self->priv->element_id, clip,
           GINT_TO_POINTER (priv->nbelements));
 
+
       /* Effects must always be serialized in the right priority order.
        * List order is guaranteed by the fact that ges_clip_get_top_effects
        * sorts the effects. */
@@ -1223,6 +1243,7 @@ _save_layers (GESXmlFormatter * self, GString * str, GESTimeline * timeline)
             GES_TRACK_ELEMENT (tmpeffect->data), timeline);
       }
 
+
       tracks = ges_timeline_get_tracks (timeline);
 
       for (tmptrackelement = GES_CONTAINER_CHILDREN (clip); tmptrackelement;
diff --git a/tests/check/python/test_clip.py b/tests/check/python/test_clip.py
index b62e1411..ad825921 100644
--- a/tests/check/python/test_clip.py
+++ b/tests/check/python/test_clip.py
@@ -19,6 +19,8 @@
 
 from . import overrides_hack
 
+import tempfile
+
 import gi
 gi.require_version("Gst", "1.0")
 gi.require_version("GES", "1.0")
@@ -68,6 +70,32 @@ class TestCopyPaste(unittest.TestCase):
         self.assertEqual(len(self.layer.get_clips()), 2)
 
 
+class TestTransitionClip(unittest.TestCase):
+
+    def test_serialize_invert(self):
+        timeline = GES.Timeline.new()
+        timeline.add_track(GES.VideoTrack.new())
+        layer = timeline.append_layer()
+
+        clip1 = GES.TransitionClip.new_for_nick("crossfade")
+        clip1.props.duration = Gst.SECOND
+        self.assertTrue(layer.add_clip(clip1))
+
+        vtransition, = clip1.children
+        vtransition.set_inverted(True)
+        self.assertEqual(vtransition.props.invert, True)
+
+        with tempfile.NamedTemporaryFile() as tmpxges:
+            uri = Gst.filename_to_uri(tmpxges.name)
+            timeline.save_to_uri(uri, None, True)
+
+            timeline = GES.Timeline.new_from_uri(uri)
+            self.assertIsNotNone(timeline)
+            layer, = timeline.get_layers()
+            clip, = layer.get_clips()
+            vtransition, = clip.children
+            self.assertEqual(vtransition.props.invert, True)
+
 class TestTitleClip(unittest.TestCase):
 
     def testSetColor(self):