fix(mpris): Fix position tracking and seeking#85
fix(mpris): Fix position tracking and seeking#85LargeModGames merged 1 commit intoLargeModGames:mainfrom
Conversation
- Fix relative seek calculation (was treating offset as absolute position) - Add SetPosition support for widgets using absolute positioning - Sync MPRIS position with player events via set_position() - Emit Seeked signal so external clients update their displays - Update song_progress_ms on seek so UI updates even when paused
There was a problem hiding this comment.
Pull request overview
Fixes broken MPRIS position reporting/seeking so external MPRIS clients (e.g., playerctl, widgets) correctly track playback position and can seek reliably, including absolute seeking via SetPosition.
Changes:
- Correct relative seek handling by computing new absolute position from current position + offset, and add support for absolute
SetPosition. - Synchronize MPRIS-reported position with native player
PositionChangedevents and emitSeekedon jumps. - Update in-app progress state on MPRIS seeks so the UI refreshes even while paused.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/mpris.rs | Adds SetPosition handling and introduces a Seeked command path to emit the MPRIS Seeked signal. |
| src/main.rs | Tracks current position via an atomic, recalculates relative seeks properly, updates UI/app state on seek, and syncs MPRIS position continuously. |
|
|
||
| // Calculate new position, clamping to prevent going negative | ||
| let new_position_ms = (current_ms + offset_ms).max(0) as u32; | ||
|
|
||
| // Seek the player | ||
| player.seek(new_position_ms); | ||
|
|
||
| // Update shared position immediately so UI reflects the change | ||
| shared_position.store(new_position_ms as u64, Ordering::Relaxed); | ||
|
|
||
| // Update app's song_progress_ms so UI updates even when paused | ||
| if let Ok(mut app_lock) = app.try_lock() { | ||
| app_lock.song_progress_ms = new_position_ms as u128; | ||
| } | ||
|
|
||
| // Emit Seeked signal so external clients know position jumped | ||
| mpris_manager.emit_seeked(new_position_ms as u64); | ||
| } | ||
| MprisEvent::SetPosition(position_micros) => { | ||
| // MPRIS SetPosition sends absolute position in microseconds | ||
| // Convert to milliseconds and seek directly | ||
| let new_position_ms = (position_micros / 1000).max(0) as u32; | ||
|
|
There was a problem hiding this comment.
new_position_ms is computed as i64 and then cast with as u32. If current_ms + offset_ms (or position_micros/1000) exceeds u32::MAX, the cast will wrap/truncate and can seek to an unintended position. Clamp to an upper bound before converting (e.g., clamp to u32::MAX or track duration if available), or keep the value as u64 until the final validated conversion to u32 for player.seek().
| MprisCommand::Position(position_ms) => { | ||
| // Silent position update (for regular playback progress) | ||
| player.set_position(Time::from_millis(position_ms as i64)); | ||
| } | ||
| MprisCommand::Seeked(position_ms) => { | ||
| // Update position AND emit Seeked signal so clients know to refresh | ||
| let time = Time::from_millis(position_ms as i64); | ||
| player.set_position(time); | ||
| if let Err(e) = player.seeked(time).await { |
There was a problem hiding this comment.
Time::from_millis(position_ms as i64) casts from u64 to i64 with as, which can wrap for large values. Even if current callers pass small positions, this is an API footgun (and Seeked uses the same pattern). Consider changing Position/Seeked to carry u32 milliseconds (matching track durations elsewhere), or use a checked conversion/clamp before building Time so invalid/oversized positions can’t become negative/incorrect.
Summary
Fix the broken MPRIS progression sync and make the modification of the song progress work.
Testing
Manual testing:
Additional notes
What was broken
What's fixed