From 0f8e6740b262958408ddc901220c4613b3f517ca Mon Sep 17 00:00:00 2001 From: Sinono3 Date: Thu, 2 Mar 2023 16:41:10 -0300 Subject: [PATCH 1/6] Implement volume for dbus and zbus --- src/lib.rs | 4 +++- src/platform/mpris/dbus.rs | 32 +++++++++++++++++++++++++++++--- src/platform/mpris/zbus.rs | 24 +++++++++++++++++++++--- 3 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index eec7e93..74e054b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -27,7 +27,7 @@ pub struct MediaMetadata<'a> { } /// Events sent by the OS media controls. -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, PartialEq, Debug)] pub enum MediaControlEvent { Play, Pause, @@ -42,6 +42,8 @@ pub enum MediaControlEvent { SeekBy(SeekDirection, Duration), /// Set the position/progress of the currently playing media item. SetPosition(MediaPosition), + /// Sets the volume from 0.0 to 1.0. + SetVolume(f64), /// Open the URI in the media player. OpenUri(String), diff --git a/src/platform/mpris/dbus.rs b/src/platform/mpris/dbus.rs index 0c7ac7f..fa42880 100644 --- a/src/platform/mpris/dbus.rs +++ b/src/platform/mpris/dbus.rs @@ -25,10 +25,11 @@ struct ServiceThreadHandle { thread: JoinHandle<()>, } -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, PartialEq, Debug)] enum InternalEvent { ChangeMetadata(OwnedMetadata), ChangePlayback(MediaPlayback), + ChangeVolume(f64), Kill, } @@ -37,6 +38,7 @@ struct ServiceState { metadata: OwnedMetadata, metadata_dict: HashMap>>, playback_status: MediaPlayback, + volume: f64, } impl ServiceState { @@ -175,6 +177,12 @@ impl MediaControls { Ok(()) } + /// Set the volume level (0.0-1.0) (Only available on MPRIS) + pub fn set_volume(&mut self, volume: f64) -> Result<(), Error> { + self.send_internal_event(InternalEvent::ChangeVolume(volume)); + Ok(()) + } + // TODO: result fn send_internal_event(&mut self, event: InternalEvent) { let channel = &self.thread.as_ref().unwrap().event_channel; @@ -246,6 +254,7 @@ where metadata: Default::default(), metadata_dict: HashMap::new(), playback_status: MediaPlayback::Stopped, + volume: 1.0, }; state.set_metadata(Default::default()); @@ -343,8 +352,20 @@ where .emits_changed_true(); b.property("Volume") - .get(|_, _| Ok(1.0)) - .set(|_, _, _| Ok(Some(1.0))) + .get({ + let state = state.clone(); + move |_, _| { + let state = state.lock().unwrap(); + Ok(state.volume) + } + }) + .set({ + let event_handler = event_handler.clone(); + move |_, _, volume: f64| { + (event_handler.lock().unwrap())(MediaControlEvent::SetVolume(volume)); + Ok(Some(volume)) + } + }) .emits_changed_true(); b.property("Position").get({ @@ -432,6 +453,11 @@ where Variant(Box::new(state.get_playback_status().to_string())), ); } + InternalEvent::ChangeVolume(volume) => { + let mut state = state.lock().unwrap(); + state.volume = volume; + changed_properties.insert("Volume".to_owned(), Variant(Box::new(volume))); + } _ => (), } diff --git a/src/platform/mpris/zbus.rs b/src/platform/mpris/zbus.rs index b5ca004..d77a44f 100644 --- a/src/platform/mpris/zbus.rs +++ b/src/platform/mpris/zbus.rs @@ -29,10 +29,11 @@ struct ServiceThreadHandle { thread: JoinHandle<()>, } -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, PartialEq, Debug)] enum InternalEvent { ChangeMetadata(OwnedMetadata), ChangePlayback(MediaPlayback), + ChangeVolume(f64), Kill, } @@ -40,6 +41,7 @@ enum InternalEvent { struct ServiceState { metadata: OwnedMetadata, playback_status: MediaPlayback, + volume: f64, } #[derive(Clone, PartialEq, Eq, Debug, Default)] @@ -125,6 +127,12 @@ impl MediaControls { Ok(()) } + /// Set the volume level (0.0 - 1.0) (Only available on MPRIS) + pub fn set_volume(&mut self, volume: f64) -> Result<(), Error> { + self.send_internal_event(InternalEvent::ChangeVolume(volume)); + Ok(()) + } + // TODO: result fn send_internal_event(&mut self, event: InternalEvent) { let channel = &self.thread.as_ref().unwrap().event_channel; @@ -307,7 +315,12 @@ impl PlayerInterface { #[dbus_interface(property)] fn volume(&self) -> f64 { - 1.0 + self.state.volume + } + + #[dbus_interface(property)] + fn set_volume(&self, volume: f64) { + self.send_event(MediaControlEvent::SetVolume(volume)); } #[dbus_interface(property)] @@ -381,6 +394,7 @@ async fn run_service( state: ServiceState { metadata: OwnedMetadata::default(), playback_status: MediaPlayback::Stopped, + volume: 1.0, }, event_handler, }; @@ -416,7 +430,11 @@ async fn run_service( interface.state.playback_status = playback; interface.playback_status_changed(&ctxt).await?; } - _ => (), + InternalEvent::ChangeVolume(volume) => { + interface.state.volume = volume; + interface.volume_changed(&ctxt).await?; + } + InternalEvent::Kill => (), } } } From 36ff35d753757e644b6deccf0e618bc99b39ebea Mon Sep 17 00:00:00 2001 From: Sinono3 Date: Thu, 21 Dec 2023 17:09:43 -0300 Subject: [PATCH 2/6] Rename example --- README.md | 2 +- examples/{example.rs => window.rs} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename examples/{example.rs => window.rs} (100%) diff --git a/README.md b/README.md index df7f316..d52a43b 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ A cross-platform library for handling OS media controls and metadata. One abstra ```shell # In one shell $ cd souvlaki -$ cargo run --example example +$ cargo run --example window # In another shell $ playerctl metadata diff --git a/examples/example.rs b/examples/window.rs similarity index 100% rename from examples/example.rs rename to examples/window.rs From d219527625346dfa06ce5e6d626a0b746eff0d25 Mon Sep 17 00:00:00 2001 From: Sinono3 Date: Mon, 1 Jan 2024 22:07:19 -0300 Subject: [PATCH 3/6] Move `dbus` module into its own directory --- src/platform/mpris/{dbus.rs => dbus/mod.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/platform/mpris/{dbus.rs => dbus/mod.rs} (100%) diff --git a/src/platform/mpris/dbus.rs b/src/platform/mpris/dbus/mod.rs similarity index 100% rename from src/platform/mpris/dbus.rs rename to src/platform/mpris/dbus/mod.rs From 5c5db700d786339c9a316e8001792d8eac6eb9dd Mon Sep 17 00:00:00 2001 From: Sinono3 Date: Mon, 1 Jan 2024 22:23:06 -0300 Subject: [PATCH 4/6] Refactor `dbus` module - Created a new module to store functions that register the MPRIS interfaces - Removed a few unwraps - Fixed a few clippy lints --- src/platform/mpris/dbus/interfaces.rs | 236 +++++++++++++++++++++++++ src/platform/mpris/dbus/mod.rs | 245 ++------------------------ 2 files changed, 251 insertions(+), 230 deletions(-) create mode 100644 src/platform/mpris/dbus/interfaces.rs diff --git a/src/platform/mpris/dbus/interfaces.rs b/src/platform/mpris/dbus/interfaces.rs new file mode 100644 index 0000000..f8527bf --- /dev/null +++ b/src/platform/mpris/dbus/interfaces.rs @@ -0,0 +1,236 @@ +use std::{ + convert::{TryFrom, TryInto}, + sync::{Arc, Mutex}, + time::Duration, +}; + +use dbus::Path; +use dbus_crossroads::{Crossroads, IfaceBuilder}; + +use crate::{MediaControlEvent, MediaPlayback, MediaPosition, SeekDirection}; + +use super::{create_metadata_dict, ServiceState}; + +// TODO: This type is super messed up, but it's the only way to get seeking working properly +// on graphical media controls using dbus-crossroads. +pub type SeekedSignal = + Arc, &(String,)) -> dbus::Message + Send + Sync>>>>; + +pub fn register_methods( + state: &Arc>, + event_handler: &Arc>, + friendly_name: String, + seeked_signal: SeekedSignal, +) -> Crossroads +where + F: Fn(MediaControlEvent) + Send + 'static, +{ + let mut cr = Crossroads::new(); + let app_interface = cr.register("org.mpris.MediaPlayer2", { + let event_handler = event_handler.clone(); + + move |b| { + b.property("Identity") + .get(move |_, _| Ok(friendly_name.clone())); + + register_method(b, &event_handler, "Raise", MediaControlEvent::Raise); + register_method(b, &event_handler, "Quit", MediaControlEvent::Quit); + + // TODO: allow user to set these properties + b.property("CanQuit") + .get(|_, _| Ok(true)) + .emits_changed_true(); + b.property("CanRaise") + .get(|_, _| Ok(true)) + .emits_changed_true(); + b.property("HasTracklist") + .get(|_, _| Ok(false)) + .emits_changed_true(); + b.property("SupportedUriSchemes") + .get(move |_, _| Ok(&[] as &[String])) + .emits_changed_true(); + b.property("SupportedMimeTypes") + .get(move |_, _| Ok(&[] as &[String])) + .emits_changed_true(); + } + }); + + let player_interface = cr.register("org.mpris.MediaPlayer2.Player", |b| { + register_method(b, event_handler, "Next", MediaControlEvent::Next); + register_method(b, event_handler, "Previous", MediaControlEvent::Previous); + register_method(b, event_handler, "Pause", MediaControlEvent::Pause); + register_method(b, event_handler, "PlayPause", MediaControlEvent::Toggle); + register_method(b, event_handler, "Stop", MediaControlEvent::Stop); + register_method(b, event_handler, "Play", MediaControlEvent::Play); + + b.method("Seek", ("Offset",), (), { + let event_handler = event_handler.clone(); + + move |ctx, _, (offset,): (i64,)| { + let abs_offset = offset.unsigned_abs(); + let direction = if offset > 0 { + SeekDirection::Forward + } else { + SeekDirection::Backward + }; + + (event_handler.lock().unwrap())(MediaControlEvent::SeekBy( + direction, + Duration::from_micros(abs_offset), + )); + ctx.push_msg(ctx.make_signal("Seeked", ())); + Ok(()) + } + }); + + b.method("SetPosition", ("TrackId", "Position"), (), { + let state = state.clone(); + let event_handler = event_handler.clone(); + + move |_, _, (_trackid, position): (Path, i64)| { + let state = state.lock().unwrap(); + + // According to the MPRIS specification: + + // TODO: If the TrackId argument is not the same as the current + // trackid, the call is ignored as stale. + // (Maybe it should be optional?) + + if let Some(duration) = state.metadata.duration { + // If the Position argument is greater than the track length, do nothing. + if position > duration { + return Ok(()); + } + } + + // If the Position argument is less than 0, do nothing. + if let Ok(position) = u64::try_from(position) { + let position = Duration::from_micros(position); + + (event_handler.lock().unwrap())(MediaControlEvent::SetPosition(MediaPosition( + position, + ))); + } + Ok(()) + } + }); + + b.method("OpenUri", ("Uri",), (), { + let event_handler = event_handler.clone(); + + move |_, _, (uri,): (String,)| { + (event_handler.lock().unwrap())(MediaControlEvent::OpenUri(uri)); + Ok(()) + } + }); + + *seeked_signal.lock().unwrap() = Some(b.signal::<(String,), _>("Seeked", ("x",)).msg_fn()); + + b.property("PlaybackStatus") + .get({ + let state = state.clone(); + move |_, _| { + let state = state.lock().unwrap(); + Ok(state.get_playback_status().to_string()) + } + }) + .emits_changed_true(); + + b.property("Rate").get(|_, _| Ok(1.0)).emits_changed_true(); + + b.property("Metadata") + .get({ + let state = state.clone(); + move |_, _| Ok(create_metadata_dict(&state.lock().unwrap().metadata)) + }) + .emits_changed_true(); + + b.property("Volume") + .get({ + let state = state.clone(); + move |_, _| { + let state = state.lock().unwrap(); + Ok(state.volume) + } + }) + .set({ + let event_handler = event_handler.clone(); + move |_, _, volume: f64| { + (event_handler.lock().unwrap())(MediaControlEvent::SetVolume(volume)); + Ok(Some(volume)) + } + }) + .emits_changed_true(); + + b.property("Position").get({ + let state = state.clone(); + move |_, _| { + let state = state.lock().unwrap(); + let progress: i64 = match state.playback_status { + MediaPlayback::Playing { + progress: Some(progress), + } + | MediaPlayback::Paused { + progress: Some(progress), + } => progress.0.as_micros(), + _ => 0, + } + .try_into() + .unwrap(); + Ok(progress) + } + }); + + b.property("MinimumRate") + .get(|_, _| Ok(1.0)) + .emits_changed_true(); + b.property("MaximumRate") + .get(|_, _| Ok(1.0)) + .emits_changed_true(); + + b.property("CanGoNext") + .get(|_, _| Ok(true)) + .emits_changed_true(); + b.property("CanGoPrevious") + .get(|_, _| Ok(true)) + .emits_changed_true(); + b.property("CanPlay") + .get(|_, _| Ok(true)) + .emits_changed_true(); + b.property("CanPause") + .get(|_, _| Ok(true)) + .emits_changed_true(); + b.property("CanSeek") + .get(|_, _| Ok(true)) + .emits_changed_true(); + b.property("CanControl") + .get(|_, _| Ok(true)) + .emits_changed_true(); + }); + + cr.insert( + "/org/mpris/MediaPlayer2", + &[app_interface, player_interface], + (), + ); + + seeked_signal.lock().ok(); + + cr +} + +fn register_method( + b: &mut IfaceBuilder<()>, + event_handler: &Arc>, + name: &'static str, + event: MediaControlEvent, +) where + F: Fn(MediaControlEvent) + Send + 'static, +{ + let event_handler = event_handler.clone(); + + b.method(name, (), (), move |_, _, _: ()| { + (event_handler.lock().unwrap())(event.clone()); + Ok(()) + }); +} diff --git a/src/platform/mpris/dbus/mod.rs b/src/platform/mpris/dbus/mod.rs index fa42880..035e38b 100644 --- a/src/platform/mpris/dbus/mod.rs +++ b/src/platform/mpris/dbus/mod.rs @@ -1,17 +1,16 @@ +mod interfaces; + use std::collections::HashMap; use std::convert::From; -use std::convert::{TryFrom, TryInto}; +use std::convert::TryInto; use std::sync::{mpsc, Arc, Mutex}; use std::thread::{self, JoinHandle}; use std::time::Duration; -use crate::{ - MediaControlEvent, MediaMetadata, MediaPlayback, MediaPosition, PlatformConfig, SeekDirection, -}; +use crate::{MediaControlEvent, MediaMetadata, MediaPlayback, PlatformConfig}; /// A platform-specific error. -#[derive(Debug)] -pub struct Error; +pub type Error = dbus::Error; /// A handle to OS media controls. pub struct MediaControls { @@ -112,6 +111,7 @@ impl From> for OwnedMetadata { artist: other.artist.map(|s| s.to_string()), album: other.album.map(|s| s.to_string()), cover_url: other.cover_url.map(|s| s.to_string()), + // TODO: This should probably not have an unwrap duration: other.duration.map(|d| d.as_micros().try_into().unwrap()), } } @@ -196,8 +196,6 @@ use dbus::channel::{MatchingReceiver, Sender}; use dbus::ffidisp::stdintf::org_freedesktop_dbus::PropertiesPropertiesChanged; use dbus::message::SignalArgs; use dbus::Path; -use dbus_crossroads::Crossroads; -use dbus_crossroads::IfaceBuilder; fn run_service( dbus_name: String, @@ -208,217 +206,20 @@ fn run_service( where F: Fn(MediaControlEvent) + Send + 'static, { - let event_handler = Arc::new(Mutex::new(event_handler)); - let seeked_signal = Arc::new(Mutex::new(None)); - let c = Connection::new_session()?; - c.request_name( - format!("org.mpris.MediaPlayer2.{}", dbus_name), - false, - true, - false, - )?; - - let mut cr = Crossroads::new(); - - let app_interface = cr.register("org.mpris.MediaPlayer2", { - let event_handler = event_handler.clone(); - - move |b| { - b.property("Identity") - .get(move |_, _| Ok(friendly_name.clone())); - - register_method(b, &event_handler, "Raise", MediaControlEvent::Raise); - register_method(b, &event_handler, "Quit", MediaControlEvent::Quit); - - // TODO: allow user to set these properties - b.property("CanQuit") - .get(|_, _| Ok(true)) - .emits_changed_true(); - b.property("CanRaise") - .get(|_, _| Ok(true)) - .emits_changed_true(); - b.property("HasTracklist") - .get(|_, _| Ok(false)) - .emits_changed_true(); - b.property("SupportedUriSchemes") - .get(move |_, _| Ok(&[] as &[String])) - .emits_changed_true(); - b.property("SupportedMimeTypes") - .get(move |_, _| Ok(&[] as &[String])) - .emits_changed_true(); - } - }); + let name = format!("org.mpris.MediaPlayer2.{}", dbus_name); + c.request_name(name, false, true, false)?; - let mut state = ServiceState { + let state = Arc::new(Mutex::new(ServiceState { metadata: Default::default(), - metadata_dict: HashMap::new(), + metadata_dict: create_metadata_dict(&Default::default()), playback_status: MediaPlayback::Stopped, volume: 1.0, - }; - - state.set_metadata(Default::default()); - - let state = Arc::new(Mutex::new(state)); - - let player_interface = cr.register("org.mpris.MediaPlayer2.Player", |b| { - register_method(b, &event_handler, "Next", MediaControlEvent::Next); - register_method(b, &event_handler, "Previous", MediaControlEvent::Previous); - register_method(b, &event_handler, "Pause", MediaControlEvent::Pause); - register_method(b, &event_handler, "PlayPause", MediaControlEvent::Toggle); - register_method(b, &event_handler, "Stop", MediaControlEvent::Stop); - register_method(b, &event_handler, "Play", MediaControlEvent::Play); - - b.method("Seek", ("Offset",), (), { - let event_handler = event_handler.clone(); - - move |ctx, _, (offset,): (i64,)| { - let abs_offset = offset.abs() as u64; - let direction = if offset > 0 { - SeekDirection::Forward - } else { - SeekDirection::Backward - }; - - (event_handler.lock().unwrap())(MediaControlEvent::SeekBy( - direction, - Duration::from_micros(abs_offset), - )); - ctx.push_msg(ctx.make_signal("Seeked", ())); - Ok(()) - } - }); - - b.method("SetPosition", ("TrackId", "Position"), (), { - let state = state.clone(); - let event_handler = event_handler.clone(); - - move |_, _, (_trackid, position): (Path, i64)| { - let state = state.lock().unwrap(); - - // According to the MPRIS specification: - - // TODO: If the TrackId argument is not the same as the current - // trackid, the call is ignored as stale. - // (Maybe it should be optional?) - - if let Some(duration) = state.metadata.duration { - // If the Position argument is greater than the track length, do nothing. - if position > duration { - return Ok(()); - } - } - - // If the Position argument is less than 0, do nothing. - if let Ok(position) = u64::try_from(position) { - let position = Duration::from_micros(position); - - (event_handler.lock().unwrap())(MediaControlEvent::SetPosition(MediaPosition( - position, - ))); - } - Ok(()) - } - }); - - b.method("OpenUri", ("Uri",), (), { - let event_handler = event_handler.clone(); - - move |_, _, (uri,): (String,)| { - (event_handler.lock().unwrap())(MediaControlEvent::OpenUri(uri)); - Ok(()) - } - }); - - *seeked_signal.lock().unwrap() = Some(b.signal::<(String,), _>("Seeked", ("x",)).msg_fn()); - - b.property("PlaybackStatus") - .get({ - let state = state.clone(); - move |_, _| { - let state = state.lock().unwrap(); - Ok(state.get_playback_status().to_string()) - } - }) - .emits_changed_true(); - - b.property("Rate").get(|_, _| Ok(1.0)).emits_changed_true(); - - b.property("Metadata") - .get({ - let state = state.clone(); - move |_, _| Ok(create_metadata_dict(&state.lock().unwrap().metadata)) - }) - .emits_changed_true(); - - b.property("Volume") - .get({ - let state = state.clone(); - move |_, _| { - let state = state.lock().unwrap(); - Ok(state.volume) - } - }) - .set({ - let event_handler = event_handler.clone(); - move |_, _, volume: f64| { - (event_handler.lock().unwrap())(MediaControlEvent::SetVolume(volume)); - Ok(Some(volume)) - } - }) - .emits_changed_true(); - - b.property("Position").get({ - let state = state.clone(); - move |_, _| { - let state = state.lock().unwrap(); - let progress: i64 = match state.playback_status { - MediaPlayback::Playing { - progress: Some(progress), - } - | MediaPlayback::Paused { - progress: Some(progress), - } => progress.0.as_micros(), - _ => 0, - } - .try_into() - .unwrap(); - Ok(progress) - } - }); + })); + let event_handler = Arc::new(Mutex::new(event_handler)); + let seeked_signal = Arc::new(Mutex::new(None)); - b.property("MinimumRate") - .get(|_, _| Ok(1.0)) - .emits_changed_true(); - b.property("MaximumRate") - .get(|_, _| Ok(1.0)) - .emits_changed_true(); - - b.property("CanGoNext") - .get(|_, _| Ok(true)) - .emits_changed_true(); - b.property("CanGoPrevious") - .get(|_, _| Ok(true)) - .emits_changed_true(); - b.property("CanPlay") - .get(|_, _| Ok(true)) - .emits_changed_true(); - b.property("CanPause") - .get(|_, _| Ok(true)) - .emits_changed_true(); - b.property("CanSeek") - .get(|_, _| Ok(true)) - .emits_changed_true(); - b.property("CanControl") - .get(|_, _| Ok(true)) - .emits_changed_true(); - }); - - cr.insert( - "/org/mpris/MediaPlayer2", - &[app_interface, player_interface], - (), - ); + let mut cr = interfaces::register_methods(&state, &event_handler, friendly_name, seeked_signal); c.start_receive( dbus::message::MatchRule::new_method_call(), @@ -470,26 +271,10 @@ where c.send( properties_changed.to_emit_message(&Path::new("/org/mpris/MediaPlayer2").unwrap()), ) - .unwrap(); + .ok(); } c.process(Duration::from_millis(1000))?; } Ok(()) } - -fn register_method( - b: &mut IfaceBuilder<()>, - event_handler: &Arc>, - name: &'static str, - event: MediaControlEvent, -) where - F: Fn(MediaControlEvent) + Send + 'static, -{ - let event_handler = event_handler.clone(); - - b.method(name, (), (), move |_, _, _: ()| { - (event_handler.lock().unwrap())(event.clone()); - Ok(()) - }); -} From 704e8126bb734e464c9601f8963ff046c10c5317 Mon Sep 17 00:00:00 2001 From: Sinono3 Date: Mon, 1 Jan 2024 22:23:32 -0300 Subject: [PATCH 5/6] Add playerctl test script --- tests/playerctl_script.sh | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100755 tests/playerctl_script.sh diff --git a/tests/playerctl_script.sh b/tests/playerctl_script.sh new file mode 100755 index 0000000..e7a93e0 --- /dev/null +++ b/tests/playerctl_script.sh @@ -0,0 +1,28 @@ +#!/bin/sh +# This scripts requires playerctl and dbus-send + +alias playerctl="playerctl -p my_player " + +playerctl metadata +playerctl play +playerctl pause +playerctl play-pause +playerctl next +playerctl previous +playerctl stop +playerctl position 30 +playerctl position 10- +playerctl position 10+ +playerctl volume 0.5 +playerctl open "https://testlink.com" +# TODO: Shuffle and repeat. +# playerctl shuffle +# playerctl repeat + +# The following are commands not supported by playerctl, thus we use dbus-send +call() { + dbus-send --dest=org.mpris.MediaPlayer2.my_player --print-reply /org/mpris/MediaPlayer2 "$1" +} + +call org.mpris.MediaPlayer2.Raise +call org.mpris.MediaPlayer2.Quit From dc01a9d9ce9d96a47c7e3aa2bccc2b8111fd6bf7 Mon Sep 17 00:00:00 2001 From: Sinono3 Date: Mon, 1 Jan 2024 23:16:58 -0300 Subject: [PATCH 6/6] Implement better error handling for D-Bus backend - Check if the D-Bus connection has been succesfully created before launching the thread. This way we can handle the error before anything occurs. - Check if the thread isn't running or has panicked at every method call of `MediaControls`. --- Cargo.toml | 1 + src/platform/mpris/dbus/mod.rs | 80 ++++++++++++++++++++-------------- 2 files changed, 48 insertions(+), 33 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6cc3de2..fbcfda9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ dbus-crossroads = { version = "0.5.0", optional = true } zbus = { version = "3.9", optional = true } zvariant = { version = "3.10", optional = true } pollster = { version = "0.3", optional = true } +thiserror = "1.0" [features] default = ["use_dbus"] diff --git a/src/platform/mpris/dbus/mod.rs b/src/platform/mpris/dbus/mod.rs index 035e38b..66c18be 100644 --- a/src/platform/mpris/dbus/mod.rs +++ b/src/platform/mpris/dbus/mod.rs @@ -1,5 +1,11 @@ mod interfaces; +use dbus::arg::{RefArg, Variant}; +use dbus::blocking::Connection; +use dbus::channel::{MatchingReceiver, Sender}; +use dbus::ffidisp::stdintf::org_freedesktop_dbus::PropertiesPropertiesChanged; +use dbus::message::SignalArgs; +use dbus::Path; use std::collections::HashMap; use std::convert::From; use std::convert::TryInto; @@ -10,7 +16,19 @@ use std::time::Duration; use crate::{MediaControlEvent, MediaMetadata, MediaPlayback, PlatformConfig}; /// A platform-specific error. -pub type Error = dbus::Error; +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error("internal D-Bus error: {0}")] + DbusError(#[from] dbus::Error), + #[error("D-bus service thread not running. Run MediaControls::attach()")] + ThreadNotRunning, + // NOTE: For now this error is not very descriptive. For now we can't do much about it + // since the panic message returned by JoinHandle::join does not implement Debug/Display, + // thus we cannot print it, though perhaps there is another way. I will leave this error here, + // to at least be able to catch it, but it is preferable to have this thread *not panic* at all. + #[error("D-Bus service thread panicked")] + ThreadPanicked, +} /// A handle to OS media controls. pub struct MediaControls { @@ -21,7 +39,7 @@ pub struct MediaControls { struct ServiceThreadHandle { event_channel: mpsc::Sender, - thread: JoinHandle<()>, + thread: JoinHandle>, } #[derive(Clone, PartialEq, Debug)] @@ -144,14 +162,18 @@ impl MediaControls { let friendly_name = self.friendly_name.clone(); let (event_channel, rx) = mpsc::channel(); + // Check if the connection can be created BEFORE spawning the new thread + let conn = Connection::new_session()?; + let name = format!("org.mpris.MediaPlayer2.{}", dbus_name); + conn.request_name(name, false, true, false)?; + self.thread = Some(ServiceThreadHandle { event_channel, - thread: thread::spawn(move || { - run_service(dbus_name, friendly_name, event_handler, rx).unwrap() - }), + thread: thread::spawn(move || run_service(conn, friendly_name, event_handler, rx)), }); Ok(()) } + /// Detach the event handler. pub fn detach(&mut self) -> Result<(), Error> { if let Some(ServiceThreadHandle { @@ -159,57 +181,49 @@ impl MediaControls { thread, }) = self.thread.take() { - event_channel.send(InternalEvent::Kill).unwrap(); - thread.join().unwrap(); + // We don't care about the result of this event, since we immedieately + // check if the thread has panicked on the next line. + event_channel.send(InternalEvent::Kill).ok(); + // One error in case the thread panics, and the other one in case the + // thread has returned an error. + thread.join().map_err(|_| Error::ThreadPanicked)??; } Ok(()) } /// Set the current playback status. pub fn set_playback(&mut self, playback: MediaPlayback) -> Result<(), Error> { - self.send_internal_event(InternalEvent::ChangePlayback(playback)); - Ok(()) + self.send_internal_event(InternalEvent::ChangePlayback(playback)) } /// Set the metadata of the currently playing media item. pub fn set_metadata(&mut self, metadata: MediaMetadata) -> Result<(), Error> { - self.send_internal_event(InternalEvent::ChangeMetadata(metadata.into())); - Ok(()) + self.send_internal_event(InternalEvent::ChangeMetadata(metadata.into())) } /// Set the volume level (0.0-1.0) (Only available on MPRIS) pub fn set_volume(&mut self, volume: f64) -> Result<(), Error> { - self.send_internal_event(InternalEvent::ChangeVolume(volume)); - Ok(()) + self.send_internal_event(InternalEvent::ChangeVolume(volume)) } - // TODO: result - fn send_internal_event(&mut self, event: InternalEvent) { - let channel = &self.thread.as_ref().unwrap().event_channel; - channel.send(event).unwrap(); + fn send_internal_event(&mut self, event: InternalEvent) -> Result<(), Error> { + let thread = &self.thread.as_ref().ok_or(Error::ThreadNotRunning)?; + thread + .event_channel + .send(event) + .map_err(|_| Error::ThreadPanicked) } } -use dbus::arg::{RefArg, Variant}; -use dbus::blocking::Connection; -use dbus::channel::{MatchingReceiver, Sender}; -use dbus::ffidisp::stdintf::org_freedesktop_dbus::PropertiesPropertiesChanged; -use dbus::message::SignalArgs; -use dbus::Path; - fn run_service( - dbus_name: String, + conn: Connection, friendly_name: String, event_handler: F, event_channel: mpsc::Receiver, -) -> Result<(), dbus::Error> +) -> Result<(), Error> where F: Fn(MediaControlEvent) + Send + 'static, { - let c = Connection::new_session()?; - let name = format!("org.mpris.MediaPlayer2.{}", dbus_name); - c.request_name(name, false, true, false)?; - let state = Arc::new(Mutex::new(ServiceState { metadata: Default::default(), metadata_dict: create_metadata_dict(&Default::default()), @@ -221,7 +235,7 @@ where let mut cr = interfaces::register_methods(&state, &event_handler, friendly_name, seeked_signal); - c.start_receive( + conn.start_receive( dbus::message::MatchRule::new_method_call(), Box::new(move |msg, conn| { cr.handle_message(msg, conn).unwrap(); @@ -268,12 +282,12 @@ where invalidated_properties: Vec::new(), }; - c.send( + conn.send( properties_changed.to_emit_message(&Path::new("/org/mpris/MediaPlayer2").unwrap()), ) .ok(); } - c.process(Duration::from_millis(1000))?; + conn.process(Duration::from_millis(1000))?; } Ok(())