rebuild registry only for newer plugins. fixes #145520

Original commit message from CVS:
rebuild registry only for newer plugins.  fixes #145520

ChangeLog

@@ -1,3 +1,13 @@
+2004-07-06  Thomas Vander Stichele  <thomas at apestaart dot org>
+
+	* gst/gstplugin.c: (gst_plugin_check_file), (gst_plugin_load_file):
+	* gst/gstplugin.h:
+	* gst/registries/gstxmlregistry.c:
+	(plugin_times_older_than_recurse), (plugin_times_older_than),
+	(gst_xml_registry_parse_padtemplate):
+	  only rebuild registry when actual plugins have a newer time than
+	  the registry.  Fixes #145520
+
 2004-07-06  Thomas Vander Stichele  <thomas at apestaart dot org>
 
 	* docs/manual/manual.xml:

gst/gstplugin.c

@@ -296,6 +296,62 @@ _gst_plugin_fault_handler_setup (void)
 
 static void _gst_plugin_fault_handler_setup ();
 
+/**
+ * gst_plugin_check_file:
+ * @filename: the plugin filename to check for pluginness
+ * @error: pointer to a NULL-valued GError
+ *
+ * Checks if the given path represents a GStreamer plugin.
+ *
+ * Returns: TRUE if the given path is a GStreamer plugin.
+ */
+gboolean
+gst_plugin_check_file (const gchar * filename, GError ** error)
+{
+  GModule *module;
+  struct stat file_status;
+
+  g_return_val_if_fail (filename != NULL, FALSE);
+
+  if (g_module_supported () == FALSE) {
+    g_set_error (error,
+        GST_PLUGIN_ERROR,
+        GST_PLUGIN_ERROR_MODULE, "Dynamic loading not supported");
+    return FALSE;
+  }
+
+  if (stat (filename, &file_status)) {
+    g_set_error (error,
+        GST_PLUGIN_ERROR,
+        GST_PLUGIN_ERROR_MODULE, "Problem opening file %s\n", filename);
+    return FALSE;
+  }
+
+  module = g_module_open (filename, G_MODULE_BIND_LAZY);
+
+  if (module == NULL) {
+    GST_DEBUG ("Error loading plugin %s, reason: %s\n", filename,
+        g_module_error ());
+    g_set_error (error, GST_PLUGIN_ERROR, GST_PLUGIN_ERROR_MODULE,
+        "Error loading plugin %s, reason: %s\n", filename, g_module_error ());
+    return FALSE;
+  }
+  gpointer ptr;
+
+  if (!g_module_symbol (module, "gst_plugin_desc", &ptr)) {
+    GST_DEBUG ("Could not find plugin entry point in \"%s\"", filename);
+    g_set_error (error,
+        GST_PLUGIN_ERROR,
+        GST_PLUGIN_ERROR_MODULE,
+        "Could not find plugin entry point in \"%s\"", filename);
+    g_module_close (module);
+    return FALSE;
+  }
+  /* it's a plugin */
+  g_module_close (module);
+  return TRUE;
+}
+
 /**
  * gst_plugin_load_file:
  * @filename: the plugin filename to load
@@ -311,115 +367,94 @@ gst_plugin_load_file (const gchar * filename, GError ** error)
   GstPlugin *plugin;
   GModule *module;
   GstPluginDesc *desc;
-  struct stat file_status;
   gboolean free_plugin;
+  gpointer ptr;
 
   g_return_val_if_fail (filename != NULL, NULL);
 
   GST_CAT_DEBUG (GST_CAT_PLUGIN_LOADING, "attempt to load plugin \"%s\"",
       filename);
 
-  if (g_module_supported () == FALSE) {
+  if (!gst_plugin_check_file (filename, error))
-    g_set_error (error,
-        GST_PLUGIN_ERROR,
-        GST_PLUGIN_ERROR_MODULE, "Dynamic loading not supported");
     return NULL;
-  }
-
-  if (stat (filename, &file_status)) {
-    g_set_error (error,
-        GST_PLUGIN_ERROR,
-        GST_PLUGIN_ERROR_MODULE, "Problem opening file %s\n", filename);
-    return NULL;
-  }
 
   module = g_module_open (filename, G_MODULE_BIND_LAZY);
 
-  if (module != NULL) {
+  if (module == NULL)
-    gpointer ptr;
+    goto load_error;
 
-    if (g_module_symbol (module, "gst_plugin_desc", &ptr)) {
+  if (!g_module_symbol (module, "gst_plugin_desc", &ptr))
-      desc = (GstPluginDesc *) ptr;
+    goto load_error;
 
-      plugin = gst_registry_pool_find_plugin (desc->name);
+  desc = (GstPluginDesc *) ptr;
-      if (!plugin) {
-        free_plugin = TRUE;
-        plugin = g_new0 (GstPlugin, 1);
-        plugin->filename = g_strdup (filename);
-        GST_DEBUG ("created new GstPlugin %p for file \"%s\"", plugin,
-            filename);
-      } else {
-        free_plugin = FALSE;
-        if (gst_plugin_is_loaded (plugin)) {
-          if (plugin->filename && strcmp (plugin->filename, filename) != 0) {
-            GST_WARNING
-                ("plugin %p from file \"%s\" with same name %s is already "
-                "loaded, aborting loading of \"%s\"", plugin, plugin->filename,
-                plugin->desc.name, filename);
-            g_set_error (error, GST_PLUGIN_ERROR,
-                GST_PLUGIN_ERROR_NAME_MISMATCH,
-                "plugin %p from file \"%s\" with same name %s is already "
-                "loaded, aborting loading of \"%s\"", plugin, plugin->filename,
-                plugin->desc.name, filename);
-            if (free_plugin)
-              g_free (plugin);
-            return NULL;
-          }
-          GST_LOG ("Plugin %p for file \"%s\" already loaded, returning it now",
-              plugin, filename);
-          return plugin;
-        }
-      }
-      GST_LOG ("Plugin %p for file \"%s\" prepared, calling entry function...",
-          plugin, filename);
 
-      if (g_module_symbol (module, "plugin_init", &ptr)) {
+  plugin = gst_registry_pool_find_plugin (desc->name);
-        g_print
+  if (!plugin) {
-            ("plugin %p from file \"%s\" exports a symbol named plugin_init\n",
+    free_plugin = TRUE;
-            plugin, plugin->filename);
+    plugin = g_new0 (GstPlugin, 1);
-        g_set_error (error, GST_PLUGIN_ERROR, GST_PLUGIN_ERROR_NAME_MISMATCH,
+    plugin->filename = g_strdup (filename);
-            "plugin \"%s\" exports a symbol named plugin_init", desc->name);
+    GST_DEBUG ("created new GstPlugin %p for file \"%s\"", plugin, filename);
-      }
+  } else {
-
+    free_plugin = FALSE;
-      /* this is where we load the actual .so, so let's trap SIGSEGV */
+    if (gst_plugin_is_loaded (plugin)) {
-      _gst_plugin_fault_handler_setup ();
+      if (plugin->filename && strcmp (plugin->filename, filename) != 0) {
-      _gst_plugin_fault_handler_filename = plugin->filename;
+        GST_WARNING
-
+            ("plugin %p from file \"%s\" with same name %s is already "
-      if (gst_plugin_register_func (plugin, module, desc)) {
+            "loaded, aborting loading of \"%s\"", plugin, plugin->filename,
-        /* remove signal handler */
+            plugin->desc.name, filename);
-        _gst_plugin_fault_handler_restore ();
+        g_set_error (error, GST_PLUGIN_ERROR,
-        _gst_plugin_fault_handler_filename = NULL;
+            GST_PLUGIN_ERROR_NAME_MISMATCH,
-        GST_INFO ("plugin \"%s\" loaded", plugin->filename);
+            "plugin %p from file \"%s\" with same name %s is already "
-        return plugin;
+            "loaded, aborting loading of \"%s\"", plugin, plugin->filename,
-      } else {
+            plugin->desc.name, filename);
-        /* remove signal handler */
-        _gst_plugin_fault_handler_restore ();
-        GST_DEBUG ("gst_plugin_register_func failed for plugin \"%s\"",
-            filename);
-        /* plugin == NULL */
-        g_set_error (error,
-            GST_PLUGIN_ERROR,
-            GST_PLUGIN_ERROR_MODULE,
-            "gst_plugin_register_func failed for plugin \"%s\"", filename);
         if (free_plugin)
           g_free (plugin);
         return NULL;
       }
-    } else {
+      GST_LOG ("Plugin %p for file \"%s\" already loaded, returning it now",
-      GST_DEBUG ("Could not find plugin entry point in \"%s\"", filename);
+          plugin, filename);
-      g_set_error (error,
+      return plugin;
-          GST_PLUGIN_ERROR,
-          GST_PLUGIN_ERROR_MODULE,
-          "Could not find plugin entry point in \"%s\"", filename);
     }
-    return NULL;
+  }
+  GST_LOG ("Plugin %p for file \"%s\" prepared, calling entry function...",
+      plugin, filename);
+
+  if (g_module_symbol (module, "plugin_init", &ptr)) {
+    g_print
+        ("plugin %p from file \"%s\" exports a symbol named plugin_init\n",
+        plugin, plugin->filename);
+    g_set_error (error, GST_PLUGIN_ERROR, GST_PLUGIN_ERROR_NAME_MISMATCH,
+        "plugin \"%s\" exports a symbol named plugin_init", desc->name);
+  }
+
+  /* this is where we load the actual .so, so let's trap SIGSEGV */
+  _gst_plugin_fault_handler_setup ();
+  _gst_plugin_fault_handler_filename = plugin->filename;
+
+  if (gst_plugin_register_func (plugin, module, desc)) {
+    /* remove signal handler */
+    _gst_plugin_fault_handler_restore ();
+    _gst_plugin_fault_handler_filename = NULL;
+    GST_INFO ("plugin \"%s\" loaded", plugin->filename);
+    return plugin;
   } else {
-    GST_DEBUG ("Error loading plugin %s, reason: %s\n", filename,
+    /* remove signal handler */
-        g_module_error ());
+    _gst_plugin_fault_handler_restore ();
-    g_set_error (error, GST_PLUGIN_ERROR, GST_PLUGIN_ERROR_MODULE,
+    GST_DEBUG ("gst_plugin_register_func failed for plugin \"%s\"", filename);
-        "Error loading plugin %s, reason: %s\n", filename, g_module_error ());
+    /* plugin == NULL */
+    g_set_error (error,
+        GST_PLUGIN_ERROR,
+        GST_PLUGIN_ERROR_MODULE,
+        "gst_plugin_register_func failed for plugin \"%s\"", filename);
+    if (free_plugin)
+      g_free (plugin);
     return NULL;
   }
+load_error:
+  g_set_error (error,
+      GST_PLUGIN_ERROR,
+      GST_PLUGIN_ERROR_MODULE, "generic load error for \"%s\"", filename);
+  return NULL;
 }
 
 static void

gst/gstplugin.h

@@ -149,6 +149,7 @@ gboolean		gst_plugin_name_filter		(GstPlugin *plugin, const gchar *name);
 GList*			gst_plugin_get_feature_list	(GstPlugin *plugin);
 GstPluginFeature*	gst_plugin_find_feature		(GstPlugin *plugin, const gchar *name, GType type);
 
+gboolean 		gst_plugin_check_file		(const gchar *filename, GError** error);
 GstPlugin * 		gst_plugin_load_file		(const gchar *filename, GError** error);
 gboolean 		gst_plugin_unload_plugin	(GstPlugin *plugin);
 

gst/registries/gstxmlregistry.c

@@ -299,6 +299,7 @@ gst_xml_registry_get_property (GObject * object, guint prop_id,
  * mtime is updated through an actual write (data)
  * ctime is updated through changing inode information
  * so this function returns the last time *anything* changed to this path
+ * it also sets the given boolean to TRUE if the given path is a directory
  */
 static time_t
 get_time (const char *path, gboolean * is_dir)
@@ -430,52 +431,75 @@ finished:
   g_free (text);
 }
 
+/* return TRUE iff regtime is more recent than the times of all the .so files
+ * in the plugin dirs; ie return TRUE if this path does not need to trigger
+ * a rebuild of registry
+ *
+ * - if it's a directory, recurse on subdirs
+ * - if it's a file
+ *   - if entry is not newer, return TRUE.
+ *   - if it's newer
+ *     - and it's a plugin, return FALSE
+ *     - otherwise return TRUE
+ */
 static gboolean
 plugin_times_older_than_recurse (gchar * path, time_t regtime)
 {
   DIR *dir;
   struct dirent *dirent;
   gboolean is_dir;
-  gchar *pluginname;
+  gchar *new_path;
 
   time_t pathtime = get_time (path, &is_dir);
 
-  if (pathtime > regtime) {
+  if (is_dir) {
-    GST_CAT_INFO (GST_CAT_PLUGIN_LOADING,
+    dir = opendir (path);
-        "time for %s was %ld; more recent than registry time of %ld\n",
+    if (dir) {
-        path, (long) pathtime, (long) regtime);
+      while ((dirent = readdir (dir))) {
-    return FALSE;
+        /* don't want to recurse in place or backwards */
+        if (strcmp (dirent->d_name, ".") && strcmp (dirent->d_name, "..")) {
+          new_path = g_build_filename (path, dirent->d_name, NULL);
+          if (!plugin_times_older_than_recurse (new_path, regtime)) {
+            GST_CAT_INFO (GST_CAT_PLUGIN_LOADING,
+                "path %s is more recent than registry time of %ld",
+                new_path, (long) regtime);
+            g_free (new_path);
+            closedir (dir);
+            return FALSE;
+          }
+          g_free (new_path);
+        }
+      }
+      closedir (dir);
+    }
+    return TRUE;
   }
 
-  if (!is_dir)
+  /* it's a file */
+  if (pathtime <= regtime) {
     return TRUE;
+  }
 
-  dir = opendir (path);
+  /* it's a file, and it's more recent */
-  if (dir) {
+  if (g_str_has_suffix (path, ".so") || g_str_has_suffix (path, ".dll")) {
-    while ((dirent = readdir (dir))) {
+    if (!gst_plugin_check_file (path, NULL))
-      /* don't want to recurse in place or backwards */
+      return TRUE;
-      if (strcmp (dirent->d_name, ".") && strcmp (dirent->d_name, "..")) {
+
-        pluginname = g_strjoin ("/", path, dirent->d_name, NULL);
+    /* it's a newer GStreamer plugin */
-        if (!plugin_times_older_than_recurse (pluginname, regtime)) {
+    GST_CAT_INFO (GST_CAT_PLUGIN_LOADING,
-          g_free (pluginname);
+        "%s looks like a plugin and is more recent than registry time of %ld",
-          closedir (dir);
+        path, (long) regtime);
-          return FALSE;
+    return FALSE;
-        }
-        g_free (pluginname);
-      }
-    }
-    closedir (dir);
   }
   return TRUE;
 }
 
+/* return TRUE iff regtime is more recent than the times of all the .so files
+ * in the plugin dirs; ie return TRUE if registry is up to date.
+ */
 static gboolean
 plugin_times_older_than (GList * paths, time_t regtime)
 {
-  /* return true iff regtime is more recent than the times of all the files
-   * in the plugin dirs.
-   */
-
   while (paths) {
     GST_CAT_LOG (GST_CAT_PLUGIN_LOADING,
         "comparing plugin times from %s with %ld",
@@ -484,6 +508,8 @@ plugin_times_older_than (GList * paths, time_t regtime)
       return FALSE;
     paths = g_list_next (paths);
   }
+  GST_CAT_LOG (GST_CAT_PLUGIN_LOADING,
+      "everything's fine, no registry rebuild needed.");
   return TRUE;
 }
 
@@ -880,7 +906,7 @@ gst_xml_registry_parse_padtemplate (GMarkupParseContext * context,
     g_assert (registry->caps == NULL);
     registry->caps = gst_caps_from_string (s);
     if (registry->caps == NULL) {
-      g_critical ("Could not parse caps: length %d, content: %*s\n",
+      g_critical ("Could not parse caps: length %d, content: %*s",
           (int) text_len, (int) text_len, text);
     }
     g_free (s);