effect: Determine the effect type as soon as possible

Making it possible to create the nleobject right at the creation
of the element.

Reviewed-by: Thibault Saunier <thibault.saunier@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D738

ges/ges-effect-asset.c

@@ -33,49 +33,35 @@ G_DEFINE_TYPE (GESEffectAsset, ges_effect_asset, GES_TYPE_TRACK_ELEMENT_ASSET);
 
 struct _GESEffectAssetPrivate
 {
-  GESTrackType track_type;
+  gpointer nothing;
 };
 
-/* GESAsset virtual methods implementation */
+
+
 static void
 _fill_track_type (GESAsset * asset)
 {
-  GList *tmp;
+  GESTrackType ttype;
-  GstElement *effect = gst_parse_bin_from_description (ges_asset_get_id (asset),
+  gchar *bin_desc;
-      TRUE, NULL);
+  const gchar *id = ges_asset_get_id (asset);
 
-  if (effect == NULL)
+  bin_desc = ges_effect_assect_id_get_type_and_bindesc (id, &ttype, NULL);
-    return;
 
-  for (tmp = GST_BIN_CHILDREN (effect); tmp; tmp = tmp->next) {
+  if (bin_desc) {
-    GstElementFactory *factory =
+    ges_track_element_asset_set_track_type (GES_TRACK_ELEMENT_ASSET (asset),
-        gst_element_get_factory (GST_ELEMENT (tmp->data));
+        ttype);
-    const gchar *klass =
+  } else {
-        gst_element_factory_get_metadata (factory, GST_ELEMENT_METADATA_KLASS);
+    GST_WARNING_OBJECT (asset, "No track type set, you should"
-
+        " specify one in [audio, video] as first component" " in the asset id");
-    if (g_strrstr (klass, "Effect")) {
-      if (g_strrstr (klass, "Audio")) {
-        GES_EFFECT_ASSET (asset)->priv->track_type = GES_TRACK_TYPE_AUDIO;
-        break;
-      } else if (g_strrstr (klass, "Video")) {
-        GES_EFFECT_ASSET (asset)->priv->track_type = GES_TRACK_TYPE_VIDEO;
-        break;
-      }
-    }
   }
-
-  gst_object_unref (effect);
-  return;
 }
 
+/* GESAsset virtual methods implementation */
 static GESExtractable *
 _extract (GESAsset * asset, GError ** error)
 {
   GESExtractable *effect;
 
-  if (GES_EFFECT_ASSET (asset)->priv->track_type == GES_TRACK_TYPE_UNKNOWN)
-    _fill_track_type (asset);
-
   effect = GES_ASSET_CLASS (ges_effect_asset_parent_class)->extract (asset,
       error);
 
@@ -85,9 +71,6 @@ _extract (GESAsset * asset, GError ** error)
     return NULL;
   }
 
-  ges_track_element_set_track_type (GES_TRACK_ELEMENT (effect),
-      GES_EFFECT_ASSET (asset)->priv->track_type);
-
   return effect;
 }
 
@@ -96,8 +79,12 @@ ges_effect_asset_init (GESEffectAsset * self)
 {
   self->priv = G_TYPE_INSTANCE_GET_PRIVATE (self,
       GES_TYPE_EFFECT_ASSET, GESEffectAssetPrivate);
+}
 
-  self->priv->track_type = GES_TRACK_TYPE_UNKNOWN;
+static void
+ges_effect_asset_constructed (GObject * object)
+{
+  _fill_track_type (GES_ASSET (object));
 }
 
 static void
@@ -117,5 +104,72 @@ ges_effect_asset_class_init (GESEffectAssetClass * klass)
   g_type_class_add_private (klass, sizeof (GESEffectAssetPrivate));
 
   object_class->finalize = ges_effect_asset_finalize;
+  object_class->constructed = ges_effect_asset_constructed;
   asset_class->extract = _extract;
 }
+
+gchar *
+ges_effect_assect_id_get_type_and_bindesc (const char *id,
+    GESTrackType * track_type, GError ** error)
+{
+  GList *tmp;
+  GstElement *effect;
+  gchar **typebin_desc = NULL;
+  gchar *bindesc = NULL;
+
+  *track_type = GES_TRACK_TYPE_UNKNOWN;
+  typebin_desc = g_strsplit (id, " ", 2);
+  if (!g_strcmp0 (typebin_desc[0], "audio")) {
+    *track_type = GES_TRACK_TYPE_AUDIO;
+    bindesc = g_strdup (typebin_desc[1]);
+  } else if (!g_strcmp0 (typebin_desc[0], "video")) {
+    *track_type = GES_TRACK_TYPE_VIDEO;
+    bindesc = g_strdup (typebin_desc[1]);
+  } else {
+    bindesc = g_strdup (id);
+  }
+
+  g_strfreev (typebin_desc);
+
+  effect = gst_parse_bin_from_description (bindesc, TRUE, error);
+  if (effect == NULL) {
+    g_free (bindesc);
+
+    GST_ERROR ("Could not create element from: %s", id);
+
+    return NULL;
+  }
+
+  if (*track_type != GES_TRACK_TYPE_UNKNOWN) {
+    gst_object_unref (effect);
+
+    return bindesc;
+  }
+
+  for (tmp = GST_BIN_CHILDREN (effect); tmp; tmp = tmp->next) {
+    GstElementFactory *factory =
+        gst_element_get_factory (GST_ELEMENT (tmp->data));
+    const gchar *klass =
+        gst_element_factory_get_metadata (factory, GST_ELEMENT_METADATA_KLASS);
+
+    if (g_strrstr (klass, "Effect")) {
+      if (g_strrstr (klass, "Audio")) {
+        *track_type = GES_TRACK_TYPE_AUDIO;
+        break;
+      } else if (g_strrstr (klass, "Video")) {
+        *track_type = GES_TRACK_TYPE_VIDEO;
+        break;
+      }
+    }
+  }
+
+  gst_object_unref (effect);
+
+  if (*track_type == GES_TRACK_TYPE_UNKNOWN) {
+    *track_type = GES_TRACK_TYPE_VIDEO;
+    GST_ERROR ("Could not determine track type for %s, defaulting to video",
+        id);
+  }
+
+  return bindesc;
+}

ges/ges-effect.c

@@ -55,26 +55,44 @@ enum
 static gchar *
 extractable_check_id (GType type, const gchar * id, GError ** error)
 {
-  GstElement *effect = gst_parse_bin_from_description (id, TRUE, error);
+  gchar *bin_desc, *real_id;
+  GESTrackType ttype;
 
-  if (effect == NULL)
+  bin_desc = ges_effect_assect_id_get_type_and_bindesc (id, &ttype, error);
+
+  if (bin_desc == NULL)
     return NULL;
 
-  gst_object_unref (effect);
+  if (ttype == GES_TRACK_TYPE_AUDIO)
-  return g_strdup (id);
+    real_id = g_strdup_printf ("audio %s", bin_desc);
+  else if (ttype == GES_TRACK_TYPE_VIDEO)
+    real_id = g_strdup_printf ("video %s", bin_desc);
+  else
+    g_assert_not_reached ();
+
+  return real_id;
 }
 
 static GParameter *
 extractable_get_parameters_from_id (const gchar * id, guint * n_params)
 {
-  GParameter *params = g_new0 (GParameter, 2);
+  GParameter *params = g_new0 (GParameter, 3);
+  gchar *bin_desc;
+  GESTrackType ttype;
+
+  bin_desc = ges_effect_assect_id_get_type_and_bindesc (id, &ttype, NULL);
 
   params[0].name = "bin-description";
   g_value_init (&params[0].value, G_TYPE_STRING);
-  g_value_set_string (&params[0].value, id);
+  g_value_set_string (&params[0].value, bin_desc);
 
-  *n_params = 1;
+  params[1].name = "track-type";
+  g_value_init (&params[1].value, GES_TYPE_TRACK_TYPE);
+  g_value_set_flags (&params[1].value, ttype);
 
+  *n_params = 2;
+
+  g_free (bin_desc);
   return params;
 }
 
@@ -188,27 +206,20 @@ ges_effect_create_element (GESTrackElement * object)
 
   GError *error = NULL;
   GESEffect *self = GES_EFFECT (object);
-  GESTrack *track = ges_track_element_get_track (object);
   const gchar *wanted_categories[] = { "Effect", NULL };
 
-  if (!track) {
+  GESTrackType type = ges_track_element_get_track_type (object);
-    GST_WARNING
-        ("The object %p should be in a Track for the element to be created",
-        object);
-    return NULL;
-  }
 
-  if (track->type == GES_TRACK_TYPE_VIDEO) {
+  if (type == GES_TRACK_TYPE_VIDEO) {
     bin_desc = g_strconcat ("videoconvert name=pre_video_convert ! ",
         self->priv->bin_description, " ! videoconvert name=post_video_convert",
         NULL);
-  } else if (track->type == GES_TRACK_TYPE_AUDIO) {
+  } else if (type == GES_TRACK_TYPE_AUDIO) {
     bin_desc =
         g_strconcat ("audioconvert ! audioresample !",
         self->priv->bin_description, NULL);
   } else {
-    GST_DEBUG ("Track type not supported");
+    g_assert_not_reached ();
-    return NULL;
   }
 
   effect = gst_parse_bin_from_description (bin_desc, TRUE, &error);
@@ -234,7 +245,12 @@ ges_effect_create_element (GESTrackElement * object)
  * ges_effect_new:
  * @bin_description: The gst-launch like bin description of the effect
  *
- * Creates a new #GESEffect from the description of the bin.
+ * Creates a new #GESEffect from the description of the bin. It should be
+ * possible to determine the type of the effect through the element
+ * 'klass' metadata of the GstElements that will be created.
+ * In that corner case, you should use:
+ * #ges_asset_request (GES_TYPE_EFFECT, "audio your ! bin ! description", NULL);
+ * and extract that asset to be in full control.
  *
  * Returns: a newly created #GESEffect, or %NULL if something went
  * wrong.

ges/ges-internal.h

@@ -148,6 +148,10 @@ ges_asset_try_proxy (GESAsset *asset, const gchar *new_id);
 G_GNUC_INTERNAL gboolean
 ges_asset_request_id_update (GESAsset *asset, gchar **proposed_id,
     GError *error);
+G_GNUC_INTERNAL gchar *
+ges_effect_assect_id_get_type_and_bindesc (const char    *id,
+                                           GESTrackType  *track_type,
+                                           GError       **error);
 
 /* GESExtractable internall methods
  *

tests/check/ges/asset.c

@@ -158,7 +158,7 @@ GST_START_TEST (test_proxy_asset)
 
   fail_unless (ges_init ());
 
-  identity = ges_asset_request (GES_TYPE_EFFECT, "identity", NULL);
+  identity = ges_asset_request (GES_TYPE_EFFECT, "video identity", NULL);
   fail_unless (identity != NULL);
 
   nothing = ges_asset_request (GES_TYPE_EFFECT, "nothing", NULL);
@@ -167,7 +167,7 @@ GST_START_TEST (test_proxy_asset)
   nothing = ges_asset_cache_lookup (GES_TYPE_EFFECT, "nothing");
   fail_unless (nothing != NULL);
 
-  fail_unless (ges_asset_try_proxy (nothing, "identity"));
+  fail_unless (ges_asset_try_proxy (nothing, "video identity"));
   fail_unless (ges_asset_set_proxy (NULL, identity));
 
   nothing_at_all = ges_asset_request (GES_TYPE_EFFECT, "nothing_at_all", NULL);

tests/check/ges/project.c

@@ -184,7 +184,7 @@ asset_added_cb (GESProject * project, GESAsset * asset)
   GstDiscovererInfo *info;
 
   if (ges_asset_get_extractable_type (asset) == GES_TYPE_EFFECT) {
-    assert_equals_string (ges_asset_get_id (asset), "agingtv");
+    assert_equals_string (ges_asset_get_id (asset), "video agingtv");
   } else {
     info = ges_uri_clip_asset_get_info (GES_URI_CLIP_ASSET (asset));
     fail_unless (GST_IS_DISCOVERER_INFO (info));