ges-structure-parser: force string types

Force a string type for structure values obtained through parsing a
serialized timeline by inserting a (string) specifier after a '=',
rather than relying on gst_structure_from_string guessing the type.

As such, the functions that extract clocktimes and properties are
modified to accept string value types.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-editing-services/-/merge_requests/123>

5 files changed, 111 insertions, 27 deletions

diff --git a/ges/ges-command-line-formatter.c b/ges/ges-command-line-formatter.c
index a5219658..d315bb69 100644
--- a/ges/ges-command-line-formatter.c
+++ b/ges/ges-command-line-formatter.c
@@ -268,7 +268,7 @@ _convert_to_clocktime (GstStructure * structure, const gchar * name,
 {
   gint res = 1;
   gdouble val;
-  GValue d_val = { 0 };
+  GValue d_val = G_VALUE_INIT, converted = G_VALUE_INIT;
   GstClockTime timestamp;
   const GValue *gvalue = gst_structure_get_value (structure, name);
 
@@ -281,20 +281,41 @@ _convert_to_clocktime (GstStructure * structure, const gchar * name,
   }
 
   if (G_VALUE_TYPE (gvalue) == G_TYPE_STRING) {
-    const gchar *v = g_value_get_string (gvalue);
-    return v && v[0] == 'f';
+    const gchar *val_string = g_value_get_string (gvalue);
+    /* if starts with an 'f', interpret as a frame number, keep as
+     * a string for now */
+    if (val_string && val_string[0] == 'f')
+      return 1;
+    /* else, try convert to a GstClockTime, or a double */
+    g_value_init (&converted, GST_TYPE_CLOCK_TIME);
+    if (!gst_value_deserialize (&converted, val_string)) {
+      g_value_unset (&converted);
+      g_value_init (&converted, G_TYPE_DOUBLE);
+      if (!gst_value_deserialize (&converted, val_string)) {
+        GST_ERROR ("Could not get timestamp for %s by deserializing %s",
+            name, val_string);
+        goto error;
+      }
+    }
+  } else {
+    g_value_init (&converted, G_VALUE_TYPE (gvalue));
+    g_value_copy (gvalue, &converted);
   }
 
-  if (G_VALUE_TYPE (gvalue) == GST_TYPE_CLOCK_TIME)
-    return 1;
+  if (G_VALUE_TYPE (&converted) == GST_TYPE_CLOCK_TIME) {
+    timestamp = g_value_get_uint64 (&converted);
+    goto done;
+  }
 
   g_value_init (&d_val, G_TYPE_DOUBLE);
-  if (!g_value_transform (gvalue, &d_val)) {
-    GST_ERROR ("Could not get timestamp for %s", name);
 
-    return 0;
+  if (!g_value_transform (&converted, &d_val)) {
+    GST_ERROR ("Could not get timestamp for %s", name);
+    goto error;
   }
+
   val = g_value_get_double ((const GValue *) &d_val);
+  g_value_unset (&d_val);
 
   if (val == -1.0)
     timestamp = GST_CLOCK_TIME_NONE;
@@ -303,8 +324,14 @@ _convert_to_clocktime (GstStructure * structure, const gchar * name,
 
 done:
   gst_structure_set (structure, name, G_TYPE_UINT64, timestamp, NULL);
+  g_value_unset (&converted);
 
   return res;
+
+error:
+  g_value_unset (&converted);
+
+  return 0;
 }
 
 static gboolean
diff --git a/ges/ges-structure-parser.c b/ges/ges-structure-parser.c
index aa6752f9..25831be1 100644
--- a/ges/ges-structure-parser.c
+++ b/ges/ges-structure-parser.c
@@ -66,6 +66,15 @@ ges_structure_parser_parse_string (GESStructureParser * self,
 }
 
 void
+ges_structure_parser_parse_value (GESStructureParser * self, const gchar * text)
+{
+  /* text starts with '=' */
+  gchar *val_string = g_strconcat ("=(string)", text + 1, NULL);
+  ges_structure_parser_parse_string (self, val_string, FALSE);
+  g_free (val_string);
+}
+
+void
 ges_structure_parser_parse_default (GESStructureParser * self,
     const gchar * text)
 {
@@ -126,13 +135,15 @@ ges_structure_parser_parse_symbol (GESStructureParser * self,
 
   self->add_comma = FALSE;
   if (!g_ascii_strncasecmp (symbol, "clip", 4))
-    ges_structure_parser_parse_string (self, "clip, uri=", TRUE);
+    ges_structure_parser_parse_string (self, "clip, uri=(string)", TRUE);
   else if (!g_ascii_strncasecmp (symbol, "test-clip", 9))
-    ges_structure_parser_parse_string (self, "test-clip, pattern=", TRUE);
+    ges_structure_parser_parse_string (self, "test-clip, pattern=(string)",
+        TRUE);
   else if (!g_ascii_strncasecmp (symbol, "effect", 6))
-    ges_structure_parser_parse_string (self, "effect, bin-description=", TRUE);
+    ges_structure_parser_parse_string (self, "effect, bin-description=(string)",
+        TRUE);
   else if (!g_ascii_strncasecmp (symbol, "transition", 10))
-    ges_structure_parser_parse_string (self, "transition, type=", TRUE);
+    ges_structure_parser_parse_string (self, "transition, type=(string)", TRUE);
   else if (!g_ascii_strncasecmp (symbol, "title", 5))
     ges_structure_parser_parse_string (self, "title, text=(string)", TRUE);
 }
@@ -153,7 +164,8 @@ ges_structure_parser_parse_setter (GESStructureParser * self,
 
   setter++;
 
-  parsed_setter = g_strdup_printf ("set-property, property=%s, value=", setter);
+  parsed_setter = g_strdup_printf ("set-property, property=(string)%s, "
+      "value=(string)", setter);
   self->add_comma = FALSE;
   ges_structure_parser_parse_string (self, parsed_setter, TRUE);
   g_free (parsed_setter);
diff --git a/ges/ges-structure-parser.h b/ges/ges-structure-parser.h
index 9da56eef..12526556 100644
--- a/ges/ges-structure-parser.h
+++ b/ges/ges-structure-parser.h
@@ -52,6 +52,7 @@ G_GNUC_INTERNAL GType ges_structure_parser_get_type (void) G_GNUC_CONST;
 
 G_GNUC_INTERNAL GError * ges_structure_parser_get_error (GESStructureParser *self);
 G_GNUC_INTERNAL void ges_structure_parser_parse_string (GESStructureParser *self, const gchar *string, gboolean is_symbol);
+G_GNUC_INTERNAL void ges_structure_parser_parse_value (GESStructureParser *self, const gchar *string);
 G_GNUC_INTERNAL void ges_structure_parser_parse_default (GESStructureParser *self, const gchar *text);
 G_GNUC_INTERNAL void ges_structure_parser_parse_whitespace (GESStructureParser *self);
 G_GNUC_INTERNAL void ges_structure_parser_parse_symbol (GESStructureParser *self, const gchar *symbol);
diff --git a/ges/ges-structured-interface.c b/ges/ges-structured-interface.c
index 94f6c6d4..b96d1a90 100644
--- a/ges/ges-structured-interface.c
+++ b/ges/ges-structured-interface.c
@@ -706,8 +706,12 @@ _ges_set_child_property_from_struct (GESTimeline * timeline,
     GstStructure * structure, GError ** error)
 {
   const GValue *value;
+  GValue prop_value = G_VALUE_INIT;
+  gboolean prop_value_set = FALSE;
   GESTimelineElement *element;
   const gchar *property_name, *element_name;
+  gchar *serialized;
+  gboolean res;
 
   const gchar *valid_fields[] =
       { "element-name", "property", "value", "project-uri", NULL };
@@ -768,11 +772,41 @@ _ges_set_child_property_from_struct (GESTimeline * timeline,
 
   value = gst_structure_get_value (structure, "value");
 
-  g_print ("%s Setting %s property to %s\n", element->name, property_name,
-      gst_value_serialize (value));
+  if (G_VALUE_TYPE (value) == G_TYPE_STRING) {
+    GParamSpec *pspec;
+    if (ges_timeline_element_lookup_child (element, property_name, NULL,
+            &pspec)) {
+      GType p_type = pspec->value_type;
+      g_param_spec_unref (pspec);
+      if (p_type != G_TYPE_STRING) {
+        const gchar *val_string = g_value_get_string (value);
+        g_value_init (&prop_value, p_type);
+        if (!gst_value_deserialize (&prop_value, val_string)) {
+          *error = g_error_new (GES_ERROR, 0, "Could not set the property %s "
+              "because the value %s could not be deserialized to the %s type",
+              property_name, val_string, g_type_name (p_type));
+          return FALSE;
+        }
+        prop_value_set = TRUE;
+      }
+    }
+    /* else, let the setter fail below */
+  }
+
+  if (!prop_value_set) {
+    g_value_init (&prop_value, G_VALUE_TYPE (value));
+    g_value_copy (value, &prop_value);
+  }
+
+  serialized = gst_value_serialize (&prop_value);
+  GST_INFO_OBJECT (element, "Setting property %s to %s\n", property_name,
+      serialized);
+  g_free (serialized);
 
-  if (!ges_timeline_element_set_child_property (element, property_name,
-          (GValue *) value)) {
+  res = ges_timeline_element_set_child_property (element, property_name,
+      &prop_value);
+  g_value_unset (&prop_value);
+  if (!res) {
     guint n_specs, i;
     GParamSpec **specs =
         ges_timeline_element_list_children_properties (element, &n_specs);
diff --git a/ges/parse.l b/ges/parse.l
index 1581d940..d4428204 100644
--- a/ges/parse.l
+++ b/ges/parse.l
@@ -11,17 +11,27 @@
 %option noinput
 %option nounistd
 
-CLIP 		[ ]+\+clip[ ]+
+CLIP            [ ]+\+clip[ ]+
 TEST_CLIP       [ ]+\+test-clip[ ]+
-TRANSITION	[ ]+\+transition[ ]+
-EFFECT		[ ]+\+effect[ ]+
-TITLE		[ ]+\+title[ ]+
+TRANSITION      [ ]+\+transition[ ]+
+EFFECT          [ ]+\+effect[ ]+
+TITLE           [ ]+\+title[ ]+
 
-SETTER		[ ]+set-[^ ]+[ ]+
+SETTER          [ ]+set-[^ ]+[ ]+
+
+STRING          \"(\\.|[^"])*\"
+/* A value string, as understood by gst_structure_from_string
+ * Characters are from GST_ASCII_IS_STRING
+ * NOTE: character set is *not* supposed to be locale dependent */
+VALUE           {STRING}|([abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_+/:.-]+)
 
 %%
 
-\"(\\.|[^"])*\"  {
+={VALUE}        {
+		ges_structure_parser_parse_value (yyextra, yytext);
+}
+
+{STRING}        {
 		ges_structure_parser_parse_string (yyextra, yytext, FALSE);
 }
 
@@ -29,15 +39,15 @@ SETTER		[ ]+set-[^ ]+[ ]+
 		ges_structure_parser_parse_symbol (yyextra, yytext);
 }
 
-{SETTER}	 {
-	ges_structure_parser_parse_setter (yyextra, yytext);
+{SETTER}        {
+		ges_structure_parser_parse_setter (yyextra, yytext);
 }
 
-[ \t\n]+ 	{
+[ \t\n]+        {
 		ges_structure_parser_parse_whitespace (yyextra);
 }
 
-. 		{
+.               {
 		/* add everything else */
 		ges_structure_parser_parse_default (yyextra, yytext);
 }