From e37d8e768fad09acf43215d9c33c4fab32de419b Mon Sep 17 00:00:00 2001 From: Thibault Saunier Date: Mon, 23 Feb 2015 17:41:59 +0100 Subject: [PATCH] ges: command-line-formatter: Properly error out on invalid arguments --- ges/ges-command-line-formatter.c | 20 ++++-- ges/ges-structure-parser.c | 46 ++++++++++++- ges/ges-structure-parser.h | 3 + ges/ges-structured-interface.c | 110 ++++++++++++++++++++++++++++--- ges/ges-structured-interface.h | 3 +- ges/ges-validate.c | 5 +- tools/ges-launch.c | 10 ++- 7 files changed, 180 insertions(+), 17 deletions(-) diff --git a/ges/ges-command-line-formatter.c b/ges/ges-command-line-formatter.c index 138d376c38..f4ed77ac14 100644 --- a/ges/ges-command-line-formatter.c +++ b/ges/ges-command-line-formatter.c @@ -270,8 +270,18 @@ _load (GESFormatter * self, GESTimeline * timeline, const gchar * string, { guint i; GList *tmp; + GError *err; GESStructureParser *parser = _parse_structures (string); + err = ges_structure_parser_get_error (parser); + + if (err) { + if (error) + *error = err; + + return FALSE; + } + g_object_set (timeline, "auto-transition", TRUE, NULL); if (!(ges_timeline_add_track (timeline, GES_TRACK (ges_video_track_new ())))) goto fail; @@ -283,10 +293,8 @@ _load (GESFormatter * self, GESTimeline * timeline, const gchar * string, * ready to start using it... by solely working with the layer !*/ for (tmp = parser->structures; tmp; tmp = tmp->next) { const gchar *name = gst_structure_get_name (tmp->data); - GError *error = NULL; - if (g_str_has_prefix (name, "set-")) { - EXEC (_set_child_property, tmp->data, &error); + EXEC (_set_child_property, tmp->data, &err); continue; } @@ -296,7 +304,7 @@ _load (GESFormatter * self, GESTimeline * timeline, const gchar * string, || (strlen (name) == 1 && *name == timeline_parsing_options[i].short_name)) { EXEC (((ActionFromStructureFunc) timeline_parsing_options[i].arg_data), - tmp->data, &error); + tmp->data, &err); } } } @@ -307,6 +315,10 @@ _load (GESFormatter * self, GESTimeline * timeline, const gchar * string, fail: gst_object_unref (parser); + if (err) { + if (error) + *error = err; + } return FALSE; } diff --git a/ges/ges-structure-parser.c b/ges/ges-structure-parser.c index 6f6dcd870e..e0dfa1fc22 100644 --- a/ges/ges-structure-parser.c +++ b/ges/ges-structure-parser.c @@ -20,6 +20,8 @@ #include "ges-structure-parser.h" +#include + G_DEFINE_TYPE (GESStructureParser, ges_structure_parser, G_TYPE_OBJECT); static void @@ -27,9 +29,24 @@ ges_structure_parser_init (GESStructureParser * self) { } +static void +_finalize (GObject * self) +{ + GList *tmp; + + for (tmp = ((GESStructureParser *) self)->structures; tmp; tmp = tmp->next) { + gst_structure_free (tmp->data); + } + + for (tmp = ((GESStructureParser *) self)->wrong_strings; tmp; tmp = tmp->next) { + g_free (tmp->data); + } +} + static void ges_structure_parser_class_init (GESStructureParserClass * klass) { + G_OBJECT_CLASS (klass)->finalize = _finalize; } void @@ -78,7 +95,10 @@ _finish_structure (GESStructureParser * self) gst_structure_new_from_string (self->current_string); if (structure == NULL) { - GST_ERROR ("Error creating structure from %s", self->current_string); + GST_ERROR ("Could not parse %s", self->current_string); + + self->wrong_strings = g_list_append (self->wrong_strings, + g_strdup (self->current_string)); return; } @@ -134,3 +154,27 @@ ges_structure_parser_new (void) { return (g_object_new (GES_TYPE_STRUCTURE_PARSER, NULL)); } + +GError * +ges_structure_parser_get_error (GESStructureParser * self) +{ + GList *tmp; + GError *error = NULL; + GString *msg = NULL; + + if (self->wrong_strings == NULL) + return NULL; + + msg = g_string_new ("Could not parse: "); + + + for (tmp = self->wrong_strings; tmp; tmp = tmp->next) { + g_string_append_printf (msg, " %s", (gchar *) tmp->data); + } + + error = g_error_new_literal (GES_ERROR, 0, msg->str); + g_string_free (msg, TRUE); + + GST_ERROR ("BoOOOM "); + return error; +} diff --git a/ges/ges-structure-parser.h b/ges/ges-structure-parser.h index 9f1e265ddf..2e1954e0ae 100644 --- a/ges/ges-structure-parser.h +++ b/ges/ges-structure-parser.h @@ -35,6 +35,8 @@ struct _GESStructureParser GObject parent; GList *structures; + GList *wrong_strings; + /*< private > */ gchar *current_string; gboolean add_comma; @@ -48,6 +50,7 @@ struct _GESStructureParserClass GType ges_structure_parser_get_type (void) G_GNUC_CONST; +GError * ges_structure_parser_get_error (GESStructureParser *self); void ges_structure_parser_parse_string (GESStructureParser *self, const gchar *string, gboolean is_symbol); void ges_structure_parser_parse_default (GESStructureParser *self, const gchar *text); void ges_structure_parser_parse_whitespace (GESStructureParser *self); diff --git a/ges/ges-structured-interface.c b/ges/ges-structured-interface.c index 9b2264c99f..3a940cb2bc 100644 --- a/ges/ges-structured-interface.c +++ b/ges/ges-structured-interface.c @@ -19,10 +19,70 @@ */ #include "ges-structured-interface.h" +#include + #define LAST_CONTAINER_QDATA g_quark_from_string("ges-structured-last-container") #define LAST_CHILD_QDATA g_quark_from_string("ges-structured-last-child") +typedef struct +{ + const gchar **fields; + GList *invalid_fields; +} FieldsError; + +static gboolean +_check_field (GQuark field_id, const GValue * value, FieldsError * fields_error) +{ + guint i; + const gchar *field = g_quark_to_string (field_id); + + for (i = 0; fields_error->fields[i]; i++) { + if (g_strcmp0 (fields_error->fields[i], field) == 0) { + + return TRUE; + } + } + + fields_error->invalid_fields = + g_list_append (fields_error->invalid_fields, (gpointer) field); + + return TRUE; +} + +static gboolean +_check_fields (GstStructure * structure, FieldsError fields_error, + GError ** error) +{ + gst_structure_foreach (structure, + (GstStructureForeachFunc) _check_field, &fields_error); + + if (fields_error.invalid_fields) { + GList *tmp; + const gchar *struct_name = gst_structure_get_name (structure); + GString *msg = g_string_new (NULL); + + g_string_append_printf (msg, "Unknown propert%s in %s%s:", + g_list_length (fields_error.invalid_fields) > 1 ? "ies" : "y", + strlen (struct_name) > 1 ? "--" : "-", + gst_structure_get_name (structure)); + + for (tmp = fields_error.invalid_fields; tmp; tmp = tmp->next) + g_string_append_printf (msg, " %s", (gchar *) tmp->data); + + if (error) + *error = g_error_new_literal (GES_ERROR, 0, msg->str); + GST_ERROR ("%s", msg->str); + + g_string_free (msg, TRUE); + + return FALSE; + } + + return TRUE; +} + + gboolean _ges_add_remove_keyframe_from_struct (GESTimeline * timeline, GstStructure * structure, GError ** error) @@ -37,6 +97,15 @@ _ges_add_remove_keyframe_from_struct (GESTimeline * timeline, gboolean ret = FALSE; + const gchar *valid_fields[] = + { "element-name", "property-name", "value", "timestamp", + NULL + }; + + FieldsError fields_error = { valid_fields, NULL }; + + if (!_check_fields (structure, fields_error, error)) + return FALSE; if (!gst_structure_get (structure, "element-name", G_TYPE_STRING, &element_name, @@ -112,17 +181,16 @@ done: GESAsset * _ges_get_asset_from_timeline (GESTimeline * timeline, GType type, - const gchar * id) + const gchar * id, GError ** error) { GESAsset *asset; - GError *error = NULL; GESProject *project = ges_timeline_get_project (timeline); - asset = ges_project_create_asset_sync (project, id, type, &error); - if (!asset || error) { + asset = ges_project_create_asset_sync (project, id, type, error); + if (!asset || (error && *error)) { GST_ERROR ("There was an error requesting the asset with id %s and type %s (%s)", - id, g_type_name (type), error ? error->message : "unknown"); + id, g_type_name (type), (*error) ? (*error)->message : "unknown"); return NULL; } @@ -177,6 +245,7 @@ done: } \ } G_STMT_END + gboolean _ges_add_clip_from_struct (GESTimeline * timeline, GstStructure * structure, GError ** error) @@ -190,9 +259,20 @@ _ges_add_clip_from_struct (GESTimeline * timeline, GstStructure * structure, const gchar *type_string; GType type; gboolean res = FALSE; + GstClockTime duration = 1 * GST_SECOND, inpoint = 0, start = GST_CLOCK_TIME_NONE; + const gchar *valid_fields[] = + { "asset-id", "name", "layer-priority", "layer", "type", + "start", "inpoint", "duration", NULL + }; + + FieldsError fields_error = { valid_fields, NULL }; + + if (!_check_fields (structure, fields_error, error)) + return FALSE; + GET_AND_CHECK ("asset-id", G_TYPE_STRING, &asset_id); TRY_GET ("name", G_TYPE_STRING, &name, NULL); @@ -210,7 +290,7 @@ _ges_add_clip_from_struct (GESTimeline * timeline, GstStructure * structure, goto beach; } - asset = _ges_get_asset_from_timeline (timeline, type, asset_id); + asset = _ges_get_asset_from_timeline (timeline, type, asset_id, error); if (!asset) { res = FALSE; @@ -275,6 +355,14 @@ _ges_container_add_child_from_struct (GESTimeline * timeline, const gchar *container_name, *child_name, *child_type, *id; gboolean res = TRUE; + const gchar *valid_fields[] = { "container-name", "asset-id", + "child-type", "child-name", NULL + }; + + FieldsError fields_error = { valid_fields, NULL }; + + if (!_check_fields (structure, fields_error, error)) + return FALSE; container_name = gst_structure_get_string (structure, "container-name"); @@ -293,11 +381,10 @@ _ges_container_add_child_from_struct (GESTimeline * timeline, if (id && child_type) { asset = _ges_get_asset_from_timeline (timeline, g_type_from_name (child_type), - id); + id, error); if (asset == NULL) { res = FALSE; - g_error_new (GES_ERROR, 0, "Could not find asset: %s", id); goto beach; } @@ -349,6 +436,13 @@ _ges_set_child_property_from_struct (GESTimeline * timeline, GESTimelineElement *element; const gchar *property_name, *element_name; + const gchar *valid_fields[] = { "element-name", "property", "value", NULL }; + + FieldsError fields_error = { valid_fields, NULL }; + + if (!_check_fields (structure, fields_error, error)) + return FALSE; + element_name = gst_structure_get_string (structure, "element-name"); if (element_name == NULL) element = g_object_get_qdata (G_OBJECT (timeline), LAST_CHILD_QDATA); diff --git a/ges/ges-structured-interface.h b/ges/ges-structured-interface.h index 9c834610e7..c25f972883 100644 --- a/ges/ges-structured-interface.h +++ b/ges/ges-structured-interface.h @@ -49,7 +49,8 @@ _ges_set_child_property_from_struct (GESTimeline * timeline, G_GNUC_INTERNAL GESAsset * _ges_get_asset_from_timeline (GESTimeline * timeline, GType type, - const gchar * id); + const gchar * id, + GError **error); G_GNUC_INTERNAL GESLayer * _ges_get_layer_by_priority (GESTimeline * timeline, gint priority); diff --git a/ges/ges-validate.c b/ges/ges-validate.c index f256fa0bea..f74a43ea2f 100644 --- a/ges/ges-validate.c +++ b/ges/ges-validate.c @@ -124,7 +124,7 @@ _add_asset (GstValidateScenario * scenario, GstValidateAction * action) goto beach; } - asset = _ges_get_asset_from_timeline (timeline, type, id); + asset = _ges_get_asset_from_timeline (timeline, type, id, NULL); if (!asset) { res = FALSE; @@ -411,7 +411,8 @@ _set_asset_on_element (GstValidateScenario * scenario, gst_validate_printf (action, "Setting asset %s on element %s\n", id, element_name); - asset = _ges_get_asset_from_timeline (timeline, G_OBJECT_TYPE (element), id); + asset = _ges_get_asset_from_timeline (timeline, G_OBJECT_TYPE (element), id, + NULL); if (asset == NULL) { res = FALSE; GST_ERROR ("Could not find asset: %s", id); diff --git a/tools/ges-launch.c b/tools/ges-launch.c index 91b5c2bb9e..e2f23e54f0 100644 --- a/tools/ges-launch.c +++ b/tools/ges-launch.c @@ -251,6 +251,8 @@ create_timeline (const gchar * serialized_timeline, const gchar * proj_uri, GESTimeline *timeline; GESProject *project; + GError *error = NULL; + if (proj_uri != NULL) { project = ges_project_new (proj_uri); } else if (scenario == NULL) { @@ -263,7 +265,13 @@ create_timeline (const gchar * serialized_timeline, const gchar * proj_uri, G_CALLBACK (error_loading_asset_cb), NULL); g_signal_connect (project, "loaded", G_CALLBACK (project_loaded_cb), NULL); - timeline = GES_TIMELINE (ges_asset_extract (GES_ASSET (project), NULL)); + timeline = GES_TIMELINE (ges_asset_extract (GES_ASSET (project), &error)); + + if (error) { + g_error ("Could not create timeline, error: %s", error->message); + + return NULL; + } return timeline; }