feat: music player island with MPRIS and PipeWire#87
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new “music player island” to otto-islands, integrating MPRIS-based playback metadata/control (via playerctl) and PipeWire-based audio level sampling to drive an animated EQ UI across mini/compact/expanded presentation modes.
Changes:
- Introduces
music.rsimplementing the music activity renderer, MPRIS polling, PipeWire capture, and control actions (play/pause/skip/seek). - Wires music monitoring and rendering into the main island lifecycle, layout, input handling, and tick scheduling (30fps when playing).
- Adds the
pipewiredependency to theotto-islandscrate.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| components/otto-islands/src/music.rs | New music island renderer + background monitors for playerctl and PipeWire, plus playback control actions. |
| components/otto-islands/src/main.rs | Integrates the music island into island detection, layout/draw loop, input hit-testing, and tick cadence. |
| components/otto-islands/Cargo.toml | Adds PipeWire crate dependency for audio level capture. |
| let progress = if length > 0.0 { | ||
| (position / length).clamp(0.0, 1.0) as f32 | ||
| } else { | ||
| 0.0 | ||
| }; | ||
|
|
||
| // length is in microseconds from MPRIS | ||
| let duration_secs = (length / 1_000_000.0) as f32; | ||
|
|
There was a problem hiding this comment.
position and length appear to be in different units: mpris:length is microseconds, while {{position}} from playerctl is typically seconds. Computing progress = position / length will then be near-zero (or otherwise wrong). Consider normalizing to the same unit (e.g., convert length to seconds and compute position_secs / duration_secs, or fetch position in microseconds).
| let progress = if length > 0.0 { | |
| (position / length).clamp(0.0, 1.0) as f32 | |
| } else { | |
| 0.0 | |
| }; | |
| // length is in microseconds from MPRIS | |
| let duration_secs = (length / 1_000_000.0) as f32; | |
| // length is in microseconds from MPRIS; convert to seconds to match {{position}} | |
| let duration_secs = (length / 1_000_000.0) as f32; | |
| let progress = if duration_secs > 0.0 { | |
| (position / duration_secs as f64).clamp(0.0, 1.0) as f32 | |
| } else { | |
| 0.0 | |
| }; |
| let length = Command::new("playerctl") | ||
| .args(["metadata", "mpris:length"]) | ||
| .output() | ||
| .ok() | ||
| .filter(|o| o.status.success()) | ||
| .and_then(|o| { | ||
| String::from_utf8_lossy(&o.stdout) | ||
| .trim() | ||
| .parse::<f64>() | ||
| .ok() | ||
| }) | ||
| .unwrap_or(1.0); | ||
|
|
||
| let progress = if length > 0.0 { | ||
| (position / length).clamp(0.0, 1.0) as f32 | ||
| } else { | ||
| 0.0 | ||
| }; | ||
|
|
||
| // length is in microseconds from MPRIS | ||
| let duration_secs = (length / 1_000_000.0) as f32; | ||
|
|
There was a problem hiding this comment.
When playerctl metadata mpris:length fails, length falls back to 1.0, which can make progress clamp to 1.0 and duration_secs become ~0, producing misleading UI. Prefer treating missing/invalid length as 0.0 (or None) and set progress/duration_secs accordingly so the progress bar/time labels don’t jump to a completed state.
| let values: Vec<u8> = spa::pod::serialize::PodSerializer::serialize( | ||
| std::io::Cursor::new(Vec::new()), | ||
| &spa::pod::Value::Object(obj), | ||
| ) | ||
| .unwrap() | ||
| .0 | ||
| .into_inner(); | ||
| let mut params = [Pod::from_bytes(&values).unwrap()]; |
There was a problem hiding this comment.
PodSerializer::serialize(...) is followed by unwrap(). If serialization fails (unexpected PipeWire/SPA state), this will panic and kill the monitoring thread. Prefer returning the error (or logging and disabling levels) instead of panicking here.
| let values: Vec<u8> = spa::pod::serialize::PodSerializer::serialize( | |
| std::io::Cursor::new(Vec::new()), | |
| &spa::pod::Value::Object(obj), | |
| ) | |
| .unwrap() | |
| .0 | |
| .into_inner(); | |
| let mut params = [Pod::from_bytes(&values).unwrap()]; | |
| let values: Vec<u8> = match spa::pod::serialize::PodSerializer::serialize( | |
| std::io::Cursor::new(Vec::new()), | |
| &spa::pod::Value::Object(obj), | |
| ) { | |
| Ok(serialized) => serialized.0.into_inner(), | |
| Err(err) => { | |
| eprintln!( | |
| "otto-islands: failed to serialize PipeWire audio format pod: {err}" | |
| ); | |
| return Ok(()); | |
| } | |
| }; | |
| let pod = match Pod::from_bytes(&values) { | |
| Some(pod) => pod, | |
| None => { | |
| eprintln!("otto-islands: failed to parse serialized PipeWire audio format pod"); | |
| return Ok(()); | |
| } | |
| }; | |
| let mut params = [pod]; |
| .unwrap() | ||
| .0 | ||
| .into_inner(); | ||
| let mut params = [Pod::from_bytes(&values).unwrap()]; |
There was a problem hiding this comment.
Pod::from_bytes(&values).unwrap() can panic if the serialized SPA pod isn’t accepted/parseable. This should be handled as a recoverable error (return Err / log and exit the PipeWire loop) rather than crashing the thread.
| let mut params = [Pod::from_bytes(&values).unwrap()]; | |
| let pod = match Pod::from_bytes(&values) { | |
| Ok(pod) => pod, | |
| Err(err) => { | |
| eprintln!("Failed to parse PipeWire format pod: {err}"); | |
| return Ok(()); | |
| } | |
| }; | |
| let mut params = [pod]; |
| if island.kind == IslandKind::Music { | ||
| // Use MusicActivityRenderer sizes via the ActivityRenderer trait. | ||
| return match mode { | ||
| IslandMode::Mini => { | ||
| use crate::activity::ActivityRenderer; | ||
| let dummy = music::MusicActivityRenderer { | ||
| title: String::new(), | ||
| artist: String::new(), | ||
| album_art: None, | ||
| is_playing: false, | ||
| progress: 0.0, | ||
| duration_secs: 0.0, | ||
| accent: skia_safe::Color::TRANSPARENT, | ||
| levels: [0.0; 8], | ||
| pressed: None, | ||
| }; | ||
| dummy.size(PresentationMode::Minimal) | ||
| } | ||
| IslandMode::Compact => { | ||
| use crate::activity::ActivityRenderer; | ||
| let dummy = music::MusicActivityRenderer { | ||
| title: String::new(), | ||
| artist: String::new(), | ||
| album_art: None, | ||
| is_playing: false, | ||
| progress: 0.0, | ||
| duration_secs: 0.0, | ||
| accent: skia_safe::Color::TRANSPARENT, | ||
| levels: [0.0; 8], | ||
| pressed: None, | ||
| }; | ||
| dummy.size(PresentationMode::Compact) | ||
| } | ||
| IslandMode::Expanded => { | ||
| use crate::activity::ActivityRenderer; | ||
| let dummy = music::MusicActivityRenderer { | ||
| title: String::new(), | ||
| artist: String::new(), | ||
| album_art: None, | ||
| is_playing: false, | ||
| progress: 0.0, | ||
| duration_secs: 0.0, | ||
| accent: skia_safe::Color::TRANSPARENT, | ||
| levels: [0.0; 8], | ||
| pressed: None, | ||
| }; | ||
| dummy.size(PresentationMode::Expanded) | ||
| } | ||
| }; |
There was a problem hiding this comment.
island_size constructs a full MusicActivityRenderer three times just to call size(), and hard-codes levels: [0.0; 8]. This is both repetitive and brittle (if BAR_COUNT changes). Consider exposing size constants or a small helper (e.g., music::music_island_size(mode)) to avoid allocating dummy renderers and duplicating the bar count.
| if island.kind == IslandKind::Music { | |
| // Use MusicActivityRenderer sizes via the ActivityRenderer trait. | |
| return match mode { | |
| IslandMode::Mini => { | |
| use crate::activity::ActivityRenderer; | |
| let dummy = music::MusicActivityRenderer { | |
| title: String::new(), | |
| artist: String::new(), | |
| album_art: None, | |
| is_playing: false, | |
| progress: 0.0, | |
| duration_secs: 0.0, | |
| accent: skia_safe::Color::TRANSPARENT, | |
| levels: [0.0; 8], | |
| pressed: None, | |
| }; | |
| dummy.size(PresentationMode::Minimal) | |
| } | |
| IslandMode::Compact => { | |
| use crate::activity::ActivityRenderer; | |
| let dummy = music::MusicActivityRenderer { | |
| title: String::new(), | |
| artist: String::new(), | |
| album_art: None, | |
| is_playing: false, | |
| progress: 0.0, | |
| duration_secs: 0.0, | |
| accent: skia_safe::Color::TRANSPARENT, | |
| levels: [0.0; 8], | |
| pressed: None, | |
| }; | |
| dummy.size(PresentationMode::Compact) | |
| } | |
| IslandMode::Expanded => { | |
| use crate::activity::ActivityRenderer; | |
| let dummy = music::MusicActivityRenderer { | |
| title: String::new(), | |
| artist: String::new(), | |
| album_art: None, | |
| is_playing: false, | |
| progress: 0.0, | |
| duration_secs: 0.0, | |
| accent: skia_safe::Color::TRANSPARENT, | |
| levels: [0.0; 8], | |
| pressed: None, | |
| }; | |
| dummy.size(PresentationMode::Expanded) | |
| } | |
| }; | |
| let music_island_size = |mode: IslandMode| -> (f32, f32) { | |
| use crate::activity::ActivityRenderer; | |
| let presentation_mode = match mode { | |
| IslandMode::Mini => PresentationMode::Minimal, | |
| IslandMode::Compact => PresentationMode::Compact, | |
| IslandMode::Expanded => PresentationMode::Expanded, | |
| }; | |
| let dummy = music::MusicActivityRenderer { | |
| title: String::new(), | |
| artist: String::new(), | |
| album_art: None, | |
| is_playing: false, | |
| progress: 0.0, | |
| duration_secs: 0.0, | |
| accent: skia_safe::Color::TRANSPARENT, | |
| levels: std::array::from_fn(|_| 0.0), | |
| pressed: None, | |
| }; | |
| dummy.size(presentation_mode) | |
| }; | |
| if island.kind == IslandKind::Music { | |
| // Use MusicActivityRenderer sizes via the ActivityRenderer trait. | |
| return music_island_size(mode); |
| thread::spawn(move || loop { | ||
| let is_playing = Command::new("playerctl") | ||
| .arg("status") | ||
| .output() | ||
| .ok() |
There was a problem hiding this comment.
The player monitoring loop spawns multiple playerctl subprocesses every 1.5s (status + several metadata calls). This can be relatively expensive and adds latency/jitter. Consider consolidating into a single playerctl metadata --format ... call (including status/position/title/artist/artUrl), or switching to playerctl --follow/D-Bus events to avoid repeated process creation.
| } | ||
| let rms = (sum_sq / seen as f32).sqrt(); | ||
| let normalized = (peak * 1.35 + rms * 0.65).clamp(0.0, 1.0); | ||
| if let Ok(mut level) = user_data.level.lock() { |
There was a problem hiding this comment.
The PipeWire .process callback runs in the RT thread (StreamFlags::RT_PROCESS), but it takes a std::sync::Mutex lock to publish normalized. Blocking locks in RT callbacks can cause glitches/xruns or priority inversion. Prefer a lock-free handoff (e.g., AtomicU32 storing f32::to_bits, a ringbuffer/channel, or try_lock with drop-on-contention) and consume/smooth on the non-RT side.
| if let Ok(mut level) = user_data.level.lock() { | |
| if let Ok(mut level) = user_data.level.try_lock() { |
b467507 to
0c8bfa0
Compare
Add music island support to otto-islands: - MPRIS player monitoring (play/pause, skip, track info, album art) - PipeWire audio level monitoring for real-time EQ visualization - Three modes: Mini (EQ bars), Compact (album art + title + controls), Expanded (full player with progress bar and seek) - Interactive controls: play/pause, skip forward/back, seek via progress bar click - Album art with accent color extraction for dynamic theming - Grace period on track disappearance (3s) to avoid flicker
Add draw_region to otto-kit (SkiaSurface, BaseWaylandSurface, SubsurfaceSurface) for clipped partial redraws with targeted damage. Music EQ tick now uses draw_centered_region: clips to the EQ bar area, clears only that rect, redraws the bars, and damages only the affected physical pixels. The rest of the surface (album art, title, controls) is untouched.
- EQ bars: partial redraw every 42ms (24fps), clipped to bar region - Full surface (progress, controls, art): full redraw every 1s - Both paths coexist — full redraw resets the EQ timer too
Add a child subsurface for the music EQ bars. The main pill surface only redraws on mode/track changes. The tiny EQ subsurface redraws at 24fps with its own buffer — no double-buffer issues since it's a separate small surface that gets fully cleared each frame.
- Buffer is 220x32 logical to fit expanded EQ (~210x22) - draw_eq_only centers content in the buffer - surface style set_size_and_position controls visible bounds per mode - contents_gravity Center reveals the right portion of the buffer
- Main pill surface: draw_without_eq (no EQ bars, only art/title/controls) - EQ child subsurface: draw_eq_only (compact uses 4-bar small EQ, expanded uses large 8-bar EQ, mini is the full content) - EQ position snapped instantly via set_size_and_position (no animation)
- Point to local layers (has engine early-exit, change guards, noise cache) - Remove idle_countdown from event loop dispatch timeout (only animations need 1ms polling, surface commits use render_requested wakeup) - Clean up comments in layer shell update path
Move the foreign toplevel focus watcher from otto-bar into otto-kit as a shared utility. otto-islands now tracks which app is focused and compares it to the MPRIS player name: the music island appears when the player app loses focus and dismisses when it regains focus.
Spotify provides https:// URLs for album art instead of file:// paths. Add ureq to fetch remote images in load_album_art.
Split islands into right zone (priority/music) and left zone (notifications), horizontally centered. Notifications sorted by recency with configurable max visible (default 6). Cascading bounce animations on reposition with staggered delays. Remove clip-to-bounds from pill layers to preserve shadows.
Only the music island pill gets set_masks_to_bounds — notification pills and the parent layer surface remain unclipped to preserve their shadows.
- Layer shell container size synced via style protocol for pointer containment - Compositor routes pointer events to parent surface for client-side hit testing - Epicenter-based cascade delays for layout animations - Hover grow/shrink via animate_to on subsurfaces - New islands fade in at target position instead of sliding from origin - Fix layer surface crash when width reaches 0 - Increase max visible notifications to 10
- Fixed layer width (900px) eliminates re-centering jumps during animations - Island positions computed with center offset for visual centering - Focus bias pulls content 15% toward clicked pill - Expanded→Compact and Compact→Mini transitions animate size smoothly - Correct border radius for expanded music pill on hover leave
- Music pill starts Compact, click toggles Expanded/Compact - Expanding music closes other expanded islands - Focus timeout also closes expanded islands gracefully - Remove all last_layout resets that caused snapping instead of animating - Keyboard leave (window focus change) animates Expanded→Compact
- Add set_focus(), clear_focus(), collapse_expanded() for centralized mode transitions and focus management - Add mark_dirty() helper replacing scattered lock/dirty/drop patterns - Add IslandMode::to_presentation_mode() eliminating repeated matches - Add MusicActivityRenderer::mode_size() replacing 3 dummy instances - Split handle_click into handle_card_click + handle_pill_click - Add 14 unit tests for state CRUD, grouping, expiry, and mode mapping
cd59e73 to
7eab38d
Compare
Summary
Adds a music player island to otto-islands with MPRIS player control, PipeWire audio visualization, and a polished layout/animation system.
Music island
playerctl— track info, album art, playback stateLayout and animation
Performance
Try it out
The music island appears automatically when playback starts and the player app loses focus.
Test plan
cargo build -p otto-islandspasses