GNOME Bugzilla – Bug 497264
Support multiple thumbnailers per MIME-type
Last modified: 2012-06-20 14:01:15 UTC
Some MIME-types have multiple reasonable ways to generate a thumbnail for any given file. Since the current thumbnail specification method only supports one thumbnailer per MIME-type, not all files can be thumbnailed. For example, a file with the application/ogg MIME-type might be an Ogg/Theora movie to be thumbnailed by Totem, or an Ogg/Vorbis music file with embedded album art. The Totem thumbnailer knows nothing of album art, and the album art thumbnailer knows nothing of video, so the user much choose between the two. I propose that if a directory in /desktop/gnome/thumbnailers has subdirectories, that each subdirectory's command be registered with libgnomeui.
Created attachment 99186 [details] [review] Support multiple thumbnailers per MIME-type Alters gnome-thumbnail.c to use GSLists in scripts_hash, rather than only strings. Each thumbnailer in the list will be attempted sequentially until one of them produces a pixbuf.
Comment on attachment 99186 [details] [review] Support multiple thumbnailers per MIME-type >Index: libgnomeui/gnome-thumbnail.c >=================================================================== >--- libgnomeui/gnome-thumbnail.c (revision 5472) >+++ libgnomeui/gnome-thumbnail.c (working copy) >@@ -83,6 +83,18 @@ > char *uri; > }; > >+/* Used to avoid deleting the list when adding thumbnailers */ >+typedef struct _ThumbnailListWrapper >+{ >+ GSList *list; >+} ThumbnailListWrapper; >+ >+static void >+read_scripts_for_mimetype_dir (const char *directory, const char *mimetype, >+ GHashTable *scripts_hash, GConfClient *client); >+void free_as_gfunc (gpointer data, gpointer user_data); >+void free_script_list (gpointer data); >+ These two should be static too I guess? > GNOME_CLASS_BOILERPLATE (GnomeThumbnailFactory, > gnome_thumbnail_factory, > GObject, G_TYPE_OBJECT) >@@ -184,14 +196,73 @@ > return TRUE; > } > >+/* Called once for each subdirectory of /desktop/gnome/thumbnailers. This >+ * function detects if the subdirectory contains additional subdirectories. >+ * If so, each subdirectory is added to the scripts_hash using a GSList. >+**/ >+static void >+read_scripts_for_mimetype_dir (const char *directory, const char *mimetype, >+ GHashTable *scripts_hash, GConfClient *client) >+{ >+ ThumbnailListWrapper *thumbnailers; >+ char *enable, *commandkey, *command; >+ >+ enable = g_strdup_printf ("%s/enable", directory); >+ if (gconf_client_get_bool (client, >+ enable, >+ NULL)) >+ { >+ commandkey = g_strdup_printf ("%s/command", directory); >+ command = gconf_client_get_string (client, commandkey, NULL); >+ g_free (commandkey); >+ >+ if (command != NULL) >+ { >+ thumbnailers = g_hash_table_lookup (scripts_hash, mimetype); >+ >+ /* If a thumbnail list has not yet been defined for this MIME-type, >+ * allocate one now. >+ **/ >+ if (!thumbnailers) >+ { >+ thumbnailers = g_new (ThumbnailListWrapper, 1); >+ g_hash_table_insert (scripts_hash, g_strdup (mimetype), thumbnailers); >+ } >+ >+ thumbnailers->list = g_slist_append (thumbnailers->list, command); >+ } >+ } >+ >+ g_free (enable); >+} >+ >+/* I'm not sure if g_free can be used as the callback to g_slist_foreach, >+ * so this function is created instead. >+**/ it can, just cast it to GFunc >+void >+free_as_gfunc (gpointer data, gpointer user_data) >+{ >+ g_free (data); >+} >+ >+/* Used to free each script list in the script hash */ >+void >+free_script_list (gpointer data) >+{ >+ ThumbnailListWrapper *wrapper = (ThumbnailListWrapper *) (data); >+ g_slist_foreach (wrapper->list, free_as_gfunc, NULL); >+ g_slist_free (wrapper->list); >+ g_free (wrapper); >+} >+ > /* Must be called on main thread */ > static GHashTable * > read_scripts (void) > { > GHashTable *scripts_hash; > GConfClient *client; >- GSList *subdirs, *l; >- char *subdir, *enable, *escape, *commandkey, *command, *mimetype; >+ GSList *subdirs, *l, *mime_dirs, *m; >+ char *subdir, *escape, *mimetype, *mime_dir; > > client = gconf_client_get_default (); > >@@ -205,7 +276,7 @@ > > scripts_hash = g_hash_table_new_full (g_str_hash, > g_str_equal, >- g_free, g_free); >+ g_free, free_script_list); > > > subdirs = gconf_client_all_dirs (client, "/desktop/gnome/thumbnailers", NULL); >@@ -213,41 +284,41 @@ > for (l = subdirs; l != NULL; l = l->next) > { > subdir = l->data; >- >- enable = g_strdup_printf ("%s/enable", subdir); >- if (gconf_client_get_bool (client, >- enable, >- NULL)) >- { >- commandkey = g_strdup_printf ("%s/command", subdir); >- command = gconf_client_get_string (client, commandkey, NULL); >- g_free (commandkey); >- >- if (command != NULL) { >- mimetype = strrchr (subdir, '/'); >- if (mimetype != NULL) >- { >- mimetype++; /* skip past slash */ >- >- /* Convert '@' to slash in mimetype */ >- escape = strchr (mimetype, '@'); >- if (escape != NULL) >- *escape = '/'; >- >- /* Convert any remaining '@' to '+' in mimetype */ >- while ((escape = strchr (mimetype, '@')) != NULL) >- *escape = '+'; >- >- g_hash_table_insert (scripts_hash, >- g_strdup (mimetype), command); >- } >- else >- { >- g_free (command); >- } >- } >- } >- g_free (enable); >+ >+ mimetype = strrchr (subdir, '/'); >+ if (mimetype != NULL) >+ { >+ mimetype++; /* skip past slash */ >+ >+ /* Convert '@' to slash in mimetype */ >+ escape = strchr (mimetype, '@'); >+ if (escape != NULL) >+ { >+ *escape = '/'; >+ } >+ >+ /* Convert any remaining '@' to '+' in mimetype */ >+ while ((escape = strchr (mimetype, '@')) != NULL) >+ { >+ *escape = '+'; >+ } >+ >+ read_scripts_for_mimetype_dir (subdir, mimetype, scripts_hash, client); >+ >+ /* Check subdirectories for thumbnailer definitions, to support >+ * multiple thumbnailers per MIME-type >+ **/ >+ mime_dirs = gconf_client_all_dirs (client, subdir, NULL); >+ for (m = mime_dirs; m != NULL; m = m->next) >+ { >+ mime_dir = m->data; >+ read_scripts_for_mimetype_dir (mime_dir, mimetype, scripts_hash, >+ client); >+ g_free (mime_dir); >+ } >+ >+ g_slist_free (mime_dirs); >+ } > > g_free (subdir); > } >@@ -880,6 +951,8 @@ > const char *mime_type) > { > GdkPixbuf *pixbuf, *scaled, *tmp_pixbuf; >+ ThumbnailListWrapper *scripts; >+ GSList *s; > char *script, *expanded_script; > int width, height, size; > int original_width = 0; >@@ -900,35 +973,40 @@ > > pixbuf = NULL; > >- script = NULL; >+ scripts = NULL; > if (factory->priv->scripts_hash != NULL) >- script = g_hash_table_lookup (factory->priv->scripts_hash, mime_type); >+ scripts = g_hash_table_lookup (factory->priv->scripts_hash, mime_type); > >- if (script) >+ if (scripts) > { >- int fd; >- GError *error = NULL; >- >- fd = g_file_open_tmp (".gnome_thumbnail.XXXXXX", &tmpname, &error); >- >- if (fd != -1) >- { >- close (fd); >- >- expanded_script = expand_thumbnailing_script (script, size, uri, tmpname); >- if (expanded_script != NULL && >- g_spawn_command_line_sync (expanded_script, >- NULL, NULL, &exit_status, NULL) && >- exit_status == 0) >- { >- pixbuf = gdk_pixbuf_new_from_file (tmpname, NULL); >- g_free (expanded_script); >- } >- >- g_unlink(tmpname); >- } >- g_free (tmpname); >- } >+ for (s = scripts->list; (s != NULL) && (pixbuf == NULL); s = s->next) >+ { >+ int fd; >+ GError *error = NULL; >+ >+ script = s->data; >+ fd = g_file_open_tmp (".gnome_thumbnail.XXXXXX", &tmpname, &error); >+ >+ if (fd != -1) >+ { >+ close (fd); >+ >+ expanded_script = expand_thumbnailing_script (script, size, uri, >+ tmpname); >+ if (expanded_script != NULL && >+ g_spawn_command_line_sync (expanded_script, >+ NULL, NULL, &exit_status, NULL) && >+ exit_status == 0) >+ { >+ pixbuf = gdk_pixbuf_new_from_file (tmpname, NULL); >+ g_free (expanded_script); >+ } >+ >+ g_unlink(tmpname); >+ } >+ g_free (tmpname); >+ } >+ } > > /* Fall back to gdk-pixbuf */ > if (pixbuf == NULL)
Created attachment 102763 [details] [review] Support multiple thumbnailers per MIME-type (v2) Thank you for the advise, updated patch. New patch also extracts escaping of the MIME-type into a separate function.
Created attachment 111127 [details] [review] Support multiple thumbnailers per MIME-type (v3) Updated to apply cleanly to svn trunk, revision 5597
Created attachment 117101 [details] [review] Support multiple thumbnailers per MIME-type (v4) Updated to apply cleanly to svn trunk, revision 5650
(In reply to comment #0) > Some MIME-types have multiple reasonable ways to generate a thumbnail for any > given file. Since the current thumbnail specification method only supports one > thumbnailer per MIME-type, not all files can be thumbnailed. > > For example, a file with the application/ogg MIME-type might be an Ogg/Theora > movie to be thumbnailed by Totem, or an Ogg/Vorbis music file with embedded > album art. The Totem thumbnailer knows nothing of album art, and the album art > thumbnailer knows nothing of video, so the user much choose between the two. Totem's video thumbnailer supports reading cover art from 2.23.4 (although you'll want to test it from trunk, as a couple of bug fixes were applied, also note that only the GStreamer backend supports it).
I hadn't noticed that due to bug #549038, but I'm glad to see it. My specific need for this patch is solved, though somebody else may have similar needs.
Hi John. Does this mean that there's no need to apply this patch to libgnomeui? I was really waiting for someone more familiar with the thumbnailing code to comment on the patch, but that didn't happen. Sorry for letting this hang for so long.
> Does this mean that there's no need to apply this patch to libgnomeui? Right, though if you feel up to it I still think it'd be a nice feature :)
This functionality would be very much useful. For instance, now, totem can't be installed alongside nailer (thumbnailer which uses mplayer) due to overlapping media types they support.
Thumbnailing has been moved from deprecated libgnomeui to gnome-desktop. http://library.gnome.org/devel/gnome-desktop/stable/thumbnail.html
(In reply to comment #0) > The Totem thumbnailer knows nothing of album art, and the album art > thumbnailer knows nothing of video, so the user much choose between the two. Given that this hasn't been true for a while and that the patch hasn't been touched in 4 years, I'll close this.