Repository navigation
fix(media): keep the mpris position moving while playing - #808
Conversation
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.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (2)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
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. 📝 WalkthroughWalkthroughLes contrôles média relisent la position du moteur chaque seconde pendant l’état ChangesPosition de lecture
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
Merge Risk: ⚪ Minimal · up to The position refresh is ready to merge after normal checks; no blocking playback-position issue was established. Security Architecture ReviewSecurity architecture risk: ⚪ Minimal · up to 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 Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Closes #807
Problem
playerctl positionstays 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
Positionproperty with the lastMediaPlaybackprogress it was handed and never extrapolates it (both thedbusandzbusbackends). We hand it a position only on a state transition (transition_state, which starts a track at 0) and on a seek, soPositionstayed at the start of the track until the next seek.Fix
The
media_controlsthread waits withrecv_timeout(1 s)instead of a blockingrecv. On a timeout, if the last state it published wasPlaying, 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 leftPlaying, so a pause still in flight is never overwritten by a stale Playing.Off the audio path entirely: one D-Bus
PropertiesChangedper 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