plugin: Unify static and dynamic plugin interface

This patch changes the entry point of each plugin in order to unify the
interface for static and dynamic plugin. What we do is replace the
current static plugin interface and extend the dymamic one. The plugin
entry was a C structure, name "gst_plugin_desc". With this patch, the
interface is now:

  GstPpluginDesc *gst_plugin_<name>_get_desc(void);

The reason we change the C structure into function, is that it is
potentially more common to have function pointers, avoiding possible
binding language limitation. Additionally to that. This change prevents
the symbols from clashing between plugins, allowing to build once the
plugin (assuming you have -fPIC).

On the plugin loader side, we symply derive the shared object basename
to extract the plugin name. If this symbol is not found, we fallback to
gst_plugin_desc for backward compatibility.

This has one side effect, which is that the shared objects now need to
be named after their plugin name. This is generally the case with few
exceptions. The benifit of this limitation is that you can control the
gst_plugin_<name>_desc clash at file level.

https://bugzilla.gnome.org/show_bug.cgi?id=779344

diff --git a/gst/gst_private.h b/gst/gst_private.h
index dc2c153..5cf993a 100644 (file)
--- a/gst/gst_private.h
+++ b/gst/gst_private.h
@@ -97,7 +97,7 @@ G_GNUC_INTERNAL  gboolean priv_gst_plugin_loading_have_whitelist (void);
 
 G_GNUC_INTERNAL  guint32  priv_gst_plugin_loading_get_whitelist_hash (void);
 
-G_GNUC_INTERNAL  gboolean priv_gst_plugin_desc_is_whitelisted (GstPluginDesc * desc,
+G_GNUC_INTERNAL  gboolean priv_gst_plugin_desc_is_whitelisted (const GstPluginDesc * desc,
                                                                const gchar   * filename);
 
 G_GNUC_INTERNAL  gboolean _priv_plugin_deps_env_vars_changed (GstPlugin * plugin);
@@ -353,8 +353,6 @@ struct _GstPlugin {
   /*< private >*/
   GstPluginDesc        desc;
 
-  GstPluginDesc *orig_desc;
-
   gchar *      filename;
   gchar *      basename;       /* base name (non-dir part) of plugin path */
 

diff --git a/gst/gstplugin.c b/gst/gstplugin.c
index 506478d..7331a31 100644 (file)
--- a/gst/gstplugin.c
+++ b/gst/gstplugin.c
@@ -344,7 +344,7 @@ _priv_gst_plugin_initialize (void)
  * name and the plugin's source package name, to keep the format simple.
  */
 static gboolean
-gst_plugin_desc_matches_whitelist_entry (GstPluginDesc * desc,
+gst_plugin_desc_matches_whitelist_entry (const GstPluginDesc * desc,
     const gchar * filename, const gchar * pattern)
 {
   const gchar *sep;
@@ -406,7 +406,7 @@ done:
 }
 
 gboolean
-priv_gst_plugin_desc_is_whitelisted (GstPluginDesc * desc,
+priv_gst_plugin_desc_is_whitelisted (const GstPluginDesc * desc,
     const gchar * filename)
 {
   gchar **entry;
@@ -681,12 +681,42 @@ gst_plugin_load_file (const gchar * filename, GError ** error)
   return _priv_gst_plugin_load_file_for_registry (filename, NULL, error);
 }
 
+static gchar *
+extract_symname (const char *filename)
+{
+  gchar *bname, *name, *symname;
+  const gchar *dot;
+  gsize prefix_len = 0, len;
+
+  bname = g_path_get_basename (filename);
+
+  if (g_str_has_prefix (bname, "libgst"))
+    prefix_len += 6;
+  else if (g_str_has_prefix (bname, "lib"))
+    prefix_len += 3;
+
+  dot = g_utf8_strrchr (bname, -1, '.');
+  if (dot)
+    len = dot - bname - prefix_len;
+  else
+    len = g_utf8_strlen (bname + prefix_len, -1);
+
+  name = g_strndup (bname + prefix_len, len);
+  g_free (bname);
+
+  symname = g_strconcat ("gst_plugin_", name, "_get_desc", NULL);
+  g_free (name);
+
+  return symname;
+}
+
 GstPlugin *
 _priv_gst_plugin_load_file_for_registry (const gchar * filename,
     GstRegistry * registry, GError ** error)
 {
-  GstPluginDesc *desc;
+  const GstPluginDesc *desc;
   GstPlugin *plugin;
+  gchar *symname;
   GModule *module;
   gboolean ret;
   gpointer ptr;
@@ -757,7 +787,20 @@ _priv_gst_plugin_load_file_for_registry (const gchar * filename,
     goto return_error;
   }
 
-  ret = g_module_symbol (module, "gst_plugin_desc", &ptr);
+  symname = extract_symname (filename);
+  ret = g_module_symbol (module, symname, &ptr);
+
+  if (ret) {
+    GstPluginDesc *(*get_desc) (void) = ptr;
+    ptr = get_desc ();
+  } else {
+    GST_DEBUG ("Could not find symbol '%s', falling back to gst_plugin_desc",
+        symname);
+    ret = g_module_symbol (module, "gst_plugin_desc", &ptr);
+  }
+
+  g_free (symname);
+
   if (!ret) {
     GST_DEBUG ("Could not find plugin entry point in \"%s\"", filename);
     g_set_error (error,
@@ -768,7 +811,7 @@ _priv_gst_plugin_load_file_for_registry (const gchar * filename,
     goto return_error;
   }
 
-  desc = (GstPluginDesc *) ptr;
+  desc = (const GstPluginDesc *) ptr;
 
   if (priv_gst_plugin_loading_have_whitelist () &&
       !priv_gst_plugin_desc_is_whitelisted (desc, filename)) {
@@ -789,28 +832,30 @@ _priv_gst_plugin_load_file_for_registry (const gchar * filename,
   }
 
   plugin->module = module;
-  plugin->orig_desc = desc;
 
   if (new_plugin) {
     /* check plugin description: complain about bad values and fail */
-    CHECK_PLUGIN_DESC_FIELD (plugin->orig_desc, name, filename);
-    CHECK_PLUGIN_DESC_FIELD (plugin->orig_desc, description, filename);
-    CHECK_PLUGIN_DESC_FIELD (plugin->orig_desc, version, filename);
-    CHECK_PLUGIN_DESC_FIELD (plugin->orig_desc, license, filename);
-    CHECK_PLUGIN_DESC_FIELD (plugin->orig_desc, source, filename);
-    CHECK_PLUGIN_DESC_FIELD (plugin->orig_desc, package, filename);
-    CHECK_PLUGIN_DESC_FIELD (plugin->orig_desc, origin, filename);
-
-    if (plugin->orig_desc->name != NULL && plugin->orig_desc->name[0] == '"') {
+    CHECK_PLUGIN_DESC_FIELD (desc, name, filename);
+    CHECK_PLUGIN_DESC_FIELD (desc, description, filename);
+    CHECK_PLUGIN_DESC_FIELD (desc, version, filename);
+    CHECK_PLUGIN_DESC_FIELD (desc, license, filename);
+    CHECK_PLUGIN_DESC_FIELD (desc, source, filename);
+    CHECK_PLUGIN_DESC_FIELD (desc, package, filename);
+    CHECK_PLUGIN_DESC_FIELD (desc, origin, filename);
+
+    if (desc->name != NULL && desc->name[0] == '"') {
       g_warning ("Invalid plugin name '%s' - fix your GST_PLUGIN_DEFINE "
-          "(remove quotes around plugin name)", plugin->orig_desc->name);
+          "(remove quotes around plugin name)", desc->name);
     }
 
-    if (plugin->orig_desc->release_datetime != NULL &&
-        !check_release_datetime (plugin->orig_desc->release_datetime)) {
-      GST_ERROR ("GstPluginDesc for '%s' has invalid datetime '%s'",
-          filename, plugin->orig_desc->release_datetime);
-      plugin->orig_desc->release_datetime = NULL;
+    if (desc->release_datetime != NULL &&
+        !check_release_datetime (desc->release_datetime)) {
+      g_warning ("GstPluginDesc for '%s' has invalid datetime '%s'",
+          filename, desc->release_datetime);
+      g_set_error (error, GST_PLUGIN_ERROR, GST_PLUGIN_ERROR_MODULE,
+          "Plugin %s has invalid plugin description field 'release_datetime'",
+          filename);
+      goto return_error;
     }
   }
 
@@ -824,7 +869,7 @@ _priv_gst_plugin_load_file_for_registry (const gchar * filename,
   GST_LOG ("Plugin %p for file \"%s\" prepared, registering...",
       plugin, filename);
 
-  if (!gst_plugin_register_func (plugin, plugin->orig_desc, NULL)) {
+  if (!gst_plugin_register_func (plugin, desc, NULL)) {
     /* remove signal handler */
     _gst_plugin_fault_handler_restore ();
     GST_DEBUG ("gst_plugin_register_func failed for plugin \"%s\"", filename);

diff --git a/gst/gstplugin.h b/gst/gstplugin.h
index 67e8e59..b6eefd9 100644 (file)
--- a/gst/gstplugin.h
+++ b/gst/gstplugin.h
@@ -246,38 +246,40 @@ struct _GstPluginDesc {
  * If defined, the GST_PACKAGE_RELEASE_DATETIME will also be used for the
  * #GstPluginDesc,release_datetime field.
  */
-#ifdef GST_PLUGIN_BUILD_STATIC
-#define GST_PLUGIN_DEFINE(major,minor,name,description,init,version,license,package,origin)    \
-G_BEGIN_DECLS                                          \
-GST_PLUGIN_EXPORT void G_PASTE(gst_plugin_, G_PASTE(name, _register)) (void);                  \
-                                                       \
-void                                                   \
-G_PASTE(gst_plugin_, G_PASTE(name, _register)) (void)  \
-{                                                      \
-  gst_plugin_register_static (major, minor, G_STRINGIFY(name), \
-      description, init, version, license,             \
-      PACKAGE, package, origin);                       \
-}                                                      \
-G_END_DECLS
-#else /* !GST_PLUGIN_BUILD_STATIC */
-#define GST_PLUGIN_DEFINE(major,minor,name,description,init,version,license,package,origin)    \
+#define GST_PLUGIN_DEFINE(major,minor,name,description,init,version,license,package,origin) \
 G_BEGIN_DECLS \
-GST_PLUGIN_EXPORT GstPluginDesc gst_plugin_desc = {    \
-  major,                                               \
-  minor,                                               \
-  G_STRINGIFY(name),                                    \
-  (gchar *) description,                               \
-  init,                                                        \
-  version,                                             \
-  license,                                             \
-  PACKAGE,                                             \
-  package,                                             \
-  origin,                                              \
-  __GST_PACKAGE_RELEASE_DATETIME,                       \
-  GST_PADDING_INIT                                     \
-}; \
+GST_PLUGIN_EXPORT const GstPluginDesc * G_PASTE(gst_plugin_, G_PASTE(name, _get_desc)) (void); \
+GST_PLUGIN_EXPORT void G_PASTE(gst_plugin_, G_PASTE(name, _register)) (void); \
+\
+static const GstPluginDesc gst_plugin_desc = { \
+  major, \
+  minor, \
+  G_STRINGIFY(name), \
+  (gchar *) description, \
+  init, \
+  version, \
+  license, \
+  PACKAGE, \
+  package, \
+  origin, \
+  __GST_PACKAGE_RELEASE_DATETIME, \
+  GST_PADDING_INIT \
+};                                       \
+\
+const GstPluginDesc * \
+G_PASTE(gst_plugin_, G_PASTE(name, _get_desc)) (void) \
+{ \
+    return &gst_plugin_desc; \
+} \
+\
+void \
+G_PASTE(gst_plugin_, G_PASTE(name, _register)) (void) \
+{ \
+  gst_plugin_register_static (major, minor, G_STRINGIFY(name), \
+      description, init, version, license, \
+      PACKAGE, package, origin); \
+} \
 G_END_DECLS
-#endif /* GST_PLUGIN_BUILD_STATIC */
 
 /**
  * GST_LICENSE_UNKNOWN: