diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b8c32df9..36fe4d55d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Crash on Android (Termux) due to unknown user runtime directory - Crash due to misconfigured or unavailable audio backend +- Missing MPRIS signal for volume changes when volume is changed from inside `ncspot` ## [1.0.0] - 2023-12-16 diff --git a/src/application.rs b/src/application.rs index 0f7e885b1..9c881066b 100644 --- a/src/application.rs +++ b/src/application.rs @@ -22,7 +22,7 @@ use crate::{authentication, ui, utils}; use crate::{command, queue, spotify}; #[cfg(feature = "mpris")] -use crate::mpris::{self, MprisManager}; +use crate::mpris::{self, MprisCommand, MprisManager}; #[cfg(unix)] use crate::ipc::{self, IpcSocket}; @@ -115,7 +115,7 @@ impl Application { let event_manager = EventManager::new(cursive.cb_sink().clone()); - let spotify = + let mut spotify = spotify::Spotify::new(event_manager.clone(), credentials, configuration.clone())?; let library = Arc::new(Library::new( @@ -138,6 +138,9 @@ impl Application { spotify.clone(), ); + #[cfg(feature = "mpris")] + spotify.set_mpris(mpris_manager.clone()); + #[cfg(unix)] let ipc = if let Ok(runtime_directory) = utils::create_runtime_directory() { Some( @@ -239,7 +242,7 @@ impl Application { self.spotify.update_status(state.clone()); #[cfg(feature = "mpris")] - self.mpris_manager.update(); + self.mpris_manager.send(MprisCommand::NotifyPlaybackUpdate); #[cfg(unix)] if let Some(ref ipc) = self.ipc { diff --git a/src/commands.rs b/src/commands.rs index 6695ed4ee..a4a2bef61 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -196,7 +196,7 @@ impl CommandManager { .spotify .volume() .saturating_add(VOLUME_PERCENT * amount); - self.spotify.set_volume(volume); + self.spotify.set_volume(volume, true); Ok(None) } Command::VolumeDown(amount) => { @@ -205,7 +205,7 @@ impl CommandManager { .volume() .saturating_sub(VOLUME_PERCENT * amount); debug!("vol {}", volume); - self.spotify.set_volume(volume); + self.spotify.set_volume(volume, true); Ok(None) } Command::Help => { diff --git a/src/mpris.rs b/src/mpris.rs index ef1014545..81fd5cb09 100644 --- a/src/mpris.rs +++ b/src/mpris.rs @@ -1,3 +1,4 @@ +use log::info; use std::collections::HashMap; use std::error::Error; use std::sync::Arc; @@ -274,7 +275,7 @@ impl MprisPlayer { log::info!("set volume: {volume}"); volume = volume.clamp(0.0, 1.0); let vol = (VOLUME_PERCENT as f64) * volume * 100.0; - self.spotify.set_volume(vol as u16); + self.spotify.set_volume(vol as u16, false); self.event.trigger(); } @@ -461,8 +462,19 @@ impl MprisPlayer { } } +/// Commands to control the [MprisManager] worker thread. +pub enum MprisCommand { + /// Notify about playback status and metadata updates. + NotifyPlaybackUpdate, + /// Notify about volume updates. + NotifyVolumeUpdate, +} + +/// An MPRIS server that internally manager a thread which can be sent commands. This is internally +/// shared and cloning it will yield a reference to the same server. +#[derive(Clone)] pub struct MprisManager { - tx: mpsc::UnboundedSender<()>, + tx: mpsc::UnboundedSender, } impl MprisManager { @@ -480,7 +492,7 @@ impl MprisManager { spotify, }; - let (tx, rx) = mpsc::unbounded_channel::<()>(); + let (tx, rx) = mpsc::unbounded_channel::(); ASYNC_RUNTIME.get().unwrap().spawn(async { let result = Self::serve(UnboundedReceiverStream::new(rx), root, player).await; @@ -493,7 +505,7 @@ impl MprisManager { } async fn serve( - mut rx: UnboundedReceiverStream<()>, + mut rx: UnboundedReceiverStream, root: MprisRoot, player: MprisPlayer, ) -> Result<(), Box> { @@ -511,15 +523,24 @@ impl MprisManager { let player_iface = player_iface_ref.get().await; loop { - rx.next().await; let ctx = player_iface_ref.signal_context(); - player_iface.playback_status_changed(ctx).await?; - player_iface.metadata_changed(ctx).await?; + match rx.next().await { + Some(MprisCommand::NotifyPlaybackUpdate) => { + player_iface.playback_status_changed(ctx).await?; + player_iface.metadata_changed(ctx).await?; + } + Some(MprisCommand::NotifyVolumeUpdate) => { + info!("sending MPRIS volume update signal"); + player_iface.volume_changed(ctx).await?; + } + None => break, + } } + Err("MPRIS server command channel closed".into()) } - pub fn update(&self) { - if let Err(e) = self.tx.send(()) { + pub fn send(&self, command: MprisCommand) { + if let Err(e) = self.tx.send(command) { log::warn!("Could not update MPRIS state: {e}"); } } diff --git a/src/spotify.rs b/src/spotify.rs index 3e7bc7943..2455fa322 100644 --- a/src/spotify.rs +++ b/src/spotify.rs @@ -25,6 +25,8 @@ use crate::application::ASYNC_RUNTIME; use crate::config; use crate::events::{Event, EventManager}; use crate::model::playable::Playable; +#[cfg(feature = "mpris")] +use crate::mpris::{MprisCommand, MprisManager}; use crate::spotify_api::WebApi; use crate::spotify_worker::{Worker, WorkerCommand}; @@ -46,6 +48,8 @@ pub enum PlayerEvent { pub struct Spotify { events: EventManager, /// The credentials for the currently logged in user, used to authenticate to the Spotify API. + #[cfg(feature = "mpris")] + mpris: Arc>>, credentials: Credentials, cfg: Arc, /// Playback status of the [Player] owned by the worker thread. @@ -67,6 +71,8 @@ impl Spotify { ) -> Result> { let mut spotify = Self { events, + #[cfg(feature = "mpris")] + mpris: Default::default(), credentials, cfg: cfg.clone(), status: Arc::new(RwLock::new(PlayerEvent::Stopped)), @@ -80,7 +86,7 @@ impl Spotify { spotify.start_worker(Some(user_tx))?; let user = ASYNC_RUNTIME.get().unwrap().block_on(user_rx).ok(); let volume = cfg.state().volume; - spotify.set_volume(volume); + spotify.set_volume(volume, true); spotify.api.set_worker_channel(spotify.channel.clone()); spotify.api.update_token(); @@ -393,11 +399,21 @@ impl Spotify { self.cfg.state().volume } - /// Set the current volume of the [Player]. - pub fn set_volume(&self, volume: u16) { + /// Set the current volume of the [Player]. If `notify` is true, also notify MPRIS clients about + /// the update. + pub fn set_volume(&self, volume: u16, notify: bool) { info!("setting volume to {}", volume); self.cfg.with_state_mut(|s| s.volume = volume); self.send_worker(WorkerCommand::SetVolume(volume)); + // HACK: This is a bit of a hack to prevent duplicate update signals when updating from the + // MPRIS implementation. + if notify { + #[cfg(feature = "mpris")] + if let Some(mpris_manager) = self.mpris.lock().unwrap().as_ref() { + info!("updating MPRIS volume"); + mpris_manager.send(MprisCommand::NotifyVolumeUpdate); + } + } } /// Preload the given [Playable] in the [Player]. This makes sure it can be played immediately @@ -410,6 +426,11 @@ impl Spotify { pub fn shutdown(&self) { self.send_worker(WorkerCommand::Shutdown); } + + #[cfg(feature = "mpris")] + pub fn set_mpris(&mut self, mpris: MprisManager) { + *self.mpris.lock().unwrap() = Some(mpris); + } } /// A type of Spotify URI. diff --git a/src/ui/statusbar.rs b/src/ui/statusbar.rs index fd937360b..6169f7309 100644 --- a/src/ui/statusbar.rs +++ b/src/ui/statusbar.rs @@ -238,7 +238,7 @@ impl View for StatusBar { .volume() .saturating_add(crate::spotify::VOLUME_PERCENT); - self.spotify.set_volume(volume); + self.spotify.set_volume(volume, true); } if event == MouseEvent::WheelDown { @@ -247,7 +247,7 @@ impl View for StatusBar { .volume() .saturating_sub(crate::spotify::VOLUME_PERCENT); - self.spotify.set_volume(volume); + self.spotify.set_volume(volume, true); } } else if event == MouseEvent::Press(MouseButton::Left) { self.queue.toggleplayback();