Merge pull request #39 from thiagokokada/bump-beep

Bump beep, use speaker.{Suspend(),Resume()} to reduce CPU usage

go.mod

@@ -8,7 +8,7 @@ require (
 	fyne.io/systray v1.10.0
 	gioui.org/cmd v0.0.0-20240115171100-84ca391d514b
 	gioui.org/x v0.4.0
-	github.com/gopxl/beep v1.3.0
+	github.com/gopxl/beep v1.3.1-0.20240124160239-28ff2edb5cdf
 	golang.org/x/exp v0.0.0-20240112132812-db7319d0e0e3
 )
 

go.sum

@@ -50,6 +50,8 @@ github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk=
 github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
 github.com/gopxl/beep v1.3.0 h1:wlAdb0Ar3q+pPxEspJM5rNFyNGZJ5/pQd6gYh09byU4=
 github.com/gopxl/beep v1.3.0/go.mod h1:gGVz7MJKlfHrmkzr0wSLGNyY7oisM6rFWJnaLjNxEwA=
+github.com/gopxl/beep v1.3.1-0.20240124160239-28ff2edb5cdf h1:/+hyFy2LSZhHoFjXUQJewvHXga+OfoIc5t3XdpfJOag=
+github.com/gopxl/beep v1.3.1-0.20240124160239-28ff2edb5cdf/go.mod h1:gGVz7MJKlfHrmkzr0wSLGNyY7oisM6rFWJnaLjNxEwA=
 github.com/jfreymuth/oggvorbis v1.0.5 h1:u+Ck+R0eLSRhgq8WTmffYnrVtSztJcYrl588DM4e3kQ=
 github.com/jfreymuth/oggvorbis v1.0.5/go.mod h1:1U4pqWmghcoVsCJJ4fRBKv9peUJMBHixthRlBeD6uII=
 github.com/jfreymuth/vorbis v1.0.2 h1:m1xH6+ZI4thH927pgKD8JOH4eaGRm18rEE9/0WKjvNE=

internal/core/core.go

@@ -180,7 +180,9 @@ func Pause(
 	select {
 	case <-timer.C:
 		log.Println("Resuming twenty-twenty-twenty...")
-		timerCallbackPre()
+		if timerCallbackPre != nil {
+			timerCallbackPre()
+		}
 		// need to start a new instance before calling the blocking
 		// SendWithDuration(), otherwise if the user call Pause() again,
 		// we are going to call Stop() in the previous loop
@@ -195,7 +197,9 @@ func Pause(
 		if err != nil {
 			log.Fatalf("Error while resuming notification: %v. Exiting...\n", err)
 		}
-		timerCallbackPos()
+		if timerCallbackPos != nil {
+			timerCallbackPos()
+		}
 	case <-ctx.Done():
 		log.Println("Cancelling twenty-twenty-twenty pause...")
 	}

internal/core/core_test.go

@@ -148,3 +148,18 @@ func TestPauseCancel(t *testing.T) {
 	assertEqual(t, notifier.notificationCount.Load(), 0)
 	assertEqual(t, notifier.notificationCancelCount.Load(), 0)
 }
+
+func TestPauseNilCallbacks(t *testing.T) {
+	notifier := newMockNotifier()
+	notification.SetNotifier(notifier)
+
+	const timeout = time.Second
+	go func() { time.Sleep(timeout); Stop() }()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	Pause(ctx, &testSettings, Optional{}, nil, nil)
+
+	assertEqual(t, notifier.notificationCount.Load(), 1)
+	assertEqual(t, notifier.notificationCancelCount.Load(), 1)
+}

internal/notification/notification.go

@@ -38,7 +38,7 @@ func Send(
 ) (notify.Notification, error) {
 	initIfNull()
 	if *sound {
-		snd.PlaySendNotification(func() {})
+		snd.PlaySendNotification(nil)
 	}
 	return (*notifier.Load()).CreateNotification(title, text)
 }
@@ -53,7 +53,7 @@ func CancelAfter(
 	select {
 	case <-timer.C:
 		if *sound {
-			snd.PlayCancelNotification(func() {})
+			snd.PlayCancelNotification(nil)
 		}
 	case <-ctx.Done(): // avoid playing notification sound if we cancel the context
 	}

internal/sound/sound.go

@@ -6,6 +6,7 @@ package sound
 import (
 	"embed"
 	"fmt"
+	"log"
 	"time"
 
 	"github.com/gopxl/beep"
@@ -15,61 +16,76 @@ import (
 
 const Enabled bool = true
 
-// Maximum lag, good enough for this use case and will use lower CPU, but need
-// to compesate the lag with time.Sleep() to not feel "strange" (e.g.: "floaty"
-// notifications because the sound comes too late).
-// About as good we can get of CPU usage for now, until this issue is fixed:
-// https://github.com/gopxl/beep/issues/137.
-const lag time.Duration = time.Second
+// Maximum sound notification lag, 1000ms / 10 = 100ms
+const lag time.Duration = time.Second / 10
 
 var (
 	buffer1 *beep.Buffer
 	buffer2 *beep.Buffer
 	//go:embed assets/*.ogg
 	notifications embed.FS
-	initialized   bool
 )
 
-func PlaySendNotification(callback func()) {
+func speakerResume() {
+	err := speaker.Resume()
+	if err != nil {
+		log.Printf("Error while resuming speaker: %v\n", err)
+	}
+}
+
+func speakerSuspend() {
+	err := speaker.Suspend()
+	if err != nil {
+		log.Printf("Error while suspending speaker: %v\n", err)
+	}
+}
+
+func PlaySendNotification(endCallback func()) {
+	speakerResume()
+
 	speaker.Play(beep.Seq(
 		buffer1.Streamer(0, buffer1.Len()),
-		beep.Callback(callback),
+		// https://github.com/gopxl/beep/issues/137#issuecomment-1908845253
+		beep.Callback(func() { time.Sleep(lag) }),
+		beep.Callback(speakerSuspend),
+		beep.Callback(endCallback),
 	))
-	// compesate the lag
-	time.Sleep(lag)
 }
 
 func PlayCancelNotification(callback func()) {
+	speakerResume()
+
 	speaker.Play(beep.Seq(
 		buffer2.Streamer(0, buffer2.Len()),
+		// https://github.com/gopxl/beep/issues/137#issuecomment-1908845253
+		beep.Callback(func() { time.Sleep(lag) }),
+		beep.Callback(speakerSuspend),
 		beep.Callback(callback),
 	))
-	// compesate the lag
-	time.Sleep(lag)
 }
 
-func Init() error {
-	// should be safe to call multiple times
-	if !initialized {
-		var format beep.Format
-		var err error
+func Init() (err error) {
+	var format beep.Format
 
-		buffer1, format, err = loadSound("assets/notification_1.ogg")
-		if err != nil {
-			return fmt.Errorf("notification 1 sound failed: %w", err)
-		}
-
-		// ignoring format since all audio files should have the same format
-		buffer2, _, err = loadSound("assets/notification_2.ogg")
-		if err != nil {
-			return fmt.Errorf("notification 2 sound failed: %w", err)
-		}
-
-		speaker.Init(format.SampleRate, format.SampleRate.N(lag))
-		initialized = true
+	buffer1, format, err = loadSound("assets/notification_1.ogg")
+	if err != nil {
+		return fmt.Errorf("notification 1 sound failed: %w", err)
+	}
+	// ignoring format since all audio files should have the same format
+	buffer2, _, err = loadSound("assets/notification_2.ogg")
+	if err != nil {
+		return fmt.Errorf("notification 2 sound failed: %w", err)
+	}
 
-		return nil
+	err = speaker.Init(format.SampleRate, format.SampleRate.N(lag))
+	if err != nil {
+		return fmt.Errorf("speaker init: %w", err)
+	}
+	err = speaker.Suspend()
+	if err != nil {
+		return fmt.Errorf("speaker suspend: %w", err)
 	}
+
 	return nil
 }
 
@@ -84,5 +100,6 @@ func loadSound(file string) (*beep.Buffer, beep.Format, error) {
 	}
 	buffer := beep.NewBuffer(format)
 	buffer.Append(streamer)
+
 	return buffer, format, nil
 }

main.go

@@ -20,9 +20,7 @@ func main() {
 	optional = core.Optional{Sound: sound.Enabled, Systray: systrayEnabled}
 	settings = core.ParseFlags(os.Args[0], os.Args[1:], version, optional)
 
-	// only init Beep if notification sound is enabled, otherwise we will cause
-	// unnecessary noise in the speakers (and also increased memory usage)
-	if settings.Sound {
+	if optional.Sound {
 		err := sound.Init()
 		if err != nil {
 			log.Fatalf("Error while initialising sound: %v\n", err)

systray.go

@@ -6,7 +6,6 @@ package main
 import (
 	"context"
 	"fmt"
-	"log"
 	"sync"
 
 	"fyne.io/systray"
@@ -85,7 +84,7 @@ func onReady() {
 								mPause.Uncheck()
 							})
 						},
-						func() {},
+						nil,
 					)
 				}()
 
@@ -100,10 +99,6 @@ func onReady() {
 
 				withMutex(&mu, func() { mSound.Uncheck() })
 			} else {
-				err := sound.Init()
-				if err != nil {
-					log.Fatalf("Error while initialising sound: %v\n", err)
-				}
 				settings.Sound = true
 
 				withMutex(&mu, func() { mSound.Check() })

twenty-twenty-twenty.nix

@@ -21,7 +21,7 @@ buildGoModule {
   pname = "twenty-twenty-twenty";
   inherit version;
   src = lib.cleanSource ./.;
-  vendorHash = "sha256-jXzhJio27ryIMqft76EoVtsFvSIC8SO5p9C8bHOxT2s=";
+  vendorHash = "sha256-EIhF/cLdgNMcuEbkihISU/YQ7a7an4SHBQTyagrH8No=";
 
   CGO_ENABLED = if withSound then "1" else "0";