Fix waveform seekbar staying stuck when playing radio after app restart - #1017
Open
systemfreund wants to merge 1 commit into
Open
Fix waveform seekbar staying stuck when playing radio after app restart#1017systemfreund wants to merge 1 commit into
systemfreund wants to merge 1 commit into
Conversation
Two related bugs in the "Use waveform seekbar" feature, both around restoring a paused track on app startup (LoadTrackPaused): 1. The onBeforeSongChange hook was overloaded for two purposes: (a) pre-generating a waveform image for the next-up track, and (b) detecting the special "just loaded paused at startup" case via engine.pendingLoadPaused. Because loadTrackPaused() also calls handleNextTrackUpdated() (to pre-cache the *next* queued track), and pendingLoadPaused is still true at that point, the "startup" branch fired a second time for the wrong item (the next-up track instead of the current one), leaving a stale `wasLoadTrackPaused` flag set to true. That stale flag then caused the *next* real OnSongChange event (e.g. starting an internet radio station) to wrongly skip updating the waveform display, so the previous track's waveform stayed on screen even though a radio station (which has no waveform) was now playing. Fix: split the overloaded hook into two: onBeforeSongChange keeps its precache-only purpose, and a new onLoadTrackPaused hook is fired exactly once, directly from loadTrackPaused, for the actual now-playing item. It can no longer be retriggered by unrelated precache calls. 2. With that double-invocation removed, a second, previously-masked bug became visible: cacheNextTracks() only requests caching for play queue indices > 0, excluding index 0. Since waveform generation piggybacks on the audio cache download (it waits for AudioCache.CacheFile() to have been called for the track), the currently-loaded-paused track's waveform would never start generating if that track happened to be at queue index 0 (e.g. the only track in the queue). Previously this was masked because bug (1) would coincidentally kick off caching/generation for the *next* track and display that instead. Fixed the off-by-one (idx > 0 -> idx >= 0) so the current track is always included. Co-Authored-By: Claude Sonnet 5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1016
Problem
With "Use waveform seekbar" enabled: if Supersonic is closed while a normal track is loaded, then restarted (last track shown paused, waveform correctly rendered), starting an internet radio station afterwards leaves the previous track's waveform image displayed instead of clearing it — even though radio streams have no waveform. This only happens right after a restart; under normal operation switching to radio clears/switches the display correctly.
Root cause
Two related bugs, both around restoring a paused track on startup (
LoadTrackPaused):Stale flag causes waveform update to be skipped.
onBeforeSongChangewas overloaded for two purposes: (a) pre-generating a waveform image for the next-up track, and (b) detecting the special "just loaded paused at startup" case viaengine.pendingLoadPaused. BecauseloadTrackPaused()also callshandleNextTrackUpdated()(to pre-cache the next queued track), andpendingLoadPausedis stilltrueat that point, the "startup" branch fired a second time for the wrong item (the next-up track instead of the current one), leavingwasLoadTrackPausedstuck attrue. That stale flag then made the next realOnSongChangeevent (e.g. starting an internet radio station) wrongly skip updating the waveform display, so the previous track's waveform stayed on screen.Fix: split the overloaded hook into two —
onBeforeSongChangekeeps its precache-only purpose, and a newonLoadTrackPausedhook fires exactly once, directly fromloadTrackPaused, for the actual now-playing item. It can no longer be retriggered by unrelated precache calls.Previously-masked off-by-one in
cacheNextTracks(). With the double-invocation above removed, a second, previously-masked bug became visible:cacheNextTracks()only requests caching for play-queue indices> 0, excluding index 0. Since waveform generation piggybacks on the audio cache download (it waits forAudioCache.CacheFile()to have been called for the track), the currently-loaded-paused track's waveform would never start generating if that track happened to be at queue index 0 (e.g. the only track in the queue). Previously this was masked because bug (1) would coincidentally kick off caching/generation for the next track and display that instead. Fixed the off-by-one (idx > 0→idx >= 0) so the current track is always included.Testing
go build ./...,go vet ./backend/...,go test ./backend/...all pass.