some simplification of PlaybackManager

backend/playbackmanager.go

@@ -2,6 +2,7 @@ package backend
 
 import (
 	"context"
+	"errors"
 	"log"
 	"time"
 
@@ -24,7 +25,6 @@ var (
 type PlaybackManager struct {
 	ctx           context.Context
 	cancelPollPos context.CancelFunc
-	pollingTick   *time.Ticker
 	sm            *ServerManager
 	player        player.BasePlayer
 
@@ -114,15 +114,6 @@ func NewPlaybackManager(
 	return pm
 }
 
-func (pm *PlaybackManager) invokeNoArgCallbacks(cbs []func()) {
-	if pm.callbacksDisabled {
-		return
-	}
-	for _, cb := range cbs {
-		cb()
-	}
-}
-
 func (p *PlaybackManager) CurrentPlayer() player.BasePlayer {
 	return p.player
 }
@@ -250,9 +241,6 @@ func (p *PlaybackManager) PlayAlbum(albumID string, firstTrack int, shuffle bool
 	if p.replayGainCfg.Mode == ReplayGainAuto {
 		p.SetReplayGainMode(player.ReplayGainAlbum)
 	}
-	if firstTrack <= 0 {
-		return p.player.PlayTrackAt(0)
-	}
 	return p.player.PlayTrackAt(firstTrack)
 }
 
@@ -263,9 +251,6 @@ func (p *PlaybackManager) PlayPlaylist(playlistID string, firstTrack int, shuffl
 	if p.replayGainCfg.Mode == ReplayGainAuto {
 		p.SetReplayGainMode(player.ReplayGainTrack)
 	}
-	if firstTrack <= 0 {
-		return p.player.PlayTrackAt(0)
-	}
 	return p.player.PlayTrackAt(firstTrack)
 }
 
@@ -290,20 +275,20 @@ func (p *PlaybackManager) PlayTrackAt(idx int) error {
 }
 
 func (p *PlaybackManager) PlayRandomSongs(genreName string) {
-	if songs, err := p.sm.Server.GetRandomTracks(genreName, 100); err != nil {
+	p.fetchAndPlayTracks(func() ([]*mediaprovider.Track, error) {
-		log.Printf("error getting random songs: %s", err.Error())
+		return p.sm.Server.GetRandomTracks(genreName, 100)
-	} else {
+	})
-		p.LoadTracks(songs, false, false)
-		if p.replayGainCfg.Mode == ReplayGainAuto {
-			p.SetReplayGainMode(player.ReplayGainTrack)
-		}
-		p.PlayFromBeginning()
-	}
 }
 
 func (p *PlaybackManager) PlaySimilarSongs(id string) {
-	if songs, err := p.sm.Server.GetSimilarTracks(id, 100); err != nil {
+	p.fetchAndPlayTracks(func() ([]*mediaprovider.Track, error) {
-		log.Printf("error getting similar songs: %s", err.Error())
+		return p.sm.Server.GetSimilarTracks(id, 100)
+	})
+}
+
+func (p *PlaybackManager) fetchAndPlayTracks(fetchFn func() ([]*mediaprovider.Track, error)) {
+	if songs, err := fetchFn(); err != nil {
+		log.Printf("error fetching tracks: %s", err.Error())
 	} else {
 		p.LoadTracks(songs, false, false)
 		if p.replayGainCfg.Mode == ReplayGainAuto {
@@ -520,7 +505,15 @@ func (p *PlaybackManager) Continue() error {
 }
 
 func (p *PlaybackManager) PlayPause() error {
-	return p.player.PlayPause()
+	switch p.player.GetStatus().State {
+	case player.Playing:
+		return p.player.Pause()
+	case player.Paused:
+		return p.player.Continue()
+	case player.Stopped:
+		return p.PlayFromBeginning()
+	}
+	return errors.New("unreached - invalid player state")
 }
 
 // call BEFORE updating p.nowPlayingIdx
@@ -573,26 +566,40 @@ func (p *PlaybackManager) invokeOnSongChangeCallbacks() {
 	p.lastScrobbled = nil
 }
 
+func (pm *PlaybackManager) invokeNoArgCallbacks(cbs []func()) {
+	if pm.callbacksDisabled {
+		return
+	}
+	for _, cb := range cbs {
+		cb()
+	}
+}
+
 func (p *PlaybackManager) startPollTimePos() {
 	ctx, cancel := context.WithCancel(p.ctx)
 	p.cancelPollPos = cancel
-	p.pollingTick = time.NewTicker(250 * time.Millisecond)
+	pollingTick := time.NewTicker(250 * time.Millisecond)
 
-	// TODO: fix occasional nil pointer dereference on app quit
 	go func() {
 		for {
 			select {
 			case <-ctx.Done():
-				p.pollingTick.Stop()
+				pollingTick.Stop()
-				p.pollingTick = nil
 				return
-			case <-p.pollingTick.C:
+			case <-pollingTick.C:
 				p.doUpdateTimePos()
 			}
 		}
 	}()
 }
 
+func (p *PlaybackManager) stopPollTimePos() {
+	if p.cancelPollPos != nil {
+		p.cancelPollPos()
+		p.cancelPollPos = nil
+	}
+}
+
 func (p *PlaybackManager) doUpdateTimePos() {
 	if p.callbacksDisabled {
 		return
@@ -605,13 +612,3 @@ func (p *PlaybackManager) doUpdateTimePos() {
 		cb(s.TimePos, s.Duration)
 	}
 }
-
-func (p *PlaybackManager) stopPollTimePos() {
-	if p.cancelPollPos != nil {
-		p.cancelPollPos()
-		p.cancelPollPos = nil
-	}
-	if p.pollingTick != nil {
-		p.pollingTick.Stop()
-	}
-}

player/mpv/player.go

@@ -312,11 +312,6 @@ func (p *Player) setPaused(paused bool) error {
 	return err
 }
 
-// Start playback from the first track in the play queue.
-func (p *Player) PlayFromBeginning() error {
-	return p.PlayTrackAt(0)
-}
-
 // Start playback from the specified track index in the play queue.
 func (p *Player) PlayTrackAt(idx int) error {
 	// check if we have anything to play
@@ -333,33 +328,6 @@ func (p *Player) PlayTrackAt(idx int) error {
 	return err
 }
 
-// Begins playback if there is anything in the play queue and player is stopped or paused.
-// If player is playing, pauses playback.
-func (p *Player) PlayPause() error {
-	if !p.initialized {
-		return ErrUnitialized
-	}
-
-	switch p.status.State {
-	case player.Stopped:
-		// check if we have anything to play
-		if c, err := p.getInt64Property("playlist-count"); err == nil && c > 0 {
-			err := p.mpv.Command([]string{"playlist-play-index", "0"})
-			if err == nil {
-				p.setState(player.Playing)
-			}
-			return err
-		}
-		return nil
-	case player.Playing:
-		return p.Pause()
-	case player.Paused:
-		return p.Continue()
-	default:
-		return errors.New("Unknown player state")
-	}
-}
-
 // Pause playback and update the player state
 func (p *Player) Pause() error {
 	if p.status.State != player.Playing {
@@ -382,7 +350,7 @@ func (p *Player) Continue() error {
 		}
 		return err
 	} else if p.status.State == player.Stopped {
-		return p.PlayFromBeginning()
+		return p.PlayTrackAt(0)
 	}
 
 	return nil

player/player.go

@@ -17,7 +17,6 @@ type BasePlayer interface {
 	PlayTrackAt(idx int) error
 	Continue() error
 	Pause() error
-	PlayPause() error
 	Stop() error
 	SeekPrevious() error
 	SeekNext() error