From c12b84788d197c714ec32653e2b751079e377c46 Mon Sep 17 00:00:00 2001 From: Henry Wilkes Date: Wed, 4 Mar 2020 11:31:32 +0000 Subject: [PATCH] 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. --- ges/ges-asset.c | 84 +++++++++++++++++++++++++------------------- ges/ges-base-xml-formatter.c | 2 +- ges/ges-internal.h | 5 +++ ges/ges-project.c | 4 +-- tests/check/ges/asset.c | 4 +-- 5 files changed, 57 insertions(+), 42 deletions(-) diff --git a/ges/ges-asset.c b/ges/ges-asset.c index 6c6eb28..8a4ddf4 100644 --- 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 --- 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 --- 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 --- 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 --- 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); -- 2.7.4