From a08786c39143a92e31a351efb0ffbd727247e86f Mon Sep 17 00:00:00 2001 From: David Schleef Date: Thu, 15 Sep 2005 20:56:30 +0000 Subject: [PATCH] gst/gstplugin.c: Implement semi-decent recounting and locking in plugins and plugin features. Original commit message from CVS: * gst/gstplugin.c: Implement semi-decent recounting and locking in plugins and plugin features. * gst/gstplugin.h: * gst/gstpluginfeature.c: * gst/gstpluginfeature.h: * gst/gstregistry.c: --- ChangeLog | 9 ++++ common | 2 +- gst/gstplugin.c | 112 +++++++++++++++++------------------------ gst/gstplugin.h | 5 +- gst/gstpluginfeature.c | 47 ++++++----------- gst/gstpluginfeature.h | 2 + gst/gstregistry.c | 35 ++++--------- 7 files changed, 85 insertions(+), 127 deletions(-) diff --git a/ChangeLog b/ChangeLog index 73565c72dc..021a196a27 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2005-09-15 David Schleef + + * gst/gstplugin.c: Implement semi-decent recounting and locking + in plugins and plugin features. + * gst/gstplugin.h: + * gst/gstpluginfeature.c: + * gst/gstpluginfeature.h: + * gst/gstregistry.c: + 2005-09-15 Michael Smith * gst/gstregistry.c: (gst_registry_get_feature_list): diff --git a/common b/common index 9a5025a2d2..019a3be6a5 160000 --- a/common +++ b/common @@ -1 +1 @@ -Subproject commit 9a5025a2d276796d8d21243ef598e679ff7477bc +Subproject commit 019a3be6a5b7cde92c5daae35fe189c8aebeb5b6 diff --git a/gst/gstplugin.c b/gst/gstplugin.c index 688abfee0a..adfc78a584 100644 --- a/gst/gstplugin.c +++ b/gst/gstplugin.c @@ -80,7 +80,7 @@ static void gst_plugin_desc_copy (GstPluginDesc * dest, const GstPluginDesc * src); -G_DEFINE_TYPE (GstPlugin, gst_plugin, G_TYPE_OBJECT); +G_DEFINE_TYPE (GstPlugin, gst_plugin, GST_TYPE_OBJECT); static void gst_plugin_init (GstPlugin * plugin) @@ -294,6 +294,8 @@ _gst_plugin_fault_handler_setup (void) static void _gst_plugin_fault_handler_setup (); +GStaticMutex gst_plugin_loading_mutex = G_STATIC_MUTEX_INIT; + /** * gst_plugin_load_file: * @filename: the plugin filename to load @@ -311,14 +313,20 @@ gst_plugin_load_file (const gchar * filename, GError ** error) gboolean ret; gpointer ptr; struct stat file_status; + GstRegistry *registry; g_return_val_if_fail (filename != NULL, NULL); - plugin = gst_registry_lookup (gst_registry_get_default (), filename); + registry = gst_registry_get_default (); + g_static_mutex_lock (&gst_plugin_loading_mutex); + + plugin = gst_registry_lookup (registry, filename); if (plugin && plugin->module) { + g_static_mutex_unlock (&gst_plugin_loading_mutex); return plugin; } + GST_CAT_DEBUG (GST_CAT_PLUGIN_LOADING, "attempt to load plugin \"%s\"", filename); @@ -327,7 +335,7 @@ gst_plugin_load_file (const gchar * filename, GError ** error) g_set_error (error, GST_PLUGIN_ERROR, GST_PLUGIN_ERROR_MODULE, "Dynamic loading not supported"); - return NULL; + goto return_error; } if (stat (filename, &file_status)) { @@ -336,7 +344,7 @@ gst_plugin_load_file (const gchar * filename, GError ** error) GST_PLUGIN_ERROR, GST_PLUGIN_ERROR_MODULE, "Problem accessing file %s: %s\n", filename, strerror (errno)); - return NULL; + goto return_error; } module = g_module_open (filename, G_MODULE_BIND_LOCAL); @@ -345,7 +353,7 @@ gst_plugin_load_file (const gchar * filename, GError ** error) g_module_error ()); g_set_error (error, GST_PLUGIN_ERROR, GST_PLUGIN_ERROR_MODULE, "Opening module failed"); - return NULL; + goto return_error; } plugin = g_object_new (GST_TYPE_PLUGIN, NULL); @@ -362,8 +370,7 @@ gst_plugin_load_file (const gchar * filename, GError ** error) GST_PLUGIN_ERROR, GST_PLUGIN_ERROR_MODULE, "Could not find plugin entry point in \"%s\"", filename); - g_object_unref (plugin); - return NULL; + goto return_error; } plugin->orig_desc = (GstPluginDesc *) ptr; @@ -387,7 +394,7 @@ gst_plugin_load_file (const gchar * filename, GError ** error) GST_PLUGIN_ERROR_MODULE, "gst_plugin_register_func failed for plugin \"%s\"", filename); g_module_close (module); - return NULL; + goto return_error; } /* remove signal handler */ @@ -397,7 +404,11 @@ gst_plugin_load_file (const gchar * filename, GError ** error) gst_default_registry_add_plugin (plugin); + g_static_mutex_unlock (&gst_plugin_loading_mutex); return plugin; +return_error: + g_static_mutex_unlock (&gst_plugin_loading_mutex); + return NULL; } static void @@ -405,20 +416,13 @@ gst_plugin_desc_copy (GstPluginDesc * dest, const GstPluginDesc * src) { dest->major_version = src->major_version; dest->minor_version = src->minor_version; - g_free (dest->name); dest->name = g_strdup (src->name); - g_free (dest->description); dest->description = g_strdup (src->description); dest->plugin_init = src->plugin_init; - g_free (dest->version); dest->version = g_strdup (src->version); - g_free (dest->license); dest->license = g_strdup (src->license); - g_free (dest->source); dest->source = g_strdup (src->source); - g_free (dest->package); dest->package = g_strdup (src->package); - g_free (dest->origin); dest->origin = g_strdup (src->origin); } @@ -438,33 +442,6 @@ gst_plugin_desc_free (GstPluginDesc * desc) memset (desc, 0, sizeof (GstPluginDesc)); } #endif -/** - * gst_plugin_unload_plugin: - * @plugin: The plugin to unload - * - * Unload the given plugin. - * - * Returns: whether or not the plugin unloaded - */ -gboolean -gst_plugin_unload_plugin (GstPlugin * plugin) -{ - g_return_val_if_fail (plugin != NULL, FALSE); - - if (!plugin->module) - return TRUE; - - if (g_module_close (plugin->module)) { - plugin->module = NULL; - GST_CAT_INFO (GST_CAT_PLUGIN_LOADING, "plugin \"%s\" unloaded", - plugin->filename); - return TRUE; - } else { - GST_CAT_INFO (GST_CAT_PLUGIN_LOADING, "failed to unload plugin \"%s\"", - plugin->filename); - return FALSE; - } -} /** * gst_plugin_get_name: @@ -645,8 +622,16 @@ GList * gst_plugin_feature_filter (GstPlugin * plugin, GstPluginFeatureFilter filter, gboolean first, gpointer user_data) { - return gst_filter_run (plugin->features, (GstFilterFunc) filter, first, + GList *list; + GList *g; + + list = gst_filter_run (plugin->features, (GstFilterFunc) filter, first, user_data); + for (g = list; g; g = g->next) { + gst_object_ref (plugin); + } + + return list; } typedef struct @@ -664,8 +649,7 @@ _feature_filter (GstPlugin * plugin, gpointer user_data) GList *result; FeatureFilterData *data = (FeatureFilterData *) user_data; - result = - gst_plugin_feature_filter (plugin, data->filter, data->first, + result = gst_plugin_feature_filter (plugin, data->filter, data->first, data->user_data); if (result) { data->result = g_list_concat (data->result, result); @@ -750,7 +734,7 @@ gst_plugin_find_feature (GstPlugin * plugin, const gchar * name, GType type) if (walk) result = GST_PLUGIN_FEATURE (walk->data); - g_list_free (walk); + gst_plugin_feature_list_free (walk); return result; } @@ -784,7 +768,7 @@ gst_plugin_find_feature_by_name (GstPlugin * plugin, const gchar * name) if (walk) result = GST_PLUGIN_FEATURE (walk->data); - g_list_free (walk); + gst_plugin_feature_list_free (walk); return result; } @@ -801,31 +785,18 @@ gst_plugin_find_feature_by_name (GstPlugin * plugin, const gchar * name) void gst_plugin_add_feature (GstPlugin * plugin, GstPluginFeature * feature) { - GstPluginFeature *oldfeature; - /* FIXME 0.9: get reference counting somewhat right in here, * GstPluginFeatures should probably be GstObjects that are sinked when * adding them to a plugin */ g_return_if_fail (plugin != NULL); g_return_if_fail (GST_IS_PLUGIN_FEATURE (feature)); g_return_if_fail (feature != NULL); + g_return_if_fail (feature->plugin == NULL); - oldfeature = gst_plugin_find_feature (plugin, - GST_PLUGIN_FEATURE_NAME (feature), G_OBJECT_TYPE (feature)); - - if (oldfeature == feature) { - GST_WARNING ("feature %s has already been added", - GST_PLUGIN_FEATURE_NAME (feature)); - /* g_object_unref (feature); */ - } else if (oldfeature) { - GST_WARNING ("feature %s already present in plugin", - GST_PLUGIN_FEATURE_NAME (feature)); - /* g_object_unref (feature); */ - } else { - feature->plugin = plugin; - plugin->features = g_list_prepend (plugin->features, feature); - plugin->numfeatures++; - } + /* gst_object_sink (feature); */ + feature->plugin = plugin; + plugin->features = g_list_prepend (plugin->features, feature); + plugin->numfeatures++; } /** @@ -839,11 +810,20 @@ gst_plugin_add_feature (GstPlugin * plugin, GstPluginFeature * feature) GList * gst_plugin_get_feature_list (GstPlugin * plugin) { + GList *list; + GList *g; + g_return_val_if_fail (plugin != NULL, NULL); - return g_list_copy (plugin->features); + list = g_list_copy (plugin->features); + for (g = list; g; g = g->next) { + gst_object_ref (plugin); + } + + return list; } +/* FIXME is this function necessary? */ /** * gst_plugin_load_1: * @name: name of plugin to load diff --git a/gst/gstplugin.h b/gst/gstplugin.h index 0298acc875..bfae206c46 100644 --- a/gst/gstplugin.h +++ b/gst/gstplugin.h @@ -29,6 +29,7 @@ #include #include #include +#include G_BEGIN_DECLS @@ -79,7 +80,7 @@ struct _GstPluginDesc { struct _GstPlugin { - GObject object; + GstObject object; GstPluginDesc desc; @@ -100,7 +101,7 @@ struct _GstPlugin { }; struct _GstPluginClass { - GObjectClass object_class; + GstObjectClass object_class; }; diff --git a/gst/gstpluginfeature.c b/gst/gstpluginfeature.c index 28c850967d..cee4e03a25 100644 --- a/gst/gstpluginfeature.c +++ b/gst/gstpluginfeature.c @@ -32,50 +32,20 @@ static void gst_plugin_feature_class_init (GstPluginFeatureClass * klass); static void gst_plugin_feature_init (GstPluginFeature * feature); -static GObjectClass *parent_class = NULL; - /* static guint gst_plugin_feature_signals[LAST_SIGNAL] = { 0 }; */ -GType -gst_plugin_feature_get_type (void) -{ - static GType plugin_feature_type = 0; - - if (!plugin_feature_type) { - static const GTypeInfo plugin_feature_info = { - sizeof (GObjectClass), - NULL, - NULL, - (GClassInitFunc) gst_plugin_feature_class_init, - NULL, - NULL, - sizeof (GObject), - 0, - (GInstanceInitFunc) gst_plugin_feature_init, - NULL - }; - - plugin_feature_type = - g_type_register_static (G_TYPE_OBJECT, "GstPluginFeature", - &plugin_feature_info, G_TYPE_FLAG_ABSTRACT); - } - return plugin_feature_type; -} +G_DEFINE_ABSTRACT_TYPE (GstPluginFeature, gst_plugin_feature, G_TYPE_OBJECT); static void gst_plugin_feature_class_init (GstPluginFeatureClass * klass) { - GObjectClass *gobject_class; - gobject_class = (GObjectClass *) klass; - - parent_class = g_type_class_ref (G_TYPE_OBJECT); } static void gst_plugin_feature_init (GstPluginFeature * feature) { - feature->plugin = NULL; + } /** @@ -198,3 +168,16 @@ gst_plugin_feature_get_rank (GstPluginFeature * feature) return feature->rank; } + +void +gst_plugin_feature_list_free (GList * list) +{ + GList *g; + + for (g = list; g; g = g->next) { + GstPluginFeature *feature = GST_PLUGIN_FEATURE (g->data); + + gst_object_unref (feature->plugin); + } + g_list_free (list); +} diff --git a/gst/gstpluginfeature.h b/gst/gstpluginfeature.h index c85d0f7a88..1ae5545672 100644 --- a/gst/gstpluginfeature.h +++ b/gst/gstpluginfeature.h @@ -91,6 +91,8 @@ void gst_plugin_feature_set_name (GstPluginFeature *feature, const gchar *name guint gst_plugin_feature_get_rank (GstPluginFeature *feature); G_CONST_RETURN gchar *gst_plugin_feature_get_name (GstPluginFeature *feature); +void gst_plugin_feature_list_free (GList *list); + G_END_DECLS diff --git a/gst/gstregistry.c b/gst/gstregistry.c index 5803b0f78e..b72560ee06 100644 --- a/gst/gstregistry.c +++ b/gst/gstregistry.c @@ -120,30 +120,7 @@ static void gst_registry_init (GstRegistry * registry); static GObjectClass *parent_class = NULL; static guint gst_registry_signals[LAST_SIGNAL] = { 0 }; -GType -gst_registry_get_type (void) -{ - static GType registry_type = 0; - - if (!registry_type) { - static const GTypeInfo registry_info = { - sizeof (GstRegistryClass), - NULL, - NULL, - (GClassInitFunc) gst_registry_class_init, - NULL, - NULL, - sizeof (GstRegistry), - 0, - (GInstanceInitFunc) gst_registry_init, - NULL - }; - - registry_type = g_type_register_static (G_TYPE_OBJECT, "GstRegistry", - ®istry_info, 0); - } - return registry_type; -} +G_DEFINE_TYPE (GstRegistry, gst_registry, G_TYPE_OBJECT); static void gst_registry_class_init (GstRegistryClass * klass) @@ -263,15 +240,21 @@ gst_registry_add_plugin (GstRegistry * registry, GstPlugin * plugin) GST_DEBUG ("Replacing existing plugin for filename \"%s\"", plugin->filename); registry->plugins = g_list_remove (registry->plugins, existing_plugin); - g_object_unref (existing_plugin); + gst_object_unref (existing_plugin); } registry->plugins = g_list_prepend (registry->plugins, plugin); + gst_object_ref (plugin); + gst_object_sink (plugin); + GST_DEBUG ("emitting plugin-added for filename %s", plugin->filename); g_signal_emit (G_OBJECT (registry), gst_registry_signals[PLUGIN_ADDED], 0, plugin); + /* FIXME hack to fix unref later */ + gst_object_ref (plugin); + return TRUE; } @@ -539,7 +522,7 @@ _gst_registry_remove_cache_plugins (GstRegistry * registry) plugin = g->data; if (plugin->flags & GST_PLUGIN_FLAG_CACHED) { registry->plugins = g_list_remove (registry->plugins, plugin); - g_object_unref (plugin); + gst_object_unref (plugin); } g = g_next; }