Merge pull request #867 from dweymouth/fix/no-playback-on-restore

Prevent scrobbling and short playback artifacts when loading the app with previous track paused

backend/app.go

@@ -683,11 +683,7 @@ func (a *App) LoadSavedPlayQueue() error {
 	}
 
 	if playQueue.TrackIndex >= 0 && playQueue.TrackIndex < len(playQueue.Tracks) {
-		// TODO: This isn't ideal but doesn't seem to cause an audible play-for-a-split-second artifact
+		a.PlaybackManager.LoadTrackPaused(playQueue.TrackIndex, playQueue.TimePos)
-		a.PlaybackManager.PlayTrackAt(playQueue.TrackIndex)
-		a.PlaybackManager.Pause()
-		time.Sleep(100 * time.Millisecond) // MPV seek fails if run quickly after
-		a.PlaybackManager.SeekSeconds(playQueue.TimePos)
 	}
 
 	return nil

backend/playbackcommands.go

@@ -30,6 +30,8 @@ const (
 	cmdLoadRadioStation // arg: *mediaprovider.RadioStation, arg2: InsertQueueMode
 
 	cmdForceRestartPlayback
+
+	cmdLoadTrackPaused // arg: int (idx), arg2: float64 (startTime)
 )
 
 type playbackCommand struct {
@@ -185,6 +187,17 @@ func (c *playbackCommandQueue) SetQueueState(tracks []*mediaprovider.Track, queu
 	c.cmdAvailable.Signal()
 }
 
+func (c *playbackCommandQueue) LoadTrackPaused(idx int, startTime float64) {
+	c.mutex.Lock()
+	c.queue = append(c.queue, playbackCommand{
+		Type: cmdLoadTrackPaused,
+		Arg:  idx,
+		Arg2: startTime,
+	})
+	c.mutex.Unlock()
+	c.cmdAvailable.Signal()
+}
+
 func (c *playbackCommandQueue) addCommand(command playbackCommand) {
 	c.mutex.Lock()
 	c.queue = append(c.queue, command)

backend/playbackengine.go

@@ -93,6 +93,11 @@ type playbackEngine struct {
 	pendingPlayerChange       bool
 	pendingPlayerChangeStatus player.Status
 
+	// set when restoring session state: track is conceptually loaded+paused
+	// but MPV hasn't been touched yet; cleared on Continue or playTrackAt
+	pendingLoadPaused    bool
+	pendingLoadStartTime float64
+
 	// Whether we need to set the next track on the Player
 	// before the current track completes (normally when 10 seconds remain
 	// in the time pos polling function)
@@ -191,9 +196,13 @@ func (p *playbackEngine) SetPlayer(pl player.BasePlayer) error {
 	needToUnpause := false
 
 	stat := p.CurrentPlayer().GetStatus()
-	if p.pendingPlayerChange {
+	if p.pendingPlayerChange || p.pendingLoadPaused {
 		stat.State = player.Paused
 	}
+	if p.pendingLoadPaused {
+		stat.TimePos = p.pendingLoadStartTime
+		stat.Duration = p.curTrackDuration
+	}
 
 	switch stat.State {
 	case player.Stopped:
@@ -301,10 +310,40 @@ func (p *playbackEngine) PlayTrackAt(idx int) error {
 	return p.playTrackAt(idx, 0)
 }
 
+// loadTrackPaused sets up engine state as if the track at idx is loaded and
+// paused at startTime, firing UI/OS-integration callbacks, but does NOT touch
+// the underlying player. Call Continue() to actually begin playback.
+func (p *playbackEngine) loadTrackPaused(idx int, startTime float64) error {
+	if l := p.getPlayQueueLength(); idx < 0 || idx >= l {
+		return fmt.Errorf("track index (%d) out of range (0-%d)", idx, l)
+	}
+	p.nowPlayingIdx = idx
+	nowPlaying := p.getPlayQueueItemAt(idx)
+	_, p.isRadio = nowPlaying.(*mediaprovider.RadioStation)
+	p.wasStopped = false
+	p.alreadyScrobbled = false
+	p.curTrackDuration = nowPlaying.Metadata().Duration.Seconds()
+	p.pendingLoadPaused = true
+	p.pendingLoadStartTime = startTime
+
+	// Pre-generate the waveform image for the paused track using the same hook
+	// that normally pre-generates waveforms for the upcoming track.
+	for _, cb := range p.onBeforeSongChange {
+		cb(nowPlaying)
+	}
+
+	p.invokeOnSongChangeCallbacks()
+	p.invokeNoArgCallbacks(p.onPaused)
+	p.handleTimePosUpdate(false)
+	p.handleNextTrackUpdated()
+	return nil
+}
+
 func (p *playbackEngine) playTrackAt(idx int, startTime float64) error {
 	if l := p.getPlayQueueLength(); idx < 0 || idx >= l {
 		return fmt.Errorf("track index (%d) out of range (0-%d)", idx, l)
 	}
+	p.pendingLoadPaused = false
 	// scrobble current track if needed
 	p.checkScrobble()
 	p.alreadyScrobbled = true
@@ -315,7 +354,10 @@ func (p *playbackEngine) playTrackAt(idx int, startTime float64) error {
 
 // Gets the curently playing media item, if any.
 func (p *playbackEngine) NowPlaying() mediaprovider.MediaItem {
-	if p.nowPlayingIdx < 0 || p.getPlayQueueLength() == 0 || p.player.GetStatus().State == player.Stopped {
+	if p.nowPlayingIdx < 0 || p.getPlayQueueLength() == 0 {
+		return nil
+	}
+	if !p.pendingLoadPaused && p.player.GetStatus().State == player.Stopped {
 		return nil
 	}
 	return p.getPlayQueueItemAt(p.nowPlayingIdx)
@@ -395,6 +437,13 @@ func (p *playbackEngine) SetShuffle(shuffle bool) {
 }
 
 func (p *playbackEngine) PlaybackStatus() PlaybackStatus {
+	if p.pendingLoadPaused {
+		return PlaybackStatus{
+			State:    player.Paused,
+			TimePos:  p.pendingLoadStartTime,
+			Duration: p.curTrackDuration,
+		}
+	}
 	stat := p.pendingPlayerChangeStatus
 	if !p.pendingPlayerChange {
 		stat = p.CurrentPlayer().GetStatus()
@@ -466,6 +515,11 @@ func (p *playbackEngine) IsSeeking() bool {
 }
 
 func (p *playbackEngine) Stop() error {
+	if p.pendingLoadPaused {
+		p.pendingLoadPaused = false
+		p.pendingLoadStartTime = 0
+		p.handleOnStopped()
+	}
 	return p.player.Stop(false)
 }
 
@@ -478,6 +532,15 @@ func (p *playbackEngine) Pause() error {
 }
 
 func (p *playbackEngine) Continue() error {
+	if p.pendingLoadPaused {
+		p.pendingLoadPaused = false
+		// Use pendingTrackChangeNum so handleOnTrackChange doesn't advance nowPlayingIdx.
+		// Set alreadyScrobbled so it doesn't try to scrobble the not-yet-played track.
+		p.pendingTrackChangeNum = p.nowPlayingIdx
+		p.alreadyScrobbled = true
+		return p.setTrack(p.nowPlayingIdx, false, p.pendingLoadStartTime)
+	}
+
 	if p.pendingPlayerChange {
 		p.pendingPlayerChange = false
 		return p.playTrackAt(p.nowPlayingIdx, p.pendingPlayerChangeStatus.TimePos)

backend/playbackmanager.go

@@ -45,7 +45,8 @@ type PlaybackManager struct {
 	wfmImageJobs         [3]*WaveformImageJob
 
 	// whether autoplay tracks are currently being fetched/enqueued
-	pendingAutoplay bool
+	pendingAutoplay    bool
+	wasLoadTrackPaused bool
 }
 
 type RemotePlaybackDevice struct {
@@ -115,18 +116,31 @@ func (p *PlaybackManager) addOnTrackChangeHook() {
 	})
 
 	p.engine.onBeforeSongChange = append(p.engine.onBeforeSongChange, func(item mediaprovider.MediaItem) {
-		if p.engine.playbackCfg.UseWaveformSeekbar {
+		if item == nil || !p.engine.playbackCfg.UseWaveformSeekbar {
-			if p.wfmGen != nil && item != nil && item.Metadata().Type == mediaprovider.MediaItemTypeTrack {
+			return
-				if _, ok := p.findWfmImageJob(item.Metadata().ID, true); !ok {
+		}
-					// start generating waveform image for next-up track
+		if p.wfmGen != nil && item.Metadata().Type == mediaprovider.MediaItemTypeTrack {
-					p.addWfmImageJob(p.wfmGen.StartWaveformGeneration(item.(*mediaprovider.Track)))
+			if _, ok := p.findWfmImageJob(item.Metadata().ID, true); !ok {
-				}
+				// start generating waveform image for next-up track
+				p.addWfmImageJob(p.wfmGen.StartWaveformGeneration(item.(*mediaprovider.Track)))
 			}
 		}
+		if p.isLoadTrackPaused() {
+			// we need to call handleWaveformImageSongChange to ensure the waveform image is updated
+			// for the track that is loaded paused when starting the app
+			p.handleWaveformImageSongChange(item)
+			p.wasLoadTrackPaused = true
+		}
 	})
 
 	p.OnSongChange(func(item mediaprovider.MediaItem, _ *mediaprovider.Track) {
-		p.handleWaveformImageSongChange(item)
+		if p.wasLoadTrackPaused {
+			// if the song change was triggered by LoadTrackPaused when starting the app,
+			// we already called handleWaveformImageSongChange in the onBeforeSongChange hook above
+			p.wasLoadTrackPaused = false
+		} else {
+			p.handleWaveformImageSongChange(item)
+		}
 
 		if runtime.GOOS != "windows" {
 			return
@@ -510,6 +524,17 @@ func (p *PlaybackManager) PlayTrackAt(idx int) {
 	p.cmdQueue.PlayTrackAt(idx)
 }
 
+// LoadTrackPaused sets up engine state as if the track at idx is loaded and
+// paused at startTime, updating the UI and OS media integrations, without
+// starting MPV. Call Continue (or PlayPause) to begin actual playback.
+func (p *PlaybackManager) LoadTrackPaused(idx int, startTime float64) {
+	p.cmdQueue.LoadTrackPaused(idx, startTime)
+}
+
+func (p *PlaybackManager) isLoadTrackPaused() bool {
+	return p.engine.pendingLoadPaused
+}
+
 func (p *PlaybackManager) PlayRandomSongs(genreName string) error {
 	return p.fetchAndPlayTracks(func() ([]*mediaprovider.Track, error) {
 		tr, err := p.engine.sm.Server.GetRandomTracks(genreName, p.appCfg.EnqueueBatchSize)
@@ -895,6 +920,8 @@ func (p *PlaybackManager) runCmdQueue(ctx context.Context) {
 					c.Arg.(*mediaprovider.RadioStation),
 					c.Arg2.(InsertQueueMode),
 				)
+			case cmdLoadTrackPaused:
+				logIfErr("LoadTrackPaused", p.engine.loadTrackPaused(c.Arg.(int), c.Arg2.(float64)))
 			case cmdForceRestartPlayback:
 				if mpv, ok := p.engine.CurrentPlayer().(*mpv.Player); ok {
 					log.Println("Force-restarting MPV playback")

ui/bottompanel.go

@@ -47,7 +47,10 @@ func NewBottomPanel(pm *backend.PlaybackManager, im *backend.ImageManager, contr
 
 	pm.OnPaused(util.FyneDoFunc(func() { bp.Controls.SetPlaying(false) }))
 	pm.OnPlaying(util.FyneDoFunc(func() { bp.Controls.SetPlaying(true) }))
-	pm.OnStopped(util.FyneDoFunc(func() { bp.Controls.SetPlaying(false) }))
+	pm.OnStopped(util.FyneDoFunc(func() {
+		bp.Controls.SetPlaying(false)
+		bp.Controls.UpdatePlayTime(0, 0)
+	}))
 
 	bp.NowPlaying = widgets.NewNowPlayingCard()
 	bp.NowPlaying.ShowAlbumYear = cfg.AlbumsPage.ShowYears