Sync lyrics highlighting with playback progress #668
Conversation
Adds progress and last_update_ms to CommonCtx and updates AppState to keep these in sync with playback. Implements a timer-driven LyricsTicker controller to update the lyrics view, highlights the current lyric line based on playback progress, and allows for offset correction when a line is clicked. Also processes lyrics end times for more accurate highlighting.
Added playback_state to CommonCtx and ensured it is updated alongside AppState's playback state. Modified lyrics widget to only advance progress when playback is playing, improving accuracy of lyrics timing during pause and resume.
|
This looks great! One think I was wondering is if your explored tracking the playback state (elapsed time) from the same tracker that is used as the reference for the seek bar UI? It would be nice if we had a single global tracking implementation that the lyrics could then just tie into. |
|
I tried to implement it with the tracking already in place, but I couldn't. It lacks precision on the timer update. It lacks precision on the timer update, which creates a visible discrepancy between speech and sound. I couldn't find any other way but to make my own timer faster, but I admit it's not the best solution. |
jacksongoode
left a comment
jacksongoode
left a comment
There was a problem hiding this comment.
I'm not sure if I can approve this without tying the lyrics syncing to the global playback progress. Having two tickers feels wrong.
Happy to re-review if you're able to move in that approach.
| .active(|c: &Ctx<Arc<CommonCtx>, TrackLines>, _env| { | ||
| let base_progress_ms = c.ctx.progress.as_millis() as f64; | ||
| let now_ms = SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_millis() as f64; | ||
| let elapsed = if matches!(c.ctx.playback_state, PlaybackState::Playing) { | ||
| now_ms - c.ctx.last_update_ms as f64 | ||
| } else { | ||
| 0.0 | ||
| }; | ||
| let progress_ms = base_progress_ms + elapsed; | ||
| let offset = offset_storage().lock().unwrap().unwrap_or(0.0); | ||
| let adj_progress = progress_ms + offset; | ||
| let start_ms = c.data.start_time_ms.parse::<f64>().unwrap_or(0.0); | ||
| let parsed_end = c.data.end_time_ms.parse::<f64>().unwrap_or(0.0); | ||
| let end_ms = if parsed_end > start_ms { parsed_end } else { start_ms + 800.0 }; | ||
| adj_progress >= start_ms && adj_progress < end_ms | ||
| }) |
There was a problem hiding this comment.
I would rather have the lyrics check the active duration of the global song ticker, rather than recreating it.
We could make the track duration updates faster to facilitate the lyrics switch, but this code is quite involved just to make the lyrics update.
Moved progress calculation logic into CommonCtx::current_progress for reuse and clarity. Updated lyrics UI to use the new method, simplifying code and improving maintainability.
Reduced playback position report interval for more frequent updates. Removed redundant playback state and timing fields from CommonCtx, simplifying progress tracking. Refactored lyrics offset logic to be managed in AppState, and removed the LyricsTicker controller and related global state from the lyrics UI.
|
I think my last commit has a better management of timer |
|
This looks a lot better in the last commit. However, and I'm not so familiar with the report precision. It might be interesting if the CPU cycles are greater with this PR. Maybe if you're interested, could you check to make sure this doesn't use a lot more CPU by increasing the timing precision? |
| }) | ||
| .rounded(theme::BUTTON_BORDER_RADIUS) | ||
| .env_scope(|env, _| { | ||
| let active = env.get(theme::BLUE_100).with_alpha(0.25); |
There was a problem hiding this comment.
One comment here, most of the buttons we have don't use blue as the hover. I would use the default hover which is a gray.
2 participants
This pull request introduces enhancements to playback state management and lyrics synchronization within the
psst-guiapplication. Key changes include the addition of playback state tracking in theCommonCtx, a newLyricsTickercontroller for real-time updates, and improvements to the lyrics widget for dynamic synchronization with playback progress.Playback State Management Enhancements:
psst-gui/src/data/mod.rs: Addedprogress,last_update_ms, andplayback_statefields toCommonCtxfor tracking playback progress and state. Introduced a helper methodset_common_progressto update progress and timestamp consistently. [1] [2] [3]Lyrics Synchronization Improvements:
psst-gui/src/ui/lyrics.rs: Added aLyricsTickercontroller for periodic updates to the lyrics widget, ensuring synchronization with playback progress. [1] [2]psst-gui/src/ui/lyrics.rs: Enhanced the lyrics widget to dynamically highlight lyrics based on playback progress, using calculated offsets and playback state. [1] [2]These changes improve the user experience by providing accurate playback state tracking and seamless lyrics synchronization during music playback.
close #658