From 6f9e6d35866f0e5474a6d84f8046b2fc77c71bd5 Mon Sep 17 00:00:00 2001 From: Thibault Saunier Date: Thu, 11 Jul 2019 15:54:27 -0400 Subject: [PATCH] formatter: Enhance error reporting And add a "loading-error" signal in GESProject so we can report issue when loading async elements for the timeline. --- ges/ges-base-xml-formatter.c | 42 ++++++++++++++++++++++++------------ ges/ges-internal.h | 3 ++- ges/ges-pitivi-formatter.c | 4 ++-- ges/ges-project.c | 24 ++++++++++++++++++++- plugins/ges/gesdemux.c | 38 ++++++++++++++++++++++++++------ tools/ges-launcher.c | 12 +++++++++++ 6 files changed, 99 insertions(+), 24 deletions(-) diff --git a/ges/ges-base-xml-formatter.c b/ges/ges-base-xml-formatter.c index f4744b5673..42ed1464f0 100644 --- a/ges/ges-base-xml-formatter.c +++ b/ges/ges-base-xml-formatter.c @@ -88,6 +88,8 @@ struct _GESBaseXmlFormatterPrivate /* List of asset waited to be created */ GList *pending_assets; + GError *asset_error; + /* current track element */ GESTrackElement *current_track_element; @@ -273,6 +275,7 @@ _load_from_uri (GESFormatter * self, GESTimeline * timeline, const gchar * uri, { GESBaseXmlFormatterPrivate *priv = _GET_PRIV (self); + GST_INFO_OBJECT (self, "Loading %s in %" GST_PTR_FORMAT, uri, timeline); ges_timeline_set_auto_transition (timeline, FALSE); priv->parsecontext = @@ -431,7 +434,7 @@ ges_base_xml_formatter_class_init (GESBaseXmlFormatterClass * self_class) self_class->save = NULL; - GST_DEBUG_CATEGORY_INIT (base_xml_formatter, "base-xml-formatter", + GST_DEBUG_CATEGORY_INIT (base_xml_formatter, "gesbasexmlformatter", GST_DEBUG_FG_BLUE | GST_DEBUG_BOLD, "Base XML Formatter"); } @@ -490,9 +493,9 @@ static void _loading_done (GESFormatter * self) { GList *assets, *tmp; + GError *error = NULL; GESBaseXmlFormatterPrivate *priv = GES_BASE_XML_FORMATTER (self)->priv; - if (priv->parsecontext) g_markup_parse_context_free (priv->parsecontext); priv->parsecontext = NULL; @@ -504,10 +507,16 @@ _loading_done (GESFormatter * self) } g_list_free_full (assets, g_object_unref); - if (priv->state == STATE_LOADING_ASSETS_AND_SYNC) { + if (priv->asset_error) { + error = priv->asset_error; + priv->asset_error = NULL; + } else if (priv->state == STATE_LOADING_ASSETS_AND_SYNC) { + GMarkupParseContext *context = + _parse (GES_BASE_XML_FORMATTER (self), &error, STATE_LOADING_CLIPS); GST_INFO_OBJECT (self, "Assets cached... now loading the timeline."); - g_markup_parse_context_free (_parse (GES_BASE_XML_FORMATTER (self), NULL, - STATE_LOADING_CLIPS)); + + if (context) + g_markup_parse_context_free (context); g_assert (priv->pending_assets == NULL); } @@ -516,7 +525,8 @@ _loading_done (GESFormatter * self) priv->timeline_auto_transition); g_hash_table_foreach (priv->layers, (GHFunc) _set_auto_transition, NULL); - ges_project_set_loaded (self->project, self); + ges_project_set_loaded (self->project, self, error); + g_clear_error (&error); } static gboolean @@ -565,14 +575,17 @@ _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 * children_properties) + GstStructure * properties, GstStructure * children_properties, + GError ** error) { GESClip *clip = ges_layer_add_asset (layer, asset, start, inpoint, duration, track_types); if (clip == NULL) { - GST_WARNING_OBJECT (clip, "Could not add object from asset: %s", - ges_asset_get_id (asset)); + g_set_error (error, GES_ERROR, GES_ERROR_FORMATTER_MALFORMED_INPUT_FILE, + "Could not add clip %s [ %" GST_TIME_FORMAT ", ( %" GST_TIME_FORMAT + ") - %" GST_TIME_FORMAT "]", id, GST_TIME_ARGS (start), + GST_TIME_ARGS (inpoint), GST_TIME_ARGS (duration)); return NULL; } @@ -667,6 +680,8 @@ new_asset_cb (GESAsset * source, GAsyncResult * res, PendingAsset * passet) error->message); _free_pending_asset (priv, passet); + if (!priv->asset_error) + priv->asset_error = g_error_copy (error); goto done; } @@ -827,7 +842,7 @@ ges_base_xml_formatter_add_asset (GESBaseXmlFormatter * self, GESBaseXmlFormatterPrivate *priv = _GET_PRIV (self); if (priv->state != STATE_LOADING_ASSETS_AND_SYNC) { - GST_INFO ("Not parsing assets in %s state", + GST_DEBUG_OBJECT (self, "Not parsing assets in %s state", loading_state_name (priv->state)); return; @@ -880,15 +895,14 @@ ges_base_xml_formatter_add_clip (GESBaseXmlFormatter * self, asset = ges_asset_request (type, asset_id, NULL); if (!asset) { g_set_error (error, GES_ERROR, GES_ERROR_FORMATTER_MALFORMED_INPUT_FILE, - "Clip references asset %s which was not present in the list of ressource" - " the file seems to be malformed.", asset_id); - GST_ERROR_OBJECT (self, "%s", (*error)->message); + "Clip references asset %s of type %s which was not present in the list of ressource," + " the file seems to be malformed.", asset_id, g_type_name (type)); return; } nclip = _add_object_to_layer (priv, id, entry->layer, asset, start, inpoint, duration, track_types, metadatas, properties, - children_properties); + children_properties, error); gst_object_unref (asset); if (!nclip) diff --git a/ges/ges-internal.h b/ges/ges-internal.h index 9b3d260a87..e109b4965a 100644 --- a/ges/ges-internal.h +++ b/ges/ges-internal.h @@ -229,7 +229,8 @@ _find_formatter_asset_for_id (const gchar *id); /* FIXME This should probably become public, but we need to make sure it * is the right API before doing so */ G_GNUC_INTERNAL gboolean ges_project_set_loaded (GESProject * project, - GESFormatter *formatter); + GESFormatter *formatter, + GError *error); G_GNUC_INTERNAL gchar * ges_project_try_updating_id (GESProject *self, GESAsset *asset, GError *error); diff --git a/ges/ges-pitivi-formatter.c b/ges/ges-pitivi-formatter.c index 38fd0a278c..ad67825917 100644 --- a/ges/ges-pitivi-formatter.c +++ b/ges/ges-pitivi-formatter.c @@ -437,7 +437,7 @@ track_element_added_cb (GESClip * clip, priv->sources_to_load = g_list_remove (priv->sources_to_load, clip); if (!priv->sources_to_load && GES_FORMATTER (formatter)->project) ges_project_set_loaded (GES_FORMATTER (formatter)->project, - GES_FORMATTER (formatter)); + GES_FORMATTER (formatter), NULL); } /* Disconnect the signal */ @@ -668,7 +668,7 @@ load_pitivi_file_from_uri (GESFormatter * self, */ if (!g_hash_table_size (priv->clips_table) && GES_FORMATTER (self)->project) { ges_project_set_loaded (GES_FORMATTER (self)->project, - GES_FORMATTER (self)); + GES_FORMATTER (self), NULL); } else { if (!make_clips (self)) { GST_ERROR ("Couldn't deserialise the project properly"); diff --git a/ges/ges-project.c b/ges/ges-project.c index b0fd37916c..65b1a6bc33 100644 --- a/ges/ges-project.c +++ b/ges/ges-project.c @@ -97,6 +97,7 @@ enum { LOADING_SIGNAL, LOADED_SIGNAL, + ERROR_LOADING, ERROR_LOADING_ASSET, ASSET_ADDED_SIGNAL, ASSET_REMOVED_SIGNAL, @@ -591,6 +592,20 @@ ges_project_class_init (GESProjectClass * klass) NULL, NULL, g_cclosure_marshal_generic, G_TYPE_NONE, 3, G_TYPE_ERROR, G_TYPE_STRING, G_TYPE_GTYPE); + /** + * GESProject::error-loading: + * @project: the #GESProject on which a problem happend when creted a #GESAsset + * @timeline: The timeline that failed loading + * @error: The #GError defining the error that occured + * + * Since: 1.18 + */ + _signals[ERROR_LOADING] = + g_signal_new ("error-loading", G_TYPE_FROM_CLASS (klass), + G_SIGNAL_RUN_LAST, 0, + NULL, NULL, g_cclosure_marshal_generic, G_TYPE_NONE, 2, GES_TYPE_TIMELINE, + G_TYPE_ERROR); + object_class->dispose = _dispose; object_class->finalize = _finalize; @@ -731,8 +746,15 @@ new_asset_cb (GESAsset * source, GAsyncResult * res, GESProject * project) * Returns: %TRUE if the signale could be emitted %FALSE otherwize */ gboolean -ges_project_set_loaded (GESProject * project, GESFormatter * formatter) +ges_project_set_loaded (GESProject * project, GESFormatter * formatter, + GError * error) { + if (error) { + GST_ERROR_OBJECT (project, "Emit project error-loading %s", error->message); + g_signal_emit (project, _signals[ERROR_LOADING], 0, formatter->timeline, + error); + } + GST_INFO_OBJECT (project, "Emit project loaded"); if (GST_STATE (formatter->timeline) < GST_STATE_PAUSED) { timeline_fill_gaps (formatter->timeline); diff --git a/plugins/ges/gesdemux.c b/plugins/ges/gesdemux.c index 00d94a4d31..e674d3a284 100644 --- a/plugins/ges/gesdemux.c +++ b/plugins/ges/gesdemux.c @@ -142,6 +142,7 @@ typedef struct GError *error; gulong loaded_sigid; gulong error_sigid; + gulong error_asset_sigid; } TimelineConstructionData; static void @@ -156,8 +157,8 @@ project_loaded_cb (GESProject * project, GESTimeline * timeline, } static void -error_loading_asset_cb (GESProject * project, GError * error, gchar * id, - GType extractable_type, TimelineConstructionData * data) +error_loading_cb (GESProject * project, GESTimeline * timeline, + GError * error, TimelineConstructionData * data) { data->error = g_error_copy (error); g_signal_handler_disconnect (project, data->error_sigid); @@ -166,6 +167,17 @@ error_loading_asset_cb (GESProject * project, GError * error, gchar * id, g_main_loop_quit (data->ml); } +static void +error_loading_asset_cb (GESProject * project, GError * error, gchar * id, + GType extractable_type, TimelineConstructionData * data) +{ + data->error = g_error_copy (error); + g_signal_handler_disconnect (project, data->error_asset_sigid); + data->error_asset_sigid = 0; + + g_main_loop_quit (data->ml); +} + static gboolean ges_demux_src_probe (GstPad * pad, GstPadProbeInfo * info, GstElement * parent) { @@ -337,9 +349,12 @@ ges_demux_create_timeline (GESDemux * self, gchar * uri, GError ** error) data.loaded_sigid = g_signal_connect (project, "loaded", G_CALLBACK (project_loaded_cb), &data); - data.error_sigid = + data.error_asset_sigid = g_signal_connect_after (project, "error-loading-asset", G_CALLBACK (error_loading_asset_cb), &data); + data.error_sigid = + g_signal_connect_after (project, "error-loading", + G_CALLBACK (error_loading_cb), &data); unused = GES_TIMELINE (ges_asset_extract (GES_ASSET (project), &data.error)); if (data.error) { @@ -350,6 +365,9 @@ ges_demux_create_timeline (GESDemux * self, gchar * uri, GError ** error) g_main_loop_run (data.ml); g_main_loop_unref (data.ml); + if (data.error) + goto done; + ges_demux_adapt_timeline_duration (self, data.timeline); query = gst_query_new_uri (); @@ -387,13 +405,21 @@ done: if (data.error_sigid) g_signal_handler_disconnect (project, data.error_sigid); + if (data.error_asset_sigid) + g_signal_handler_disconnect (project, data.error_asset_sigid); + g_clear_object (&project); GST_INFO_OBJECT (self, "Timeline properly loaded: %" GST_PTR_FORMAT, data.timeline); - ges_base_bin_set_timeline (GES_BASE_BIN (self), data.timeline); - gst_element_foreach_src_pad (GST_ELEMENT (self), ges_demux_set_srcpad_probe, - NULL); + + if (!data.error) { + ges_base_bin_set_timeline (GES_BASE_BIN (self), data.timeline); + gst_element_foreach_src_pad (GST_ELEMENT (self), ges_demux_set_srcpad_probe, + NULL); + } else { + *error = data.error; + } g_main_context_pop_thread_default (ctx); diff --git a/tools/ges-launcher.c b/tools/ges-launcher.c index 90451c60b9..74685de651 100644 --- a/tools/ges-launcher.c +++ b/tools/ges-launcher.c @@ -172,6 +172,16 @@ retry: return TRUE; } +static void +_project_loading_error_cb (GESProject * project, GESTimeline * timeline, + GError * error, GESLauncher * self) +{ + g_printerr ("Error loading timeline: '%s'\n", error->message); + self->priv->seenerrors = TRUE; + + g_application_quit (G_APPLICATION (self)); +} + static void _project_loaded_cb (GESProject * project, GESTimeline * timeline, GESLauncher * self) @@ -256,6 +266,8 @@ _create_timeline (GESLauncher * self, const gchar * serialized_timeline, g_signal_connect (project, "error-loading-asset", G_CALLBACK (_error_loading_asset_cb), self); g_signal_connect (project, "loaded", G_CALLBACK (_project_loaded_cb), self); + g_signal_connect (project, "error-loading", + G_CALLBACK (_project_loading_error_cb), self); self->priv->timeline = GES_TIMELINE (ges_asset_extract (GES_ASSET (project), &error));