hrkfdn / ncspot

Cross-platform ncurses Spotify client written in Rust, inspired by ncmpc and the likes.
BSD 2-Clause "Simplified" License
5.02k stars 209 forks source link

Add support for `Seeked` signal on mpris interface #1492

Closed elParaguayo closed 1 month ago

elParaguayo commented 2 months ago

Is your feature request related to a problem? Please describe. According to the mpris2 spec, a player should provide the Seeked signal to show when the playback position has changed due to seek activity.

Describe the solution you'd like I would like support for this signal to be added to ncspot's mpris interface.

Describe alternatives you've considered n/a

Additional context n/a

Sorry this is short - I hope the request is self-explanatory. Thanks for your work on ncspot, it's greatly appreciated.

elParaguayo commented 2 months ago

I'm a complete beginner when it comes to Rust but I've had a go at implementing this. This is what I've got so far:

diff --git a/src/mpris.rs b/src/mpris.rs
index 4a66ac4..4b7d194 100644
--- a/src/mpris.rs
+++ b/src/mpris.rs
@@ -8,6 +8,7 @@ use tokio_stream::wrappers::UnboundedReceiverStream;
 use tokio_stream::StreamExt;
 use zbus::zvariant::{ObjectPath, Value};
 use zbus::{interface, ConnectionBuilder};
+use zbus::object_server::SignalContext;

 use crate::application::ASYNC_RUNTIME;
 use crate::library::Library;
@@ -314,6 +315,9 @@ impl MprisPlayer {
         self.queue.get_current().is_some()
     }

+    #[zbus(signal)]
+    async fn seeked(context: &SignalContext<'_>, position: &i64) -> Result<(), zbus::Error>;
+
     fn next(&self) {
         self.queue.next(true)
     }
@@ -468,6 +472,8 @@ pub enum MprisCommand {
     NotifyPlaybackUpdate,
     /// Notify about volume updates.
     NotifyVolumeUpdate,
+    /// Notify about seek changes,
+    NotifySeekedUpdate,
 }

 /// An MPRIS server that internally manager a thread which can be sent commands. This is internally
@@ -533,6 +539,11 @@ impl MprisManager {
                     info!("sending MPRIS volume update signal");
                     player_iface.volume_changed(ctx).await?;
                 }
+                Some(MprisCommand::NotifySeekedUpdate) => {
+                    let pos = player_iface.position();
+                    info!("sending MPRIS seeked signal");
+                    MprisPlayer::seeked(ctx, &pos).await?;
+                }
                 None => break,
             }
         }
diff --git a/src/spotify.rs b/src/spotify.rs
index 36d754e..594e499 100644
--- a/src/spotify.rs
+++ b/src/spotify.rs
@@ -394,6 +394,11 @@ impl Spotify {
     /// Seek in the currently played [Playable] played by the [Player].
     pub fn seek(&self, position_ms: u32) {
         self.send_worker(WorkerCommand::Seek(position_ms));
+        #[cfg(feature = "mpris")]
+        if let Some(mpris_manager) = self.mpris.lock().unwrap().as_ref() {
+            info!("Seeked event");
+            mpris_manager.send(MprisCommand::NotifySeekedUpdate);
+        }
     }

     /// Seek relatively to the current playback position of the [Player].

While this does indeed send the signal, the issue is that the position reported by the signal is the position before the seek has completed. I would assume this is because the signal is fired before spotify has been able to implement the seek command.

Is there a way to fire the signal once that seek command has been implemented or pass the new position value with the event that's handled by mpris_manager?