fix(mpris): missing PropertyChanged signal for volume

fix(mpris): missing PropertyChanged signal for volume

Send a `PropertyChanged` signal for the MPRIS volume when the volume
changes inside `ncspot`.
This commit is contained in:
Thomas Frans
2024-02-19 22:46:00 +01:00
committed by GitHub
parent 5e916fd7ec
commit 5b4a17597d
6 changed files with 65 additions and 19 deletions

View File

@@ -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

View File

@@ -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 {

View File

@@ -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 => {

View File

@@ -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<MprisCommand>,
}
impl MprisManager {
@@ -480,7 +492,7 @@ impl MprisManager {
spotify,
};
let (tx, rx) = mpsc::unbounded_channel::<()>();
let (tx, rx) = mpsc::unbounded_channel::<MprisCommand>();
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<MprisCommand>,
root: MprisRoot,
player: MprisPlayer,
) -> Result<(), Box<dyn Error + Sync + Send>> {
@@ -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}");
}
}

View File

@@ -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<std::sync::Mutex<Option<MprisManager>>>,
credentials: Credentials,
cfg: Arc<config::Config>,
/// Playback status of the [Player] owned by the worker thread.
@@ -67,6 +71,8 @@ impl Spotify {
) -> Result<Self, Box<dyn Error>> {
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
@@ -396,11 +402,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
@@ -413,6 +429,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.

View File

@@ -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();