asset: move set_proxy (NULL, proxy) behaviour to new method

We should not be accepting ges_asset_set_proxy (NULL, proxy) as part of
the API! This behaviour was used internally in combination with
ges_asset_try_proxy, which is called on a still loading asset, so it was
moved to ges_asset_finish_proxy.

diff --git a/ges/ges-asset.c b/ges/ges-asset.c
index 6c6eb28..8a4ddf4 100644 (file)
--- a/ges/ges-asset.c
+++ b/ges/ges-asset.c
@@ -160,10 +160,12 @@ struct _GESAssetPrivate
   GESAssetState state;
   GType extractable_type;
 
-  /* When an asset is proxied, instantiating it will
-   * return the asset it points to */
+  /* used internally by try_proxy to pre-set a proxy whilst an asset is
+   * still loading. It can be used later to set the proxy for the asset
+   * once it has finished loading */
   char *proxied_asset_id;
 
+  /* actual list of proxies */
   GList *proxies;
   GESAsset *proxy_target;
 
@@ -769,6 +771,8 @@ ges_asset_request_id_update (GESAsset * asset, gchar ** proposed_id,
       error);
 }
 
+/* pre-set a proxy id whilst the asset is still loading. Once the proxy
+ * is loaded, call ges_asset_finish_proxy (proxy) */
 gboolean
 ges_asset_try_proxy (GESAsset * asset, const gchar * new_id)
 {
@@ -812,6 +816,46 @@ _lookup_proxied_asset (const gchar * id, GESAssetCacheEntry * entry,
   return !g_strcmp0 (asset_id, entry->asset->priv->proxied_asset_id);
 }
 
+/* find the assets that called try_proxy for the asset id of @proxy
+ * and set @proxy as their proxy */
+gboolean
+ges_asset_finish_proxy (GESAsset * proxy)
+{
+  GESAsset *proxied_asset;
+  GHashTable *entries_table;
+  GESAssetCacheEntry *entry;
+
+  LOCK_CACHE;
+  entries_table = g_hash_table_lookup (_get_type_entries (),
+      _extractable_type_name (proxy->priv->extractable_type));
+  entry = g_hash_table_find (entries_table, (GHRFunc) _lookup_proxied_asset,
+      (gpointer) ges_asset_get_id (proxy));
+
+  if (!entry) {
+    UNLOCK_CACHE;
+    GST_DEBUG_OBJECT (proxy, "Not proxying any asset %s", proxy->priv->id);
+    return FALSE;
+  }
+
+  proxied_asset = entry->asset;
+  UNLOCK_CACHE;
+
+  /* If the asset with the matching ->proxied_asset_id is already proxied
+   * by another asset, we actually want @proxy to proxy this instead */
+  while (proxied_asset->priv->proxies)
+    proxied_asset = proxied_asset->priv->proxies->data;
+
+  /* unless it is ourselves. I.e. it is already proxied by us */
+  if (proxied_asset == proxy)
+    return FALSE;
+
+  GST_INFO_OBJECT (proxied_asset,
+      "%s Making sure the proxy chain is fully set.",
+      ges_asset_get_id (entry->asset));
+  ges_asset_finish_proxy (proxied_asset);
+  return ges_asset_set_proxy (proxied_asset, proxy);
+}
+
 /**
  * ges_asset_set_proxy:
  * @asset: The #GESAsset to proxy
@@ -836,7 +880,7 @@ _lookup_proxied_asset (const gchar * id, GESAssetCacheEntry * entry,
 gboolean
 ges_asset_set_proxy (GESAsset * asset, GESAsset * proxy)
 {
-  g_return_val_if_fail (asset == NULL || GES_IS_ASSET (asset), FALSE);
+  g_return_val_if_fail (GES_IS_ASSET (asset), FALSE);
   g_return_val_if_fail (proxy == NULL || GES_IS_ASSET (proxy), FALSE);
   g_return_val_if_fail (asset != proxy, FALSE);
 
@@ -864,40 +908,6 @@ ges_asset_set_proxy (GESAsset * asset, GESAsset * proxy)
     return TRUE;
   }
 
-  /* FIXME: why are we allowing for a NULL asset? What is the desired
-   * behaviour? */
-  if (asset == NULL) {
-    GHashTable *entries_table;
-    GESAssetCacheEntry *entry;
-
-    LOCK_CACHE;
-    entries_table = g_hash_table_lookup (_get_type_entries (),
-        _extractable_type_name (proxy->priv->extractable_type));
-    entry = g_hash_table_find (entries_table, (GHRFunc) _lookup_proxied_asset,
-        (gpointer) ges_asset_get_id (proxy));
-
-    if (!entry) {
-      UNLOCK_CACHE;
-      GST_DEBUG_OBJECT (asset, "Not proxying any asset %s", proxy->priv->id);
-      return FALSE;
-    }
-
-    asset = entry->asset;
-    UNLOCK_CACHE;
-
-    while (asset->priv->proxies)
-      asset = asset->priv->proxies->data;
-
-    if (asset == proxy)
-      return FALSE;
-
-    GST_INFO_OBJECT (asset, "%s Making sure the proxy chain is fully set.",
-        ges_asset_get_id (entry->asset));
-    if (g_strcmp0 (asset->priv->proxied_asset_id, proxy->priv->id) ||
-        g_strcmp0 (asset->priv->id, proxy->priv->proxied_asset_id))
-      ges_asset_set_proxy (NULL, asset);
-  }
-
   if (proxy->priv->proxy_target) {
     GST_ERROR_OBJECT (asset,
         "Trying to use %s as a proxy, but it is already proxying %s",

diff --git a/ges/ges-base-xml-formatter.c b/ges/ges-base-xml-formatter.c
index 6946d56..5d8708e 100644 (file)
--- a/ges/ges-base-xml-formatter.c
+++ b/ges/ges-base-xml-formatter.c
@@ -503,7 +503,7 @@ _loading_done (GESFormatter * self)
    * properly set */
   assets = ges_project_list_assets (self->project, GES_TYPE_EXTRACTABLE);
   for (tmp = assets; tmp; tmp = tmp->next) {
-    ges_asset_set_proxy (NULL, tmp->data);
+    ges_asset_finish_proxy (tmp->data);
   }
   g_list_free_full (assets, g_object_unref);
 

diff --git a/ges/ges-internal.h b/ges/ges-internal.h
index 87323f3..c22ba81 100644 (file)
--- a/ges/ges-internal.h
+++ b/ges/ges-internal.h
@@ -175,6 +175,8 @@ ges_asset_cache_put (GESAsset * asset, GTask *task);
 G_GNUC_INTERNAL gboolean
 ges_asset_cache_set_loaded(GType extractable_type, const gchar * id, GError *error);
 
+/* FIXME: marked as GES_API just so they can be used in tests! */
+
 GES_API GESAsset*
 ges_asset_cache_lookup(GType extractable_type, const gchar * id);
 
@@ -182,6 +184,9 @@ GES_API gboolean
 ges_asset_try_proxy (GESAsset *asset, const gchar *new_id);
 
 G_GNUC_INTERNAL gboolean
+ges_asset_finish_proxy (GESAsset * proxy);
+
+G_GNUC_INTERNAL gboolean
 ges_asset_request_id_update (GESAsset *asset, gchar **proposed_id,
     GError *error);
 G_GNUC_INTERNAL gchar *

diff --git a/ges/ges-project.c b/ges/ges-project.c
index e1eabcf..4320fd0 100644 (file)
--- a/ges/ges-project.c
+++ b/ges/ges-project.c
@@ -737,7 +737,7 @@ new_asset_cb (GESAsset * source, GAsyncResult * res, GESProject * project)
     return;
   }
 
-  ges_asset_set_proxy (NULL, asset);
+  ges_asset_finish_proxy (asset);
   ges_project_add_asset (project, asset);
   if (asset)
     gst_object_unref (asset);
@@ -923,7 +923,7 @@ ges_project_create_asset_sync (GESProject * project, const gchar * id,
   }
 
   if (!ges_asset_get_proxy_target (asset))
-    ges_asset_set_proxy (NULL, asset);
+    ges_asset_finish_proxy (asset);
 
   ges_project_add_asset (project, asset);
 

diff --git a/tests/check/ges/asset.c b/tests/check/ges/asset.c
index 89ebdfd..04f96b9 100644 (file)
--- a/tests/check/ges/asset.c
+++ b/tests/check/ges/asset.c
@@ -283,7 +283,7 @@ GST_START_TEST (test_proxy_asset)
   fail_unless (nothing != NULL);
 
   fail_unless (ges_asset_try_proxy (nothing, "video identity"));
-  fail_unless (ges_asset_set_proxy (NULL, identity));
+  fail_unless (ges_asset_finish_proxy (identity));
 
   nothing_at_all = ges_asset_request (GES_TYPE_EFFECT, "nothing_at_all", NULL);
   fail_if (nothing_at_all);
@@ -293,7 +293,7 @@ GST_START_TEST (test_proxy_asset)
 
   /* Now we proxy nothing_at_all to nothing which is itself proxied to identity */
   fail_unless (ges_asset_try_proxy (nothing_at_all, "nothing"));
-  fail_unless (ges_asset_set_proxy (NULL, nothing));
+  fail_unless (ges_asset_finish_proxy (nothing));
   fail_unless_equals_int (g_list_length (ges_asset_list_proxies
           (nothing_at_all)), 1);