Implement proxy editing about gst-editing-services

[thiblahute]

Some comments at: dark-al/gst-editing-services-old@a0153ea#commitcomment-3635072

[thiblahute]

First review, just edit the comment and mark the [TODO] as [TO REVIEW] when you think it is ready to be reviewed

This is just the first part, and am plaaning to review the rest when I find some time :)b

diff --git a/ges/ges-asset.c b/ges/ges-asset.c
index afd6bba..0ab5b8b 100644
--- a/ges/ges-asset.c
+++ b/ges/ges-asset.c
@@ -100,7 +100,8 @@ typedef enum
   ASSET_NOT_INITIALIZED,
   ASSET_INITIALIZING, ASSET_INITIALIZED_WITH_ERROR,
   ASSET_PROXIED,
-  ASSET_INITIALIZED
+  ASSET_NEEDS_RELOAD,
+  ASSET_INITIALIZED,
 } GESAssetState;

 static GParamSpec *_properties[PROP_LAST];
@@ -113,7 +114,10 @@ struct _GESAssetPrivate

   /* When a asset is proxied, instanciating it will
    * return the asset it points to */
-  char *proxied_asset_id;
+  gchar *proxied_asset_id;
+
+  GESAsset *parent;

[TODO]:

WHat is parent? The asset that we act as a proxy for? not sure it is the best name, but can not figure a better one right now

+  GList *proxies;

   /* The error that accured when a asset has been initialized with error */
   GError *error;
@@ -499,7 +503,10 @@ ges_asset_cache_set_loaded (GType extractable_type, const gchar * id,
       g_simple_async_result_set_from_error (G_SIMPLE_ASYNC_RESULT (tmp->data),
           error);
       g_simple_async_result_complete (G_SIMPLE_ASYNC_RESULT (tmp->data));
+      /* FIXME: ASSET_NEEDS_RELOAD don't work with that */
+#if 0
       gst_object_unref (tmp->data);
+#endif
     }

     g_list_free (results);
@@ -608,6 +615,28 @@ ges_asset_set_proxy (GESAsset * asset, const gchar * new_id)
   return TRUE;
 }

+const gchar *
+ges_asset_get_parent_id (GESAsset * asset)
+{
+  g_return_val_if_fail (GES_IS_ASSET (asset), NULL);
+
+  return ges_asset_get_id (asset->priv->parent);
+}
+
+gboolean
+ges_asset_set_parent (GESAsset * asset, GESAsset * parent)
+{
+  g_return_val_if_fail (GES_IS_ASSET (asset), FALSE);
+  g_return_val_if_fail (GES_IS_ASSET (parent), FALSE);
+
+  asset->priv->parent = gst_object_ref (parent);
+
+  GST_DEBUG ("Setted parent %s for asset: %s", ges_asset_get_id (parent),

[TODO]:

s/Setted/Set/

+      ges_asset_get_id (asset));
+
+  return TRUE;
+}
+
 /* Caution, this method should be used in rare cases (ie: for the project
  * as we can change its ID from a useless one to a proper URI). In most
  * cases you want to update the ID creating a proxy
@@ -746,6 +775,11 @@ ges_asset_request (GType extractable_type, const gchar * id, GError ** error)
             goto done;
           }
           break;
+        case ASSET_NEEDS_RELOAD:
+          GST_DEBUG_OBJECT (asset, "Asset in cache and needs reload");
+          initable_init (G_INITABLE (asset), NULL, NULL);

[TODO]:

Looks weird to call that method directly, you should rework that a bit so we have a method that
does the same thing but has a proper name.

+
+          goto done;
         case ASSET_INITIALIZED_WITH_ERROR:
           GST_WARNING_OBJECT (asset, "Initialized with error, not returning");
           if (error)
@@ -888,6 +922,12 @@ ges_asset_request_async (GType extractable_type,
             goto done;
           }
           break;
+        case ASSET_NEEDS_RELOAD:
+          GST_DEBUG_OBJECT (asset, "Asset in cache and needs reload");
+          ges_asset_cache_append_result (extractable_type, real_id, simple);
+          GES_ASSET_GET_CLASS (asset)->start_loading (asset, &error);

[TODO]:

You should do something similare to what is done for the sync version, using a variant
of async_initable_init_async as here you are pretty missing a few things, and what you want
to do is basically the same thing.

Which means you should create:

static void ges_asset_init_async (GESAsset *asset, GSimpleAsyncResult *simple,
GAsyncReadyCallback callback, gpointer use_data);

and
static gboolean ges_asset_init (GESAsset *asset);

and obviously use them in initable_init and async_initable_init_async.

+
+          goto done;
         case ASSET_INITIALIZED_WITH_ERROR:
           g_simple_async_report_gerror_in_idle (G_OBJECT (asset), callback,
               user_data, error ? error : asset->priv->error);
@@ -910,6 +950,37 @@ done:
     g_free (real_id);
 }

[TODO]:

Please document that new API!
I think that name is not really great, I would rename that method to

gboolean
ges_asset_set_reload_needed (GType extractable_type, const gchar * id);

+gboolean
+ges_asset_needs_reload (GType extractable_type, const gchar * id)
+{
+  gchar *real_id;
+  GESAsset *asset;
+  GError *error = NULL;
+
+  real_id = _check_and_update_parameters (&extractable_type, id, &error);
+  if (error) {
+    _unsure_material_for_wrong_id (id, extractable_type, error);
+    real_id = g_strdup (id);
+  }
+
+  asset = ges_asset_cache_lookup (extractable_type, real_id);
+  if (real_id) {
+    g_free (real_id);
+  }

[TODO]:

Please factor that part out into and make use of it in:

GESAsset *
ges_asset_request (GType extractable_type, const gchar * id, GError ** error);

+
+  if (asset) {
+    GST_DEBUG_OBJECT (asset,
+        "Asset with id %s switch state to ASSET_NEEDS_RELOAD",
+        ges_asset_get_id (asset));
+    asset->priv->state = ASSET_NEEDS_RELOAD;
+    return TRUE;
+  }
+
+  GST_DEBUG ("Asset with id %s not found in cache", id);
+  return FALSE;
+}
+
 /**
  * ges_asset_get_id:
  * @self: The #GESAsset to get ID from
diff --git a/ges/ges-asset.h b/ges/ges-asset.h
index e4b2189..7fb68fc 100644
--- a/ges/ges-asset.h
+++ b/ges/ges-asset.h
@@ -92,6 +92,7 @@ void ges_asset_request_async         (GType extractable_type,
 GESAsset * ges_asset_request         (GType extractable_type,
                                       const gchar * id,
                                       GError **error);
+gboolean ges_asset_needs_reload      (GType extractable_type, const gchar * id);
 const gchar * ges_asset_get_id       (GESAsset* self);
 GESAsset * ges_asset_request_finish  (GAsyncResult *res,
                                       GError **error);
diff --git a/ges/ges-base-xml-formatter.c b/ges/ges-base-xml-formatter.c
index 8bd5106..eba6e81 100644
--- a/ges/ges-base-xml-formatter.c
+++ b/ges/ges-base-xml-formatter.c
@@ -80,6 +80,7 @@ typedef struct PendingAsset
 {
   GESFormatter *formatter;
   gchar *metadatas;
+  gchar *parent_id;
   GstStructure *properties;
 } PendingAsset;

@@ -573,6 +574,12 @@ new_asset_cb (GESAsset * source, GAsyncResult * res, PendingAsset * passet)
     if (passet->metadatas)
       ges_meta_container_add_metas_from_string (GES_META_CONTAINER (source),
           passet->metadatas);
+    if (passet->parent_id) {
+      GType extractable_type = ges_asset_get_extractable_type (asset);
+      GESAsset *parent =
+          ges_asset_cache_lookup (extractable_type, passet->parent_id);

[TODO]:

What happens here if the parent has not been loaded yet? please make sure to handle that case.

+      ges_asset_set_parent (asset, parent);
+    }
     if (passet->properties)
       gst_structure_foreach (passet->properties,
           (GstStructureForeachFunc) set_property_foreach, source);
@@ -712,8 +719,8 @@ _create_profile (GESBaseXmlFormatter * self,

 void
 ges_base_xml_formatter_add_asset (GESBaseXmlFormatter * self,
-    const gchar * id, GType extractable_type, GstStructure * properties,
-    const gchar * metadatas, GError ** error)
+    const gchar * id, const gchar * parent_id, GType extractable_type,
+    GstStructure * properties, const gchar * metadatas, GError ** error)
 {
   PendingAsset *passet;
   GESBaseXmlFormatterPrivate *priv = _GET_PRIV (self);
@@ -724,6 +731,8 @@ ges_base_xml_formatter_add_asset (GESBaseXmlFormatter * self,
   passet = g_slice_new0 (PendingAsset);
   passet->metadatas = g_strdup (metadatas);
   passet->formatter = gst_object_ref (self);
+  if (parent_id)
+    passet->parent_id = g_strdup (parent_id);
   if (properties)
     passet->properties = gst_structure_copy (properties);

diff --git a/ges/ges-internal.h b/ges/ges-internal.h
index 06c60dd..9dfa5d8 100644
--- a/ges/ges-internal.h
+++ b/ges/ges-internal.h
@@ -112,6 +112,12 @@ ges_asset_cache_lookup(GType extractable_type, const gchar * id);
 gboolean
 ges_asset_set_proxy (GESAsset *asset, const gchar *new_id);

+G_GNUC_INTERNAL const gchar *
+ges_asset_get_parent_id (GESAsset * asset);
+
+G_GNUC_INTERNAL gboolean
+ges_asset_set_parent (GESAsset * asset, GESAsset * parent);
+
 G_GNUC_INTERNAL gboolean
 ges_asset_request_id_update (GESAsset *asset, gchar **proposed_id,
     GError *error);
@@ -189,6 +195,7 @@ G_GNUC_INTERNAL void ges_base_xml_formatter_add_clip (GESBaseXmlFormatter * self
                                                                  GError **error);
 G_GNUC_INTERNAL void ges_base_xml_formatter_add_asset        (GESBaseXmlFormatter * self,
                                                                  const gchar * id,
+                                                                 const gchar * parent_id,
                                                                  GType extractable_type,
                                                                  GstStructure *properties,
                                                                  const gchar *metadatas,
diff --git a/ges/ges-project.c b/ges/ges-project.c
index 5600a3a..525fa8d 100644
--- a/ges/ges-project.c
+++ b/ges/ges-project.c
@@ -45,6 +45,7 @@
  */
 #include "ges.h"
 #include "ges-internal.h"
+#include <glib/gstdio.h>

 /* TODO We should rely on both extractable_type and @id to identify
  * a Asset, not only @id
@@ -64,6 +65,19 @@ struct _GESProjectPrivate
   gchar *uri;

   GList *encoding_profiles;
+
+  GstEncodingProfile *proxy_profile;

[TODO]:

Please call it proxies_profile;

+  GstElement *proxy_pipeline;

[TODO]:

Please call it proxies_creation_pipeline

[thiblahute]

Done