diff --git a/ChangeLog b/ChangeLog index e8324c9945..607e815da8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2006-04-11 Wim Taymans + + * 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 * gst/gstsegment.c: diff --git a/common b/common index 1783855e98..a6710e67fd 160000 --- a/common +++ b/common @@ -1 +1 @@ -Subproject commit 1783855e983a5294434673694e8a57e44980b6f1 +Subproject commit a6710e67fd82147e32a18f1b63177583faffd498 diff --git a/gst/gstelementfactory.c b/gst/gstelementfactory.c index 6958228aa9..9b9121652d 100644 --- a/gst/gstelementfactory.c +++ b/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) diff --git a/gst/gstpluginfeature.c b/gst/gstpluginfeature.c index 2a05a273fd..737fcb314b 100644 --- a/gst/gstpluginfeature.c +++ b/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; } /** diff --git a/tests/check/gst/gstelement.c b/tests/check/gst/gstelement.c index 7c8558a89b..401957b134 100644 --- a/tests/check/gst/gstelement.c +++ b/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; }