formatter: Enhance error reporting

And add a "loading-error" signal in GESProject so we can report
issue when loading async elements for the timeline.

diff --git a/ges/ges-base-xml-formatter.c b/ges/ges-base-xml-formatter.c
index f4744b5..42ed146 100644 (file)
--- 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 9b3d260..e109b49 100644 (file)
--- 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 38fd0a2..ad67825 100644 (file)
--- 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 b0fd379..65b1a6b 100644 (file)
--- 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 00d94a4..e674d3a 100644 (file)
--- 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 90451c6..74685de 100644 (file)
--- a/tools/ges-launcher.c
+++ b/tools/ges-launcher.c
@@ -173,6 +173,16 @@ retry:
 }
 
 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));