From 3480e7594a3bab78b83f637e90da9f49cb3cd8be Mon Sep 17 00:00:00 2001 From: NeilClack Date: Wed, 29 Jul 2026 14:58:15 -0500 Subject: [PATCH] Replace wpctl volume control with SDL_mixer VolumeMusic Remove all PipeWire/wpctl system calls (SetDefaultSinkVolume, listSinkIDs, listSinks, SetAllSinkVolumes, parseSinks, parseSinkLine, Sink struct). Volume is now managed entirely through SDL_mixer's VolumeMusic(), making audio fully system-agnostic. Changes: - mixer.go: delete ~170 lines of wpctl code; add SetVolume() public API using mix.VolumeMusic(math.Round(level * MAX_VOLUME)) - Init/Play/SwitchDevice set volume inline (they already hold the mutex) - Play() now uses stored currentVolume instead of hardcoded MAX_VOLUME - AudioOptions: SetAllSinkVolumes -> SetVolume - mixer_test.go: add TestSetVolume (validation + error path coverage) --- internal/mixer/mixer.go | 229 ++++------------------------------ internal/mixer/mixer_test.go | 33 +++++ internal/page/AudioOptions.go | 4 +- 3 files changed, 62 insertions(+), 204 deletions(-) diff --git a/internal/mixer/mixer.go b/internal/mixer/mixer.go index 5d4f086..00b225f 100644 --- a/internal/mixer/mixer.go +++ b/internal/mixer/mixer.go @@ -1,14 +1,9 @@ -// internal/audio/mixer/mixer.go package mixer import ( - "bufio" - "bytes" "errors" "fmt" - "os/exec" - "strconv" - "strings" + "math" "sync" "github.com/veandco/go-sdl2/mix" @@ -25,9 +20,10 @@ var ( currentMusic *mix.Music currentPath string - currentLoops int // 0 = once, -1 = forever + currentLoops int + + currentVolume float64 - // Config knobs (could be made public or read from your config) defaultFreq = 48000 defaultFormat = uint16(sdl.AUDIO_F32) defaultChannels = 2 @@ -37,13 +33,6 @@ var ( refreshCacheCallback func() ) -type Sink struct { - ID int - Name string - Volume float64 - IsDefault bool -} - // Init initializes SDL audio + SDL_mixer. Safe to call multiple times. func Init(cfg *db.AppConfig, onUpdate func()) error { mu.Lock() @@ -58,7 +47,6 @@ func Init(cfg *db.AppConfig, onUpdate func()) error { return fmt.Errorf("SDL audio init failed: %w", err) } - // FIX 1: Initialize MP3 and OGG decoders err := mix.Init(mix.INIT_MP3 | mix.INIT_OGG) if err != nil { logger.Error("Failed to initialize MP3/OGG support: %v", err) @@ -73,7 +61,9 @@ func Init(cfg *db.AppConfig, onUpdate func()) error { return err } - SetAllSinkVolumes(cfg.SavedVolume) + currentVolume = cfg.SavedVolume + v := int(math.Round(currentVolume * float64(mix.MAX_VOLUME))) + mix.VolumeMusic(v) inited = true return nil @@ -83,8 +73,6 @@ func Init(cfg *db.AppConfig, onUpdate func()) error { func openDeviceLocked(name string) error { if err := mix.OpenAudioDevice(defaultFreq, defaultFormat, defaultChannels, defaultChunkSize, name, defaultAllow); err != nil { if name != "" { - // Log the failure of the requested device - // Fallback to default device if err2 := mix.OpenAudioDevice(defaultFreq, defaultFormat, defaultChannels, defaultChunkSize, "", defaultAllow); err2 != nil { return fmt.Errorf("failed to open device %q and default device: %w", name, err2) @@ -106,16 +94,13 @@ func openDeviceLocked(name string) error { // Play loads a WAV (as music) and plays it. If loopForever is true, loops forever. // If something is already playing, it stops and replaces it. func Play(path string, loopForever bool, cfg *db.AppConfig) error { - // Check if audio is enabled or disabled if !cfg.AudioEnabled { return nil } - // Lock the mutex mu.Lock() defer mu.Unlock() - // Check if mixer has been initialized and that a device is open if !inited { logger.Error("audio mixer not initialized") return errors.New("audio mixer not initialized") @@ -130,19 +115,16 @@ func Play(path string, loopForever bool, cfg *db.AppConfig) error { return errors.New("empty audio path") } - // Stop and free any prior music mix.HaltMusic() if currentMusic != nil { currentMusic.Free() currentMusic = nil } - // Load the music file m, err := mix.LoadMUS(path) if err != nil { return fmt.Errorf("LoadMUS(%q): %w", path, err) } - // Set current music to the loaded music file currentMusic = m currentPath = path if loopForever { @@ -151,9 +133,9 @@ func Play(path string, loopForever bool, cfg *db.AppConfig) error { currentLoops = 0 } - mix.VolumeMusic(mix.MAX_VOLUME) - volume := mix.VolumeMusic(-1) - logger.Info("Current volume is %d", volume) + v := int(math.Round(currentVolume * float64(mix.MAX_VOLUME))) + mix.VolumeMusic(v) + logger.Info("Current volume is %d", v) if err := currentMusic.Play(currentLoops); err != nil { currentMusic.Free() @@ -174,7 +156,7 @@ func Stop(fadeMs int, cfg *db.AppConfig) { } if fadeMs > 0 && mix.PlayingMusic() { - _ = mix.FadeOutMusic(fadeMs) // async fade; we’ll still free below + _ = mix.FadeOutMusic(fadeMs) } mix.HaltMusic() @@ -196,59 +178,24 @@ func IsPlaying() bool { return mix.PlayingMusic() } -// SetDefaultSinkVolume sets volume 0.0–1.0 on the default sink -func SetDefaultSinkVolume(level float64) error { +// SetVolume sets the SDL_mixer music volume to level (0.0–1.0). +func SetVolume(level float64) error { if level < 0 || level > 1 { return fmt.Errorf("volume must be between 0.0 and 1.0, got %f", level) } - pct := fmt.Sprintf("%.2f", level) - cmd := exec.Command("wpctl", "set-volume", "@DEFAULT_AUDIO_SINK@", pct) - if out, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("wpctl failed: %w\noutput: %s", err, out) - } - return nil -} - -// listSinkIDs returns just the numeric IDs of all audio sinks. -func listSinkIDs() ([]int, error) { - sinks, err := listSinks() - if err != nil { - return nil, err - } - ids := make([]int, len(sinks)) - for i, s := range sinks { - ids[i] = s.ID - } - return ids, nil -} - -// listSinks returns full Sink info for all audio sinks. -func listSinks() ([]Sink, error) { - out, err := exec.Command("wpctl", "status").Output() - if err != nil { - return nil, fmt.Errorf("wpctl status: %w", err) - } - return parseSinks(out) -} - -// SetAllSinkVolumes sets volume on every audio sink found via wpctl status -func SetAllSinkVolumes(level float64) error { - ids, err := listSinkIDs() - if err != nil { - return err - } - pct := fmt.Sprintf("%.2f", level) - for _, id := range ids { - cmd := exec.Command("wpctl", "set-volume", strconv.Itoa(id), pct) - if out, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("wpctl set-volume %d failed: %w\noutput: %s", id, err, out) - } + mu.Lock() + defer mu.Unlock() + if !inited { + return errors.New("audio mixer not initialized") } + currentVolume = level + v := int(math.Round(level * float64(mix.MAX_VOLUME))) + mix.VolumeMusic(v) return nil } // ConvertVolume converts a desired volume percentage (0%, 25%, 33%, 90%, etc) to -// the float64 value needed to pass to SetAllSinkVolumes 0..1.0 +// the float64 value needed to pass to SetVolume 0..1.0 // Percentages should be passed as integers. 25% -> 25 func ConvertVolume(volPerc int) float64 { if volPerc == 0 { @@ -257,116 +204,6 @@ func ConvertVolume(volPerc int) float64 { return float64(volPerc) / 100.0 } -// parseSinks parses the output of `wpctl status` and extracts audio sinks. -func parseSinks(data []byte) ([]Sink, error) { - var sinks []Sink - - scanner := bufio.NewScanner(bytes.NewReader(data)) - - inAudioSection := false - inSinksSection := false - - for scanner.Scan() { - line := scanner.Text() - stripped := strings.TrimLeft(line, " │├└─") - - // Detect top-level sections: "Audio", "Video", "Settings", etc. - // These are lines with no leading tree characters. - if !strings.HasPrefix(line, " ") && !strings.HasPrefix(line, "│") { - inAudioSection = strings.TrimSpace(line) == "Audio" - inSinksSection = false - continue - } - - if !inAudioSection { - continue - } - - // Detect subsection headers within Audio - if strings.Contains(stripped, "Sinks:") { - inSinksSection = true - continue - } - // Any other subsection header resets sink parsing - if strings.HasSuffix(strings.TrimSpace(stripped), ":") { - inSinksSection = false - continue - } - - if !inSinksSection { - continue - } - - sink, ok := parseSinkLine(stripped) - if ok { - sinks = append(sinks, sink) - } - } - - if err := scanner.Err(); err != nil { - return nil, fmt.Errorf("scanning wpctl output: %w", err) - } - - return sinks, nil -} - -// parseSinkLine parses a single sink entry line like: -// -// "*51. USB Audio [vol: 1.00]" -// "33. Built-in Audio Analog Stereo [vol: 0.65]" -func parseSinkLine(s string) (Sink, bool) { - s = strings.TrimSpace(s) - if s == "" { - return Sink{}, false - } - - var sink Sink - - // Leading '*' marks the default sink - if strings.HasPrefix(s, "*") { - sink.IsDefault = true - s = strings.TrimPrefix(s, "*") - s = strings.TrimSpace(s) - } - - // Expect ". ..." - dotIdx := strings.Index(s, ".") - if dotIdx < 0 { - return Sink{}, false - } - - idStr := strings.TrimSpace(s[:dotIdx]) - id, err := strconv.Atoi(idStr) - if err != nil { - return Sink{}, false - } - sink.ID = id - - rest := strings.TrimSpace(s[dotIdx+1:]) - - // Extract [vol: X.XX] if present - volStart := strings.Index(rest, "[vol:") - if volStart >= 0 { - sink.Name = strings.TrimSpace(rest[:volStart]) - volPart := rest[volStart:] - // "[vol: 0.65]" or "[vol: 0.65 MUTED]" - volPart = strings.TrimPrefix(volPart, "[vol:") - volPart = strings.TrimRight(volPart, "]") - volPart = strings.TrimSpace(volPart) - fields := strings.Fields(volPart) - if len(fields) > 0 { - v, err := strconv.ParseFloat(fields[0], 64) - if err == nil { - sink.Volume = v - } - } - } else { - sink.Name = strings.TrimSpace(rest) - } - - return sink, true -} - // ListDevices returns the list of available output device names (from core SDL). func ListDevices() []string { mu.Lock() @@ -398,20 +235,15 @@ func SwitchDevice(newName string, cfg *db.AppConfig) error { return errors.New("audio mixer not initialized") } - // Log device switch attempt - // Automatically defaults to "system default" - oldDevice := CurrentDevice() // Which device is to be closed - // SDL expects "" for default device, not "system default" - if newName == "" { // Which device is being switched to? + oldDevice := CurrentDevice() + if newName == "" { newName = "" } - // Capture current state wasPlaying := mix.PlayingMusic() savedPath := currentPath savedLoops := currentLoops - // Stop and release current music before reopening device mix.HaltMusic() if currentMusic != nil { currentMusic.Free() @@ -420,34 +252,27 @@ func SwitchDevice(newName string, cfg *db.AppConfig) error { currentPath = "" currentLoops = 0 - // Close and reopen device if deviceOpen { mix.CloseAudio() deviceOpen = false } - // Attempt to open the newName device if err := openDeviceLocked(newName); err != nil { - // If error switching to new device, switch back to the old device - // oldDevice is already "" for default (which openDeviceLocked expects) if restoreErr := openDeviceLocked(oldDevice); restoreErr != nil { - // We should not hit an error when switching back, but if we do... logger.Error("Failed to switch to device: %s", oldDevice) } - // Could not switch to the new device, return the error return err } - // Restore saved volume after reopening device if cfg != nil { - SetAllSinkVolumes(cfg.SavedVolume) + currentVolume = cfg.SavedVolume + v := int(math.Round(cfg.SavedVolume * float64(mix.MAX_VOLUME))) + mix.VolumeMusic(v) } - // Optionally resume previous track if there was one if wasPlaying && savedPath != "" { m, err := mix.LoadMUS(savedPath) if err != nil { - // Don't fail the device switch just because reload failed return fmt.Errorf("device switched, but reloading %q failed: %w", savedPath, err) } currentMusic = m @@ -489,7 +314,7 @@ func Shutdown() { } if inited { - mix.Quit() // Add this to clean up the decoders + mix.Quit() sdl.QuitSubSystem(sdl.INIT_AUDIO) inited = false } diff --git a/internal/mixer/mixer_test.go b/internal/mixer/mixer_test.go index 796f032..d7babf7 100644 --- a/internal/mixer/mixer_test.go +++ b/internal/mixer/mixer_test.go @@ -7,6 +7,39 @@ import ( "rhemawavetech.com/rhemawave/internal/mixer" ) +func TestSetVolume(t *testing.T) { + tests := []struct { + name string + level float64 + wantErr bool + }{ + {"zero volume", 0.0, false}, + {"half volume", 0.5, false}, + {"max volume", 1.0, false}, + {"above range", 1.5, true}, + {"negative", -0.1, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := mixer.SetVolume(tt.level) + if tt.wantErr { + if err == nil { + t.Errorf("SetVolume(%v) = nil, want error", tt.level) + } + return + } + if err != nil { + // For valid input, failure is expected when SDL isn't initialized + // (that's fine — we're testing validation, not SDL integration) + if err.Error() != "audio mixer not initialized" { + t.Errorf("SetVolume(%v) = %v, want 'audio mixer not initialized' or nil", tt.level, err) + } + } + }) + } +} + func TestConvertVolume(t *testing.T) { tests := []struct { name string diff --git a/internal/page/AudioOptions.go b/internal/page/AudioOptions.go index aa48357..edd1721 100644 --- a/internal/page/AudioOptions.go +++ b/internal/page/AudioOptions.go @@ -333,8 +333,8 @@ func (p *AudioOptions) UpdateVolume(level int) { } config.SavedVolume = newVol p.Context.DbPtr.Save(&config) - if err := mixer.SetAllSinkVolumes(newVol); err != nil { - logger.Error("Error setting allSinkVolumes: %v", err) + if err := mixer.SetVolume(newVol); err != nil { + logger.Error("Error setting volume: %v", err) } if err := p.CreateVolumePercentItem(); err != nil { logger.Error("failed to refresh volume percent item: %v", err)