After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 740484 - [patch] Use GSound rather than libcanberra for playing sounds
[patch] Use GSound rather than libcanberra for playing sounds
Status: RESOLVED FIXED
Product: gnome-clocks
Classification: Applications
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: Clocks maintainer(s)
Clocks maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-11-21 10:59 UTC by Tristan Brindle
Modified: 2014-11-23 08:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
utils: Use GSound rather than calling libcanberra directly (3.99 KB, patch)
2014-11-21 11:00 UTC, Tristan Brindle
needs-work Details | Review
utils: Use GSound rather than calling libcanberra directly (4.00 KB, patch)
2014-11-22 03:20 UTC, Tristan Brindle
committed Details | Review

Description Tristan Brindle 2014-11-21 10:59:08 UTC
I've written a little GObject wrapper around libcanberra called GSound, which you can read about at [1]. It's mostly intended for use in introspected languages where libcanberra isn't available, but it works well in Vala and has a GObject-y API than canberra.

Attached (assuming I can get git-bz to work) is a simple patch swapping libcanberra for GSound in gnome-clocks. It saves a massive 9 lines of code, but as an added bonus does proper error checking and sends the correct application name to Pulseaudio (visible in the sound tab in gnome-control-center, for example).

[1] https://wiki.gnome.org/Projects/GSound
Comment 1 Tristan Brindle 2014-11-21 11:00:47 UTC
Created attachment 291148 [details] [review]
utils: Use GSound rather than calling libcanberra directly
Comment 2 Paolo Borelli 2014-11-21 11:56:14 UTC
Review of attachment 291148 [details] [review]:

Thanks!

I am ok with this change, some minor nitpicks below

::: src/utils.vala
@@ +305,3 @@
     }
 
+    public async void ring_real (bool repeat)

this method should be private

@@ -303,3 +306,3 @@
 
-    private bool keep_ringing () {
-        Canberra.Proplist pl;
+    public async void ring_real (bool repeat)
+    {

{ on the function line

@@ -304,3 +307,3 @@
-    private bool keep_ringing () {
-        Canberra.Proplist pl;
-        Canberra.Proplist.create (out pl);
+    public async void ring_real (bool repeat)
+    {
+        if (gsound == null)

always use { } also for 1 line conditions
Comment 3 Tristan Brindle 2014-11-22 03:20:02 UTC
Created attachment 291220 [details] [review]
utils: Use GSound rather than calling libcanberra directly

Issues fixed as per comments. Okay to commit?
Comment 4 Paolo Borelli 2014-11-22 09:02:08 UTC
Review of attachment 291220 [details] [review]:

Looks good! Thanks again.


Please make sure to also add the gsound dependency in https://git.gnome.org/browse/jhbuild/tree/modulesets/gnome-apps-3.16.modules#n637
Comment 5 Tristan Brindle 2014-11-23 08:51:05 UTC
Attachment 291220 [details] pushed as 2f52b22 - utils: Use GSound rather than calling libcanberra directly