gst/gstelementfactory.c: Some cleanups.

Original commit message from CVS:
* gst/gstelementfactory.c: (gst_element_register),
(gst_element_factory_create), (gst_element_factory_make):
Some cleanups.
Fixed a FIXME.
Updated docs (Fixes #131079)
* gst/gstpluginfeature.c: (gst_plugin_feature_load):
Small cleanups.
* tests/check/gst/gstelement.c: (GST_START_TEST),
(gst_element_suite):
Added testcase for elementfactory class field.

ChangeLog

@@ -1,3 +1,18 @@
+2006-04-11  Wim Taymans  <wim@fluendo.com>
+
+	* gst/gstelementfactory.c: (gst_element_register),
+	(gst_element_factory_create), (gst_element_factory_make):
+	Some cleanups.
+	Fixed a FIXME.
+	Updated docs (Fixes #131079)
+
+	* gst/gstpluginfeature.c: (gst_plugin_feature_load):
+	Small cleanups.
+
+	* tests/check/gst/gstelement.c: (GST_START_TEST),
+	(gst_element_suite):
+	Added testcase for elementfactory class field.
+
 2006-04-10  Wim Taymans  <wim@fluendo.com>
 
 	* gst/gstsegment.c:

common

@@ -1 +1 @@
-Subproject commit 1783855e983a5294434673694e8a57e44980b6f1
+Subproject commit a6710e67fd82147e32a18f1b63177583faffd498

gst/gstelementfactory.c

@@ -248,7 +248,7 @@ gst_element_factory_cleanup (GstElementFactory * factory)
  * @type: GType of element to register
  *
  * Create a new elementfactory capable of instantiating objects of the
- * given type.
+ * @type and add the factory to @plugin.
  *
  * Returns: TRUE, if the registering succeeded, FALSE on error
  */
@@ -318,9 +318,13 @@ gst_element_register (GstPlugin * plugin, const gchar * name, guint rank,
 
   return TRUE;
 
+  /* ERRORS */
 error:
-  gst_element_factory_cleanup (factory);
-  return FALSE;
+  {
+    GST_WARNING_OBJECT (factory, "error with uri handler!");
+    gst_element_factory_cleanup (factory);
+    return FALSE;
+  }
 }
 
 /**
@@ -348,13 +352,11 @@ gst_element_factory_create (GstElementFactory * factory, const gchar * name)
   newfactory =
       GST_ELEMENT_FACTORY (gst_plugin_feature_load (GST_PLUGIN_FEATURE
           (factory)));
-  if (newfactory == NULL) {
-    GST_WARNING_OBJECT (factory, "loading plugin returned NULL!");
-    return NULL;
-  } else {
-    gst_object_unref (factory);
-    factory = newfactory;
-  }
+  if (newfactory == NULL)
+    goto load_failed;
+
+  gst_object_unref (factory);
+  factory = newfactory;
 
   if (name)
     GST_INFO ("creating element \"%s\" named \"%s\"",
@@ -362,34 +364,43 @@ gst_element_factory_create (GstElementFactory * factory, const gchar * name)
   else
     GST_INFO ("creating element \"%s\"", GST_PLUGIN_FEATURE_NAME (factory));
 
-#if 0
-  if (factory->type == 0) {
-    g_critical ("Plugin didn't set object type in feature.");
+  if (factory->type == 0)
+    goto no_type;
 
-    return NULL;
-  }
-#endif
+  /* create an instance of the element, cast so we don't assert on NULL */
+  element = GST_ELEMENT_CAST (g_object_new (factory->type, NULL));
+  if (element == NULL)
+    goto no_element;
 
-  /* FIXME: the object class gets a pointer to the factory that might
-   * be disposed at the end of this call if it was newly loaded;
-   * to fix that, we should ref and then unref in an object class finalize,
-   * which we don't have currently. */
-  oclass = GST_ELEMENT_CLASS (g_type_class_ref (factory->type));
+  /* fill in the pointer to the factory in the element class. The
+   * class will not be unreffed currently. */
+  oclass = GST_ELEMENT_GET_CLASS (element);
   if (oclass->elementfactory == NULL)
     oclass->elementfactory = factory;
 
-  /* create an instance of the element */
-  element = GST_ELEMENT (g_object_new (factory->type, NULL));
-  g_assert (element != NULL);
-
-  g_type_class_unref (oclass);
-
   if (name)
     gst_object_set_name (GST_OBJECT (element), name);
 
   GST_DEBUG ("created element \"%s\"", GST_PLUGIN_FEATURE_NAME (factory));
 
   return element;
+
+  /* ERRORS */
+load_failed:
+  {
+    GST_WARNING_OBJECT (factory, "loading plugin returned NULL!");
+    return NULL;
+  }
+no_type:
+  {
+    GST_WARNING_OBJECT (factory, "factory has no type");
+    return NULL;
+  }
+no_element:
+  {
+    GST_WARNING_OBJECT (factory, "could not create element");
+    return NULL;
+  }
 }
 
 /**
@@ -415,21 +426,29 @@ gst_element_factory_make (const gchar * factoryname, const gchar * name)
   GST_LOG ("gstelementfactory: make \"%s\" \"%s\"",
       factoryname, GST_STR_NULL (name));
 
-  /* gst_plugin_load_element_factory (factoryname); */
   factory = gst_element_factory_find (factoryname);
-  if (factory == NULL) {
+  if (factory == NULL)
+    goto no_factory;
+
+  GST_LOG_OBJECT (factory, "found factory %p", factory);
+  element = gst_element_factory_create (factory, name);
+  gst_object_unref (factory);
+  if (element == NULL)
+    goto create_failed;
+
+  return element;
+
+  /* ERRORS */
+no_factory:
+  {
     GST_INFO ("no such element factory \"%s\"!", factoryname);
     return NULL;
   }
-  GST_LOG ("gstelementfactory: found factory %p", factory);
-  element = gst_element_factory_create (factory, name);
-  gst_object_unref (factory);
-  if (element == NULL) {
+create_failed:
+  {
     GST_INFO_OBJECT (factory, "couldn't create instance!");
     return NULL;
   }
-
-  return element;
 }
 
 void
@@ -448,9 +467,12 @@ __gst_element_factory_add_static_pad_template (GstElementFactory * factory,
  * gst_element_factory_get_element_type:
  * @factory: factory to get managed #GType from
  *
- * Get the #GType for elements managed by this factory
+ * Get the #GType for elements managed by this factory. The type can
+ * only be retrieved if the element factory is loaded, which can be
+ * assured with gst_plugin_feature_load().
  *
- * Returns: the #GType for elements managed by this factory
+ * Returns: the #GType for elements managed by this factory or 0 if
+ * the factory is not loaded.
  */
 GType
 gst_element_factory_get_element_type (GstElementFactory * factory)

gst/gstpluginfeature.c

@@ -111,28 +111,42 @@ gst_plugin_feature_load (GstPluginFeature * feature)
 
   GST_DEBUG ("loading plugin %s", feature->plugin_name);
   plugin = gst_plugin_load_by_name (feature->plugin_name);
-  if (!plugin) {
-    GST_WARNING ("Failed to load plugin containing feature '%s'.",
-        GST_PLUGIN_FEATURE_NAME (feature));
-    return NULL;
-  }
+  if (!plugin)
+    goto load_failed;
+
   GST_DEBUG ("loaded plugin %s", feature->plugin_name);
   gst_object_unref (plugin);
 
   real_feature =
       gst_registry_lookup_feature (gst_registry_get_default (), feature->name);
 
-  if (real_feature == NULL) {
+  if (real_feature == NULL)
+    goto disappeared;
+  else if (!real_feature->loaded)
+    goto not_found;
+
+  return real_feature;
+
+  /* ERRORS */
+load_failed:
+  {
+    GST_WARNING ("Failed to load plugin containing feature '%s'.",
+        GST_PLUGIN_FEATURE_NAME (feature));
+    return NULL;
+  }
+disappeared:
+  {
     GST_INFO
         ("Loaded plugin containing feature '%s', but feature disappeared.",
         feature->name);
-  } else if (!real_feature->loaded) {
+    return NULL;
+  }
+not_found:
+  {
     GST_INFO ("Tried to load plugin containing feature '%s', but feature was "
         "not found.", real_feature->name);
     return NULL;
   }
-
-  return real_feature;
 }
 
 /**

tests/check/gst/gstelement.c

@@ -159,6 +159,43 @@ GST_START_TEST (test_link_no_pads)
 
 GST_END_TEST;
 
+/* check if the elementfactory of a class is filled (see #131079) */
+GST_START_TEST (test_class)
+{
+  GstElementClass *klass;
+  GstElementFactory *factory;
+  GType type;
+
+  GST_DEBUG ("finding factory for queue");
+  factory = gst_element_factory_find ("queue");
+  fail_if (factory == NULL);
+
+  GST_DEBUG ("getting the type");
+  /* feature is not loaded, should return 0 as the type */
+  type = gst_element_factory_get_element_type (factory);
+  fail_if (type != 0);
+
+  GST_DEBUG ("now loading the plugin");
+  factory =
+      GST_ELEMENT_FACTORY (gst_plugin_feature_load (GST_PLUGIN_FEATURE
+          (factory)));
+  fail_if (factory == NULL);
+
+  /* feature is now loaded */
+  type = gst_element_factory_get_element_type (factory);
+  fail_if (type == 0);
+
+  klass = g_type_class_ref (factory->type);
+  fail_if (klass == NULL);
+
+  GST_DEBUG ("checking the element factory class field");
+  /* and elementfactory is filled in */
+  fail_if (klass->elementfactory == NULL);
+  fail_if (klass->elementfactory != factory);
+}
+
+GST_END_TEST;
+
 Suite *
 gst_element_suite (void)
 {
@@ -171,6 +208,7 @@ gst_element_suite (void)
   tcase_add_test (tc_chain, test_error_no_bus);
   tcase_add_test (tc_chain, test_link);
   tcase_add_test (tc_chain, test_link_no_pads);
+  tcase_add_test (tc_chain, test_class);
 
   return s;
 }