gst/: Re-commit the registry changes, along with an extra fix:
authorJan Schmidt <thaytan@mad.scientist.com>
Thu, 28 Sep 2006 14:00:43 +0000 (14:00 +0000)
committerJan Schmidt <thaytan@mad.scientist.com>
Thu, 28 Sep 2006 14:00:43 +0000 (14:00 +0000)
Original commit message from CVS:
* gst/gst.c: (init_pre), (scan_and_update_registry),
(ensure_current_registry_nonforking),
(ensure_current_registry_forking), (ensure_current_registry),
(init_post), (gst_debug_help), (gst_deinit):
* gst/gst_private.h:
* gst/gstregistry.c: (gst_registry_finalize),
(gst_registry_remove_features_for_plugin_unlocked),
(gst_registry_remove_plugin), (gst_registry_scan_path_level),
(gst_registry_scan_path),
(_priv_gst_registry_remove_cache_plugins),
(_priv_gst_registry_cleanup):
* gst/gstregistry.h:
Re-commit the registry changes, along with an extra fix:
When a cached plugin is encountered at a different file path,
update the stored path in the registry cache so that the parent
process knows where it actually is now when it re-reads the registry
cache. Fixes the thing that broke distcheck with the previous commit.
* tests/check/Makefile.am:
Clean up files named 'core' too when running make clean.
* tests/examples/manual/Makefile.am:
Set up a registry path for running these tests, and clean it properly
for distcheck.

ChangeLog
gst/gst.c
gst/gst_private.h
gst/gstregistry.c
gst/gstregistry.h
tests/check/Makefile.am
tests/examples/manual/Makefile.am

index 529311e..085f495 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,32 @@
 2006-09-28  Jan Schmidt  <thaytan@mad.scientist.com>
 
+       * gst/gst.c: (init_pre), (scan_and_update_registry),
+       (ensure_current_registry_nonforking),
+       (ensure_current_registry_forking), (ensure_current_registry),
+       (init_post), (gst_debug_help), (gst_deinit):
+       * gst/gst_private.h:
+       * gst/gstregistry.c: (gst_registry_finalize),
+       (gst_registry_remove_features_for_plugin_unlocked),
+       (gst_registry_remove_plugin), (gst_registry_scan_path_level),
+       (gst_registry_scan_path),
+       (_priv_gst_registry_remove_cache_plugins),
+       (_priv_gst_registry_cleanup):
+       * gst/gstregistry.h:
+       Re-commit the registry changes, along with an extra fix:
+         When a cached plugin is encountered at a different file path,
+         update the stored path in the registry cache so that the parent
+         process knows where it actually is now when it re-reads the registry
+         cache. Fixes the thing that broke distcheck with the previous commit.
+
+       * tests/check/Makefile.am:
+       Clean up files named 'core' too when running make clean.
+
+       * tests/examples/manual/Makefile.am:
+       Set up a registry path for running these tests, and clean it properly
+       for distcheck.
+
+2006-09-28  Jan Schmidt  <thaytan@mad.scientist.com>
+
        * configure.ac:
        Don't pull in gmodule-2.0.pc as a dependency in our .pc files - we
        want gmodule-no-export-2.0.pc instead so that we don't drag in
index 955ef87..3e372ec 100644 (file)
--- a/gst/gst.c
+++ b/gst/gst.c
@@ -142,8 +142,10 @@ gboolean _gst_disable_segtrap = FALSE;
 static gboolean _gst_enable_registry_fork = DEFAULT_FORK;
 
 static void load_plugin_func (gpointer data, gpointer user_data);
-static gboolean init_pre (void);
-static gboolean init_post (void);
+static gboolean init_pre (GOptionContext * context, GOptionGroup * group,
+    gpointer data, GError ** error);
+static gboolean init_post (GOptionContext * context, GOptionGroup * group,
+    gpointer data, GError ** error);
 static gboolean parse_goption_arg (const gchar * s_opt,
     const gchar * arg, gpointer data, GError ** err);
 
@@ -486,7 +488,8 @@ split_and_iterate (const gchar * stringlist, gchar * separator, GFunc iterator,
 
 /* we have no fail cases yet, but maybe in the future */
 static gboolean
-init_pre (void)
+init_pre (GOptionContext * context, GOptionGroup * group, gpointer data,
+    GError ** error)
 {
   /* GStreamer was built against a GLib >= 2.8 and is therefore not doing
    * the refcount hack. Check that it isn't being run against an older GLib */
@@ -577,7 +580,7 @@ static GstPluginDesc plugin_desc = {
  */
 static gboolean
 scan_and_update_registry (GstRegistry * default_registry,
-    const gchar * registry_file, gboolean write_changes)
+    const gchar * registry_file, gboolean write_changes, GError ** error)
 {
   const gchar *plugin_path;
   gboolean changed = FALSE;
@@ -644,34 +647,38 @@ scan_and_update_registry (GstRegistry * default_registry,
     g_strfreev (list);
   }
 
+  /* Remove cached plugins so stale info is cleared. */
+  changed |= _priv_gst_registry_remove_cache_plugins (default_registry);
+
   if (!changed) {
-    GST_DEBUG ("registry cache has not changed");
+    GST_INFO ("Registry cache has not changed");
     return TRUE;
   }
 
   if (!write_changes) {
-    GST_DEBUG ("not trying to write registry cache changes to file");
+    GST_INFO ("Registry cached changed, but writing is disabled. Not writing.");
     return TRUE;
   }
 
-  GST_DEBUG ("writing registry cache");
+  GST_INFO ("Registry cache changed. Writing new registry cache");
   if (!gst_registry_xml_write_cache (default_registry, registry_file)) {
-    g_warning ("Problem writing registry cache to %s: %s", registry_file,
-        g_strerror (errno));
+    g_set_error (error, GST_CORE_ERROR, GST_CORE_ERROR_FAILED,
+        _("Error writing registry cache to %s: %s"),
+        registry_file, g_strerror (errno));
     return FALSE;
   }
 
-  GST_DEBUG ("registry cache written successfully");
+  GST_INFO ("Registry cache written successfully");
   return TRUE;
 }
 
 static gboolean
 ensure_current_registry_nonforking (GstRegistry * default_registry,
-    const gchar * registry_file)
+    const gchar * registry_file, GError ** error)
 {
   /* fork() not available */
-  GST_DEBUG ("updating registry cache");
-  scan_and_update_registry (default_registry, registry_file, TRUE);
+  GST_DEBUG ("Updating registry cache in-process");
+  scan_and_update_registry (default_registry, registry_file, TRUE, error);
   return TRUE;
 }
 
@@ -679,14 +686,14 @@ ensure_current_registry_nonforking (GstRegistry * default_registry,
  * TRUE immediatly */
 static gboolean
 ensure_current_registry_forking (GstRegistry * default_registry,
-    const gchar * registry_file)
+    const gchar * registry_file, GError ** error)
 {
 #ifdef HAVE_FORK
   pid_t pid;
 
   /* We fork here, and let the child read and possibly rebuild the registry.
    * After that, the parent will re-read the freshly generated registry. */
-  GST_DEBUG ("forking");
+  GST_DEBUG ("forking to update registry");
   pid = fork ();
   if (pid == -1) {
     GST_ERROR ("Failed to fork()");
@@ -698,8 +705,8 @@ ensure_current_registry_forking (GstRegistry * default_registry,
 
     /* this is the child */
     GST_DEBUG ("child reading registry cache");
-    res = scan_and_update_registry (default_registry, registry_file, TRUE);
-    _gst_registry_remove_cache_plugins (default_registry);
+    res =
+        scan_and_update_registry (default_registry, registry_file, TRUE, NULL);
 
     /* need to use _exit, so that any exit handlers registered don't
      * bring down the main program */
@@ -719,6 +726,9 @@ ensure_current_registry_forking (GstRegistry * default_registry,
     GST_DEBUG ("parent done waiting on child");
     if (ret == -1) {
       GST_ERROR ("error during waitpid: %s", g_strerror (errno));
+      g_set_error (error, GST_CORE_ERROR, GST_CORE_ERROR_FAILED,
+          _("Error re-scanning registry %s: %s"),
+          ", waitpid returned error", g_strerror (errno));
       return FALSE;
     }
 
@@ -726,8 +736,14 @@ ensure_current_registry_forking (GstRegistry * default_registry,
       if (WIFSIGNALED (status)) {
         GST_ERROR ("child did not exit normally, terminated by signal %d",
             WTERMSIG (status));
+        g_set_error (error, GST_CORE_ERROR, GST_CORE_ERROR_FAILED,
+            _("Error re-scanning registry %s: %d"),
+            ", child terminated by signal", WTERMSIG (status));
       } else {
         GST_ERROR ("child did not exit normally, status: %d", status);
+        g_set_error (error, GST_CORE_ERROR, GST_CORE_ERROR_FAILED,
+            _("Error re-scanning registry %s: %d"),
+            ", child did not exit normally, status", status);
       }
       return FALSE;
     }
@@ -739,8 +755,8 @@ ensure_current_registry_forking (GstRegistry * default_registry,
       GST_DEBUG ("parent reading registry cache");
       gst_registry_xml_read_cache (default_registry, registry_file);
     } else {
-      GST_DEBUG ("parent re-scanning registry");
-      scan_and_update_registry (default_registry, registry_file, FALSE);
+      GST_DEBUG ("parent re-scanning registry. Ignoring errors.");
+      scan_and_update_registry (default_registry, registry_file, FALSE, NULL);
     }
   }
 #endif /* HAVE_FORK */
@@ -748,7 +764,7 @@ ensure_current_registry_forking (GstRegistry * default_registry,
 }
 
 static gboolean
-ensure_current_registry (void)
+ensure_current_registry (GError ** error)
 {
   char *registry_file;
   GstRegistry *default_registry;
@@ -777,10 +793,12 @@ ensure_current_registry (void)
   /* now check registry with or without forking */
   if (do_fork) {
     GST_DEBUG ("forking for registry rebuild");
-    ret = ensure_current_registry_forking (default_registry, registry_file);
+    ret = ensure_current_registry_forking (default_registry, registry_file,
+        error);
   } else {
     GST_DEBUG ("requested not to fork for registry rebuild");
-    ret = ensure_current_registry_nonforking (default_registry, registry_file);
+    ret = ensure_current_registry_nonforking (default_registry, registry_file,
+        error);
   }
 
   g_free (registry_file);
@@ -801,7 +819,8 @@ ensure_current_registry (void)
  *   we might and then it's nice to be able to return that
  */
 static gboolean
-init_post (void)
+init_post (GOptionContext * context, GOptionGroup * group, gpointer data,
+    GError ** error)
 {
   GLogLevelFlags llf;
 
@@ -854,7 +873,7 @@ init_post (void)
   gst_initialized = TRUE;
 
 #ifndef GST_DISABLE_REGISTRY
-  if (!ensure_current_registry ())
+  if (!ensure_current_registry (error))
     return FALSE;
 #endif /* GST_DISABLE_REGISTRY */
 
@@ -894,7 +913,8 @@ gst_debug_help (void)
   GSList *list, *walk;
   GList *list2, *g;
 
-  if (!init_post ())
+  /* Need to ensure the registry is loaded to get debug categories */
+  if (!init_post (NULL, NULL, NULL, NULL))
     exit (1);
 
   list2 = gst_registry_plugin_filter (gst_registry_get_default (),
@@ -1074,7 +1094,7 @@ gst_deinit (void)
   gst_object_unref (clock);
   gst_object_unref (clock);
 
-  _gst_registry_cleanup ();
+  _priv_gst_registry_cleanup ();
 
   gst_initialized = FALSE;
   GST_INFO ("deinitialized GStreamer");
index b9b398a..2b8b4ac 100644 (file)
@@ -37,6 +37,9 @@ extern const char             g_log_domain_gstreamer[];
 #include <stdlib.h>
 #include <string.h>
 
+/* Needed for GstRegistry * */
+#include "gstregistry.h"
+
 G_BEGIN_DECLS
 
 gboolean __gst_in_valgrind (void);
@@ -44,6 +47,10 @@ gboolean __gst_in_valgrind (void);
 /* Initialize GStreamer private quark storage */
 void _priv_gst_quarks_initialize (void);
 
+/* Private registry functions */
+gboolean _priv_gst_registry_remove_cache_plugins (GstRegistry *registry);
+void _priv_gst_registry_cleanup (void);
+
 /*** debugging categories *****************************************************/
 
 #ifndef GST_DISABLE_GST_DEBUG
index 3be0e8a..503690e 100644 (file)
@@ -180,7 +180,7 @@ gst_registry_finalize (GObject * object)
     if (plugin) {
       GST_DEBUG_OBJECT (registry, "removing plugin %s",
           gst_plugin_get_name (plugin));
-      gst_registry_remove_plugin (registry, plugin);
+      gst_object_unref (plugin);
     }
     p = g_list_next (p);
   }
@@ -194,7 +194,9 @@ gst_registry_finalize (GObject * object)
     GstPluginFeature *feature = f->data;
 
     if (feature) {
-      gst_registry_remove_feature (registry, feature);
+      GST_DEBUG_OBJECT (registry, "removing feature %p (%s)",
+          feature, gst_plugin_feature_get_name (feature));
+      gst_object_unref (feature);
     }
     f = g_list_next (f);
   }
@@ -344,6 +346,33 @@ gst_registry_add_plugin (GstRegistry * registry, GstPlugin * plugin)
   return TRUE;
 }
 
+static void
+gst_registry_remove_features_for_plugin_unlocked (GstRegistry * registry,
+    GstPlugin * plugin)
+{
+  GList *f;
+
+  g_return_if_fail (GST_IS_REGISTRY (registry));
+  g_return_if_fail (GST_IS_PLUGIN (plugin));
+
+  /* Remove all features for this plugin */
+  f = registry->features;
+  while (f != NULL) {
+    GList *next = g_list_next (f);
+    GstPluginFeature *feature = f->data;
+
+    if (feature && !strcmp (feature->plugin_name, gst_plugin_get_name (plugin))) {
+      GST_DEBUG_OBJECT (registry, "removing feature %p (%s) for plugin %s",
+          feature, gst_plugin_feature_get_name (feature),
+          gst_plugin_get_name (plugin));
+
+      registry->features = g_list_delete_link (registry->features, f);
+      gst_object_unref (feature);
+    }
+    f = next;
+  }
+}
+
 /**
  * gst_registry_remove_plugin:
  * @registry: the registry to remove the plugin from
@@ -359,8 +388,12 @@ gst_registry_remove_plugin (GstRegistry * registry, GstPlugin * plugin)
   g_return_if_fail (GST_IS_REGISTRY (registry));
   g_return_if_fail (GST_IS_PLUGIN (plugin));
 
+  GST_DEBUG_OBJECT (registry, "removing plugin %p (%s)",
+      plugin, gst_plugin_get_name (plugin));
+
   GST_OBJECT_LOCK (registry);
   registry->plugins = g_list_remove (registry->plugins, plugin);
+  gst_registry_remove_features_for_plugin_unlocked (registry, plugin);
   GST_OBJECT_UNLOCK (registry);
   gst_object_unref (plugin);
 }
@@ -793,8 +826,10 @@ gst_registry_scan_path_level (GstRegistry * registry, const gchar * path,
       struct stat file_status;
 
       if (stat (filename, &file_status)) {
-        /* FIXME remove from cache */
+        /* Plugin will be removed from cache after the scan completes if it
+         * is still marked 'cached' */
         g_free (filename);
+        gst_object_unref (plugin);
         continue;
       }
       if (plugin->registered) {
@@ -809,8 +844,16 @@ gst_registry_scan_path_level (GstRegistry * registry, const gchar * path,
           plugin->file_size == file_status.st_size) {
         GST_DEBUG_OBJECT (registry, "file %s cached", filename);
         plugin->flags &= ~GST_PLUGIN_FLAG_CACHED;
-        GST_DEBUG_OBJECT (registry, "marking plugin %p as registered", plugin);
+        GST_DEBUG_OBJECT (registry, "marking plugin %p as registered as %s",
+            plugin, filename);
         plugin->registered = TRUE;
+        /* Update the file path on which we've seen this cached plugin 
+         * to ensure the registry cache will reflect up to date information */
+        if (strcmp (plugin->filename, filename) != 0) {
+          g_free (plugin->filename);
+          plugin->filename = g_strdup (filename);
+          changed = TRUE;
+        }
       } else {
         GST_INFO_OBJECT (registry, "cached info for %s is stale", filename);
         GST_DEBUG_OBJECT (registry, "mtime %ld != %ld or size %"
@@ -844,8 +887,6 @@ gst_registry_scan_path_level (GstRegistry * registry, const gchar * path,
 
   g_dir_close (dir);
 
-  GST_DEBUG_OBJECT (registry, "registry changed? %d", changed);
-
   return changed;
 }
 
@@ -871,19 +912,22 @@ gst_registry_scan_path (GstRegistry * registry, const gchar * path)
   GST_DEBUG_OBJECT (registry, "scanning path %s", path);
   changed = gst_registry_scan_path_level (registry, path, 10);
 
-  GST_DEBUG_OBJECT (registry, "registry changed? %d", changed);
+  GST_DEBUG_OBJECT (registry, "registry changed in path %s: %d", path, changed);
 
   return changed;
 }
 
-void
-_gst_registry_remove_cache_plugins (GstRegistry * registry)
+/* Unref all plugins marked 'cached', to clear old plugins that no
+ * longer exist. Returns TRUE if any plugins were removed */
+gboolean
+_priv_gst_registry_remove_cache_plugins (GstRegistry * registry)
 {
   GList *g;
   GList *g_next;
   GstPlugin *plugin;
+  gboolean changed = FALSE;
 
-  g_return_if_fail (GST_IS_REGISTRY (registry));
+  g_return_val_if_fail (GST_IS_REGISTRY (registry), FALSE);
 
   GST_OBJECT_LOCK (registry);
 
@@ -895,16 +939,16 @@ _gst_registry_remove_cache_plugins (GstRegistry * registry)
     if (plugin->flags & GST_PLUGIN_FLAG_CACHED) {
       GST_DEBUG_OBJECT (registry, "removing cached plugin \"%s\"",
           GST_STR_NULL (plugin->filename));
-      /* seems it would be sufficient just to do a delete_link for o(1) deletion
-       * -- we have to traverse the whole list anyway, and dup entries (if
-       * possible) should have dup refcounts */
-      registry->plugins = g_list_remove (registry->plugins, plugin);
+      registry->plugins = g_list_delete_link (registry->plugins, g);
       gst_object_unref (plugin);
+      changed = TRUE;
     }
     g = g_next;
   }
 
   GST_OBJECT_UNLOCK (registry);
+
+  return changed;
 }
 
 
@@ -935,8 +979,9 @@ gst_registry_get_feature_list_by_plugin (GstRegistry * registry,
       _gst_plugin_feature_filter_plugin_name, FALSE, (gpointer) name);
 }
 
+/* Unref and delete the default registry */
 void
-_gst_registry_cleanup ()
+_priv_gst_registry_cleanup ()
 {
   GstRegistry *registry;
 
index 3c65e00..743e058 100644 (file)
@@ -109,10 +109,6 @@ GstPluginFeature *         gst_registry_lookup_feature     (GstRegistry *registry, const c
 gboolean               gst_registry_xml_read_cache     (GstRegistry * registry, const char *location);
 gboolean               gst_registry_xml_write_cache    (GstRegistry * registry, const char *location);
 
-void                   _gst_registry_remove_cache_plugins (GstRegistry *registry);
-
-void                    _gst_registry_cleanup           (void);
-
 /* convinience defines for the default registry */
 
 /**
index 9f12059..1216ffa 100644 (file)
@@ -18,7 +18,7 @@ plugindir = $(libdir)/gstreamer-@GST_MAJORMINOR@
 install-pluginLTLIBRARIES:
 
 # the core dumps of some machines have PIDs appended
-CLEANFILES = core.* test-registry.xml *.gcno *.gcda
+CLEANFILES = core core.* test-registry.xml *.gcno *.gcda
 
 SUPPRESSIONS = $(top_srcdir)/common/gst.supp
 
index a058858..beae1be 100644 (file)
@@ -15,6 +15,8 @@ INCLUDES = $(GST_OBJ_CFLAGS)
 # gnome_LDADD = $(GST_OBJ_LIBS) $(LIBGNOMEUI_LIBS)
 # gnome_CFLAGS = $(GST_OBJ_CFLAGS) $(LIBGNOMEUI_CFLAGS)
 
+CLEANFILES = core core.* test-registry.xml *.gcno *.gcda
+
 EXTRA_DIST = extract.pl
 
 EXAMPLES = \
@@ -81,6 +83,16 @@ xml-mp3.c: $(top_srcdir)/docs/manual/highlevel-xml.xml
        $(PERL_PATH) $(srcdir)/extract.pl $@ \
                $(top_srcdir)/docs/manual/highlevel-xml.xml 
 
+CHECK_REGISTRY = $(top_builddir)/tests/examples/manual/test-registry.xml
+
+REGISTRY_ENVIRONMENT = \
+        GST_REGISTRY=$(CHECK_REGISTRY)
+
+TESTS_ENVIRONMENT = \
+        $(REGISTRY_ENVIRONMENT)                                 \
+        GST_PLUGIN_SYSTEM_PATH=                                 \
+        GST_PLUGIN_PATH=$(top_builddir)/plugins
+
 TESTS = bin \
        elementcreate elementfactory elementget elementlink elementmake \
        ghostpad init