GNOME Bugzilla – Bug 571488
Migrate from deprecated gnome_sound to libcanberra
Last modified: 2013-09-13 01:03:30 UTC
See http://www.freedesktop.org/wiki/Specifications/sound-theme-spec ; and maybe http://bugzilla.gnome.org/buglist.cgi?short_desc_type=allwordssubstr&short_desc=canberra&status_whiteboard_type=allwordssubstr&keywords_type=allwords&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&bug_status=NEEDINFO&bug_status=RESOLVED&bug_status=VERIFIED&bug_status=CLOSED for some sample implementations. ./evolution/plugins/mail-notification/mail-notification.c: gnome_sound_play (file); ./evolution/plugins/mail-notification/mail-notification.c: gnome_sound_init ("localhost"); ./evolution/plugins/mail-notification/mail-notification.c: gnome_sound_shutdown (); ./evolution/shell/main.c: gnome_sound_init ("localhost"); ./evolution/shell/main.c: gnome_sound_shutdown (); ./evolution/calendar/gui/alarm-notify/notify-main.c: gnome_sound_init ("localhost"); ./evolution/calendar/gui/alarm-notify/notify-main.c: gnome_sound_shutdown (); ./evolution/calendar/gui/alarm-notify/alarm-queue.c: gnome_sound_play (url); /* this sucks */ ./evolution/mail/mail-session.c: gnome_sound_play (filename);
See also bug #513762 and bug #561843. Moving to libcanberra may fix them.
We should be generating sounds for the following events from the Sound Naming Specification. This does not include events that GTK+ would generate a sound for automatically. message-new-email The sound used when a new email is received. complete-download The sound used when a file completed downloading. (Perhaps when attaching remote files or when syncing is complete.) message-sent-email The sound used when a new email is sent. trash-empty The sound used when the user empties the trash. (Perhaps for expunging.) item-deleted The sound used when a item is deleted. (Not sure when to use this or file-trash.) file-trash The sound used when a file or folder is sent to the trash. (Only if it's not too annoying.) link-pressed The sound used when a link in a web or help browser is pressed. (Have GtkHtml handle this?) link-released The sound used when a link in a web or help browser is released. (Have GtkHtml handle this?)
(In reply to comment #2) > We should be generating sounds for the following events from the Sound Naming > Specification. Means I should sit in front of my piano and record some weird stuff?
Yes, we need our own "YOU'VE GOT MAIL!"
..and for the sake, the grep for headers: ./shell/main.c:#include <libgnome/gnome-sound.h> ./calendar/gui/alarm-notify/notify-main.c:#include <libgnome/gnome-sound.h> ./calendar/gui/alarm-notify/alarm-queue.c:#include <libgnome/gnome-sound.h> ./mail/mail-folder-cache.c:#include <libgnome/gnome-sound.h> ./mail/mail-session.c:#include <libgnome/gnome-sound.h>
Created attachment 134206 [details] [review] Canberra for mailnotification Hi... I have added canberra support to the mail notification plugin. The question is just if everythings fine and what to do with the rest - I have made it very simple and basic as an example. Are there concrete plans or should it be a simple replacement? What is missing is the autotools parts ( libcanberra-gtk must be included ). If it is okay like this ( if it should be just replaced ) leave a comment, a second patch is ready :-)
Comment on attachment 134206 [details] [review] Canberra for mailnotification >diff --git a/plugins/mail-notification/mail-notification.c b/plugins/mail-notification/mail-notification.c >index 5bb65ce..de1ca92 100644 >--- a/plugins/mail-notification/mail-notification.c >+++ b/plugins/mail-notification/mail-notification.c >@@ -29,7 +29,7 @@ > #include <glib/gi18n.h> > #include <gtk/gtk.h> > #include <gconf/gconf-client.h> >-#include <libgnome/gnome-sound.h> >+#include <canberra-gtk.h> > > #ifdef HAVE_DBUS > #include <dbus/dbus-glib.h> >@@ -54,6 +54,7 @@ > #define GCONF_KEY_ENABLED_STATUS GCONF_KEY_ROOT "status-enabled" > #define GCONF_KEY_ENABLED_SOUND GCONF_KEY_ROOT "sound-enabled" > >+static ca_context *mailnotification = NULL; > static gboolean enabled = FALSE; > static GtkWidget *get_cfg_widget (void); > static GStaticMutex mlock = G_STATIC_MUTEX_INIT; >@@ -605,7 +606,9 @@ do_play_sound (gboolean beep, const gchar *file) > else if (!file || !*file) > g_warning ("No file to play!"); > else >- gnome_sound_play (file); >+ ca_context_play(mailnotification, 0, >+ CA_PROP_MEDIA_FILENAME, file, >+ NULL); > } > > struct _SoundConfigureWidgets >@@ -733,9 +736,14 @@ static void > enable_sound (int enable) > { > if (enable) >- gnome_sound_init ("localhost"); >+ { >+ ca_context_create(&mailnotification); >+ ca_context_change_props(mailnotification, >+ CA_PROP_APPLICATION_NAME, "mailnotification Plugin", >+ NULL); >+ } > else >- gnome_sound_shutdown (); >+ ca_context_destroy(&mailnotification); > } > > static GtkWidget *
Created attachment 134210 [details] [review] Second patch / simple replace Sorry for polluting this so much, I'm not used to bugzilla and was a bit too fast, the unedited patch from before contains a bad hack in order to have the libcanberra includes ... I don't like autotools and know what I've done is very bad stuff, it shouldn't have been uploaded. Here is the second simple replacement of the gnome-sound stuff. Should work with ogg files (!) and hope everything is fine this time.
(In reply to comment #8) > Sorry for polluting this so much I'm way more happy about the patches than grumbling about bugzilla pollution. :-P
Created attachment 135252 [details] [review] canberra - move , 2 try This new patch obsoletes the other comment and patches of me.. It should fix bug 513762 and bug 561843 but it isn't a beautiful patch I think in every way. In order to fix bug 513762 I needed to add some hackish things to mail_notification.c ... First there is a new button for setting the sound to the system default ( if the play button is clicked, the default sound will be played ), then there is the new string syssound (bool) to distinguish between the normal file playing and the event playing (unsure if it's the right way to do this, but works for me ). And other stuff for example that the filechooser shouldn't be activated when syssound is activated etc... In order to let it compile by default and as canberra is needed not only in mail-notification, this patch also makes canberra a necessary requirement ( without version, a simple check ). configure.ac & Makefiles.am were edited where necessary - hope it was okay and I've done it right. The other changes were like in the previous patches, simple replace for gnome_sound_play. The good thing is it works for me, but I don't think it will work on every computer... The main thing that's necessary for canberra to work, is a sound theme - I have installed the freedesktop-sound-theme. There are settings for canberra where you can set stuff like this, for example CA_PROP_CANBERRA_XDG_THEME_NAME... Gnome has only a few sounds and no default sound theme afaik. So this might be a serious problem as there will nothing happen if the default system sound should be played but doesn't exist. Then there is another thing - Matthew Barnes wanted to have more events and sounds - but how should this be done? Should we do it in a plugin like mail-notification ? Or should it be done directly in evolution ? And I have a little problem with the calendar : In alarm_queue.c I have added the necessary changes to replace gnome_sound_play, but it doesn't work... It always beeps - not sure If I made a bug or overseeing something, but I don't have a clue in this case.
Matthew, can you answer the last comment and review this?
(In reply to comment #10) > It should fix bug 513762 and bug 561843 but it isn't a beautiful patch I think > in every way. > > In order to fix bug 513762 I needed to add some hackish things to > mail_notification.c ... I don't know, I thought your patch was pretty straight-forward. Couple nitpicky comments about the mail-notification part: - I'd prefer "Use sound theme" over "Play default system sound". - Similarly, I'd prefer the new GConf key be named "sound-use-theme". I noticed the existing code in mail-notification.c could be greatly simplified by using GConfBridge and our own EBinding (in e-util) utilities. These utilities let you binding object properties (such as "active" and "sensitive") to GConf keys or other object properties, respectively. You might consider doing some code cleanup with these utilities if you're feeling ambitious, but it's not at all required for patch acceptance. > In order to let it compile by default and as canberra is needed not only in > mail-notification, this patch also makes canberra a necessary requirement ( > without version, a simple check ). I think that's fine. You might consider checking for libcanberra earlier in the configure process though, so we fail as early as possible. (That's why I put all the version checks up at the top.) > configure.ac & Makefiles.am were edited where necessary - hope it was okay and > I've done it right. Yeah, looks good. > So this might be a serious problem as there will nothing happen if the default > system sound should be played but doesn't exist. Is there API somewhere that lets you check for the existence of sound themes and/or the particular sound name you want to play? Then you could disable the theme option if none are available. I didn't see anything in libcanberra for this. > Then there is another thing - Matthew Barnes wanted to have more events and > sounds - but how should this be done? Should we do it in a plugin like > mail-notification ? Or should it be done directly in evolution ? It should be done directly in Evolution, but don't worry about it. It's a separate enhancement. The important thing here is to stop relying on gnome-sound. > And I have a little problem with the calendar : In alarm_queue.c I have added > the necessary changes to replace gnome_sound_play, but it doesn't work... > It always beeps - not sure If I made a bug or overseeing something, but I don't > have a clue in this case. Might it be because you're passing a URL to a filename property? ca_context_play(ca_gtk_context_get(), 0, CA_PROP_MEDIA_FILENAME, url, NULL); Consider using g_filename_from_uri(). Marking the patch as "needs-work" for this and the two mail-notification nitpicks.
Created attachment 139784 [details] [review] Adapted to git master Updated patch applies cleanly to today's git master. No other changes.
Matthew: So can we get this into the kill-bonobo branch and mark it as [KB-Fixed]? Which negative side effects will happen? Do we still miss sound files? If so, for what specific events?
The sound events can wait. That's a separate enhancement. My comments above should be easy to address. I'll take a look at this over the weekend if liam doesn't beat me to it. We're very close to dropping libgnome; I'm eager to get it over with.
Committed patch with suggested fixes in: http://git.gnome.org/cgit/evolution/commit/?h=kill-bonobo&id=ff7084ffe4a935a4cfafc9cd44a9b9daf43cb976
*** Bug 599696 has been marked as a duplicate of this bug. ***
*** Bug 576917 has been marked as a duplicate of this bug. ***