Skip to content

fix(media): keep the mpris position moving while playing - #808

Merged
InstaZDLL merged 1 commit into
mainfrom
fix/mpris-position
Oct 5, 2026
Merged

InstaZDLL merged 1 commit into
mainfrom
fix/mpris-position

Conversation

@InstaZDLL

@InstaZDLL InstaZDLL commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

Closes #807

Problem

playerctl position stays at 0 while a track plays, so lyrics clients that follow MPRIS (sptlrx and the like) never advance.

souvlaki 0.8.3 answers the MPRIS Position property with the last MediaPlayback progress it was handed and never extrapolates it (both the dbus and zbus backends). We hand it a position only on a state transition (transition_state, which starts a track at 0) and on a seek, so Position stayed at the start of the track until the next seek.

Fix

The media_controls thread waits with recv_timeout(1 s) instead of a blocking recv. On a timeout, if the last state it published was Playing, it re-reads the live position from the engine (SharedPlayback::current_position_ms, which already accounts for speed, seeks and crossfade hand-offs) and republishes it. It skips the publish if the engine has already left Playing, so a pause still in flight is never overwritten by a stale Playing.

Off the audio path entirely: one D-Bus PropertiesChanged per second while playing, nothing while paused or stopped. Windows (SMTC) and macOS get the same refresh, which keeps their timelines accurate too.

Docs: playback.md, OS media controls.

Summary by CodeRabbit

  • Correctifs
    • La position de lecture affichée par les contrôles multimédias est désormais actualisée chaque seconde pendant la lecture.
    • L’actualisation cesse lorsque la lecture s’arrête.

souvlaki answers MPRIS Position with the last value it was handed, and we
handed one only on a state change or a seek, so playerctl position read the
start of the track. The controls thread now re-reads the engine's position
once a second while playing and republishes it.
@InstaZDLL InstaZDLL added scope: backend Rust/Tauri backend (src-tauri/) scope: docs Docs, README, assets type: fix Bug fix size: m 50-200 lines labels Oct 5, 2026
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (2)
AGENTS.md — auto-discovered
CLAUDE.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: InstaZDLL/WaveFlow/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: f2448976-c133-4d01-b690-2ddf1f5b49af
📥 Commits

Reviewing files that changed from the base of the PR and between 03e40dd and e7d9517.

📒 Files selected for processing (2)
  • docs/features/playback.md
  • src-tauri/crates/app/src/media_controls.rs

Included review availability: This review used your included allowance. 9 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.


📝 Walkthrough

Walkthrough

Les contrôles média relisent la position du moteur chaque seconde pendant l’état Playing et la republient à souvlaki. Ils cessent l’actualisation lorsque la lecture s’arrête et quittent la boucle si le canal est déconnecté.

Changes

Position de lecture

Layer / File(s) Summary
Actualisation périodique de la position
src-tauri/crates/app/src/media_controls.rs, docs/features/playback.md
Le thread utilise un délai d’une seconde pour actualiser la position du moteur pendant la lecture. Il ne la publie pas si le moteur n’est plus en lecture. La documentation décrit cette actualisation. La déconnexion du canal termine la boucle.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant MediaControlsThread
  participant TauriAppState
  participant AudioEngine
  participant Souvlaki
  loop Toutes les secondes pendant Playing
    MediaControlsThread->>TauriAppState: Récupérer AudioEngine
    TauriAppState-->>MediaControlsThread: Fournir AudioEngine
    MediaControlsThread->>AudioEngine: Lire la position courante
    AudioEngine-->>MediaControlsThread: Renvoyer la position
    MediaControlsThread->>Souvlaki: Publier la position
  end
Loading

Merge Risk: ⚪ Minimal · up to e7d95

The position refresh is ready to merge after normal checks; no blocking playback-position issue was established.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to e7d95

The refresh uses the existing media-control connection and does not add commands, permissions, or access to additional data. Transition races can briefly affect displayed playback information, but no material security risk was identified in the change.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The incremental exposure is fresher playback-position information on the application's existing OS media session. The inspected change does not extend that session to additional assets, data stores, credentials, or services.

Trust Boundaries and Controls

  • observed — Incoming OS events still pass through the same callback and player-command mapping as at the base revision. The new timeout branch is internally initiated and does not introduce an attacker-supplied argument, a new command sink, or a changed authorization decision.

Resilience and Maintainability Implications

  • observed — Missing engine state makes refresh a no-op, publication errors are logged without changing engine state, and receiver disconnection exits the controls loop. Initialization and attachment failures retain their existing thread-local return paths.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Le titre résume clairement le changement principal et respecte le format Conventional Commits avec une portée en kebab-case.
Description check ✅ Passed La description explique le problème, la solution et l’issue liée. Elle ne précise pas comment le changement a été testé et ne renseigne pas la checklist.
Linked Issues check ✅ Passed [ #807 ] demande une position MPRIS qui progresse pendant la lecture. media_controls.rs attend au plus une seconde entre les messages et, si le dernier état publié est Playing, relit `SharedPlayba…
Out of Scope Changes check ✅ Passed Les changements se limitent à l’actualisation périodique de la position des contrôles multimédias et à sa documentation dans playback.md. Ces changements soutiennent directement l’objectif de l’issu…
Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


Comment @coderabbitai help to get the list of available commands.

@InstaZDLL InstaZDLL self-assigned this Oct 5, 2026
@InstaZDLL
InstaZDLL merged commit 4add4cb into main Oct 5, 2026
17 checks passed
@InstaZDLL
InstaZDLL deleted the fix/mpris-position branch October 5, 2026 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: backend Rust/Tauri backend (src-tauri/) scope: docs Docs, README, assets size: m 50-200 lines type: fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: playerctl position bug

1 participant