todo mpris rethink

src/mpris/player.rs

@@ -1,8 +1,10 @@
-use glib::SendWeakRef;
+use gtk::glib::spawn_future_local;
-use gtk::glib;
 use std::collections::HashMap;
-use zbus::object_server::SignalEmitter;
+use std::rc::{Rc, Weak};
-use zbus::zvariant::{ObjectPath, OwnedObjectPath, Value};
+use zbus::zvariant::{ObjectPath, OwnedObjectPath, OwnedValue, Value};
+
+use crate::playbin::Song as PlaybinSong;
+type Playbin = crate::playbin2::Playbin<PlaybinSong>;
 
 const MICROSECONDS: f64 = 1e6; // in a second
 
@@ -57,70 +59,91 @@ impl MetadataMap {
         .unwrap_or_default()
     }
 
-    fn as_hash_map(&self) -> HashMap<&'static str, Value> {
+    fn as_hash_map(&self) -> HashMap<&'static str, OwnedValue> {
         let mut map = HashMap::new();
 
         if let Some(track_id) = &self.track_id {
-            map.insert("mpris:trackid", Value::new(track_id.as_ref()));
+            map.insert(
+                "mpris:trackid",
+                Value::new(track_id.as_ref()).try_into().unwrap(),
+            );
         }
         if let Some(art_url) = &self.art_url {
-            map.insert("mpris:artUrl", Value::new(art_url.to_string()));
+            map.insert(
+                "mpris:artUrl",
+                Value::new(art_url.to_string()).try_into().unwrap(),
+            );
         }
         if let Some(length) = &self.length {
-            map.insert("mpris:length", Value::new(length));
+            map.insert("mpris:length", Value::new(length).try_into().unwrap());
         }
         if let Some(album) = &self.album {
-            map.insert("xesam:album", Value::new(album));
+            map.insert("xesam:album", Value::new(album).try_into().unwrap());
         }
         if let Some(artist) = &self.artist {
-            map.insert("xesam:artist", Value::new(artist));
+            map.insert("xesam:artist", Value::new(artist).try_into().unwrap());
         }
         if let Some(content_created) = &self.content_created {
             map.insert(
                 "xesam:contentCreated",
-                Value::new(content_created.format("%+").to_string()),
+                Value::new(content_created.format("%+").to_string())
+                    .try_into()
+                    .unwrap(),
             );
         }
         if let Some(genre) = &self.genre {
-            map.insert("xesam:genre", Value::new(genre));
+            map.insert("xesam:genre", Value::new(genre).try_into().unwrap());
         }
         if let Some(track_number) = self.track_number {
-            map.insert("xesam:trackNumber", Value::new(track_number));
+            map.insert(
+                "xesam:trackNumber",
+                Value::new(track_number).try_into().unwrap(),
+            );
         }
         if let Some(title) = &self.title {
-            map.insert("xesam:title", Value::new(title));
+            map.insert("xesam:title", Value::new(title).try_into().unwrap());
         }
         if let Some(user_rating) = self.user_rating {
-            map.insert("xesam:userRating", Value::new(user_rating));
+            map.insert(
+                "xesam:userRating",
+                Value::new(user_rating).try_into().unwrap(),
+            );
         }
 
         map
     }
 }
 
-pub struct Player {
+pub struct Player(async_channel::Sender<Box<dyn FnOnce(Rc<LocalPlayer>) + Send>>);
-    playbin: SendWeakRef<crate::Playbin>,
-    metadata: MetadataMap,
-}
 
 impl Player {
     pub async fn setup(
         object_server: &zbus::ObjectServer,
-        playbin: &crate::Playbin,
+        playbin: &Rc<Playbin>,
     ) -> Result<(), zbus::Error> {
-        use adw::prelude::*;
+        let local = LocalPlayer {
-
-        let player = Self {
-            playbin: playbin.downgrade().into(),
             metadata: MetadataMap::from_playbin_song(None),
+            playbin: Rc::downgrade(playbin),
         };
 
+        let (with_local_send, with_local_recv) = async_channel::unbounded();
+
+        let player = Self(with_local_send);
+
+        spawn_future_local(async move {
+            let local = Rc::new(local);
+            while let Ok(f) = with_local_recv.recv().await {
+                f(Rc::clone(&local));
+            }
+        });
+
         object_server.at("/org/mpris/MediaPlayer2", player).await?;
 
-        let player_ref = object_server
+        let _player_ref = object_server
             .interface::<_, Self>("/org/mpris/MediaPlayer2")
             .await?;
 
+        /*
         playbin.connect_new_track(glib::clone!(
             #[strong]
             player_ref,
@@ -216,182 +239,46 @@ impl Player {
                 }
             ),
         );
+        */
 
         Ok(())
     }
 
-    fn playbin(&self) -> zbus::fdo::Result<crate::Playbin> {
+    async fn with_local<T: Send + 'static, F: std::future::Future<Output = T>>(
-        match self.playbin.upgrade() {
+        &self,
-            None => Err(zbus::fdo::Error::Failed("playbin was discarded".into())),
+        f: impl FnOnce(Rc<LocalPlayer>) -> F + Send + 'static,
-            Some(playbin) => Ok(playbin),
+    ) -> T {
-        }
+        let (send, recv) = async_channel::bounded(1);
+
+        self.0
+            .send(Box::new(move |local| {
+                gtk::glib::spawn_future_local(async move {
+                    send.send(f(local).await).await.unwrap();
+                });
+            }))
+            .await
+            .unwrap();
+        recv.recv().await.unwrap()
     }
 }
 
+// because zbus insists in being Send+Sync all proper
+struct LocalPlayer {
+    metadata: MetadataMap,
+    playbin: Weak<Playbin>,
+}
+
 #[zbus::interface(name = "org.mpris.MediaPlayer2.Player")]
 impl Player {
-    fn next(&self) -> zbus::fdo::Result<()> {
+    async fn next(&self) -> zbus::fdo::Result<()> {
-        // If CanGoNext is false, attempting to call this method should have no effect.
+        self.with_local(move |local| async move { local.next() })
-        if !self.can_go_next()? {
+            .await
-            return Ok(());
     }
 
-        let playbin = self.playbin()?;
-        if playbin.play_queue_position() + 1 > playbin.play_queue_length() {
-            // If there is no next track (and endless playback and track repeat are both off), stop playback.
-            // (interpret this as something else than what Stop does)
-            playbin.stop();
-        } else {
-            playbin.go_to_next_track();
-        }
-
-        Ok(())
-    }
-
-    fn previous(&self) -> zbus::fdo::Result<()> {
-        let playbin = self.playbin()?;
-
-        // If CanGoPrevious is false, attempting to call this method should have no effect.
-        if !self.can_go_previous()? {
-            return Ok(());
-        }
-
-        if playbin.play_queue_position() == 0 {
-            // If there is no previous track (and endless playback and track repeat are both off), stop playback.
-            // (interpret this as something else than what Stop does)
-            playbin.stop();
-        } else {
-            playbin.go_to_prev_track();
-        }
-
-        Ok(())
-    }
-
-    fn pause(&self) -> zbus::fdo::Result<()> {
-        let playbin = self.playbin()?;
-
-        // If CanPause is false, attempting to call this method should have no effect.
-        if !self.can_pause() {
-            return Ok(());
-        }
-
-        // If playback is already paused, this has no effect.
-        if playbin.state() != crate::playbin::State::Playing {
-            return Ok(());
-        }
-
-        playbin.pause();
-        Ok(())
-    }
-
-    fn play_pause(&self) -> zbus::fdo::Result<()> {
-        // don't think this is exactly according to spec but it looks more reasonable to me
-        if self.playbin()?.state() == crate::playbin::State::Paused {
-            self.play()
-        } else {
-            self.pause()
-        }
-    }
-
-    fn stop(&self) -> zbus::fdo::Result<()> {
-        let playbin = self.playbin()?;
-
-        // If playback is already stopped, this has no effect.
-        if playbin.state() != crate::playbin::State::Playing {
-            return Ok(());
-        }
-
-        // Calling Play after this should cause playback to start again from the beginning of the track.
-        playbin.pause();
-        playbin.seek(0.0);
-        Ok(())
-    }
-
-    fn play(&self) -> zbus::fdo::Result<()> {
-        let playbin = self.playbin()?;
-
-        // If CanPlay is false, attempting to call this method should have no effect.
-        if !self.can_play()? {
-            return Ok(());
-        }
-
-        // If already playing, this has no effect.
-        if playbin.state() == crate::playbin::State::Playing {
-            return Ok(());
-        }
-
-        // If there is no track to play, this has no effect.
-        if playbin.play_queue_length() == 0 {
-            return Ok(());
-        }
-
-        playbin.play();
-        Ok(())
-    }
-
-    fn seek(&self, offset: i64) -> zbus::fdo::Result<()> {
-        // If the CanSeek property is false, this has no effect.
-        if !self.can_seek() {
-            return Ok(());
-        }
-
-        let playbin = self.playbin()?;
-        // Seeks forward in the current track by the specified number of microseconds.
-        let mut new_position = (playbin.position() * MICROSECONDS) as i64 + offset;
-
-        // A negative value seeks back. If this would mean seeking back further than the start of the track, the position is set to 0.
-        if new_position < 0 {
-            new_position = 0;
-        }
-
-        // If the value passed in would mean seeking beyond the end of the track, acts like a call to Next.
-        if new_position >= (playbin.duration() * MICROSECONDS) as i64 {
-            return self.next();
-        }
-
-        playbin.seek(new_position as f64 / MICROSECONDS);
-        Ok(())
-    }
-
-    fn set_position(&self, track_id: ObjectPath<'_>, position: i64) -> zbus::fdo::Result<()> {
-        let playbin = self.playbin()?;
-
-        // If the Position argument is less than 0, do nothing.
-        if position < 0 {
-            return Ok(());
-        }
-        // If the Position argument is greater than the track length, do nothing.
-        if position > (playbin.duration() * MICROSECONDS) as i64 {
-            return Ok(());
-        }
-        // If the CanSeek property is false, this has no effect.
-        if !self.can_seek() {
-            return Ok(());
-        }
-        // check if it's stale
-        if self.metadata.track_id.as_deref() != Some(&track_id) {
-            // TODO: warn of stale seek
-            return Ok(());
-        }
-
-        playbin.seek(position as f64 / MICROSECONDS);
-        Ok(())
-    }
-
-    fn open_uri(&self, _s: &str) -> zbus::fdo::Result<()> {
-        Err(zbus::fdo::Error::NotSupported("OpenUri".into()))
-    }
-
-    #[zbus(signal)]
-    async fn seeked(signal_emitter: &SignalEmitter<'_>, position: i64) -> zbus::Result<()>;
-
     #[zbus(property)]
-    fn playback_status(&self) -> zbus::fdo::Result<&str> {
+    async fn playback_status(&self) -> zbus::fdo::Result<String> {
-        match self.playbin()?.state() {
+        self.with_local(|local| async move { local.playback_status() })
-            crate::playbin::State::Stopped => Ok("Stopped"),
+            .await
-            crate::playbin::State::Playing => Ok("Playing"),
-            crate::playbin::State::Paused => Ok("Paused"),
-        }
     }
 
     #[zbus(property)]
@@ -410,10 +297,11 @@ impl Player {
     }
 
     #[zbus(property)]
-    fn set_rate(&self, rate: f64) -> zbus::Result<()> {
+    async fn set_rate(&self, rate: f64) -> zbus::Result<()> {
         // A value of 0.0 should not be set by the client. If it is, the media player should act as though Pause was called.
         if rate == 0.0 {
-            self.pause()?;
+            self.with_local(|local| async move { local.pause() })
+                .await?;
         }
 
         // just ignore anything else
@@ -421,42 +309,39 @@ impl Player {
     }
 
     #[zbus(property)]
-    // FIXME: zbus bug (?): this getter can't be infallible
+    // FIXME: https://github.com/dbus2/zbus/issues/992
     fn shuffle(&self) -> zbus::fdo::Result<bool> {
         Ok(false)
     }
 
     #[zbus(property)]
-    // FIXME: zbus bug (?): this setter can't return zbus::fdo::Result
+    // FIXME: see above
     fn set_shuffle(&self, _shuffle: bool) -> zbus::Result<()> {
         Err(zbus::fdo::Error::NotSupported("setting Shuffle".into()).into())
     }
 
     #[zbus(property)]
-    fn metadata(&self) -> HashMap<&'static str, Value> {
+    async fn metadata(&self) -> zbus::fdo::Result<HashMap<&'static str, OwnedValue>> {
-        self.metadata.as_hash_map()
+        self.with_local(move |local| async move { Ok(local.metadata.as_hash_map()) })
+            .await
     }
 
     #[zbus(property)]
-    fn volume(&self) -> zbus::fdo::Result<f64> {
+    async fn volume(&self) -> zbus::fdo::Result<f64> {
-        Ok(self.playbin()?.volume() as f64 / 100.0)
+        self.with_local(|local| async move { local.volume() }).await
     }
 
     #[zbus(property)]
-    fn set_volume(&mut self, mut volume: f64) -> zbus::fdo::Result<()> {
+    async fn set_volume(&self, volume: f64) -> zbus::Result<()> {
-        // When setting, if a negative value is passed, the volume should be set to 0.0.
+        self.with_local(move |local| async move { local.set_volume(volume) })
-        if volume < 0.0 {
+            .await?;
-            volume = 0.0;
-        }
-        let playbin = self.playbin()?;
-        // FIXME: check if this is set by the notify callback: self.volume = volume;
-        playbin.set_volume((volume * 100.0) as i32);
         Ok(())
     }
 
     #[zbus(property(emits_changed_signal = "false"))]
-    fn position(&self) -> zbus::fdo::Result<i64> {
+    async fn position(&self) -> zbus::fdo::Result<i64> {
-        Ok((self.playbin()?.position() * MICROSECONDS) as i64)
+        self.with_local(|local| async move { local.position() })
+            .await
     }
 
     #[zbus(property)]
@@ -470,36 +355,186 @@ impl Player {
     }
 
     #[zbus(property)]
+    async fn can_go_next(&self) -> zbus::fdo::Result<bool> {
+        self.with_local(|local| async move { local.can_go_next() })
+            .await
+    }
+
+    #[zbus(property)]
+    async fn can_go_previous(&self) -> zbus::fdo::Result<bool> {
+        self.with_local(|local| async move { local.can_go_previous() })
+            .await
+    }
+
+    #[zbus(property)]
+    async fn can_play(&self) -> zbus::fdo::Result<bool> {
+        self.with_local(|local| async move { local.can_play() })
+            .await
+    }
+
+    #[zbus(property)]
+    async fn can_pause(&self) -> bool {
+        self.with_local(|local| async move { local.can_pause() })
+            .await
+    }
+
+    #[zbus(property)]
+    async fn can_seek(&self) -> bool {
+        self.with_local(|local| async move { local.can_seek() })
+            .await
+    }
+
+    #[zbus(property(emits_changed_signal = "const"))]
+    async fn can_control(&self) -> bool {
+        true
+    }
+}
+
+impl LocalPlayer {
+    fn playbin(&self) -> zbus::fdo::Result<Rc<Playbin>> {
+        match self.playbin.upgrade() {
+            None => Err(zbus::fdo::Error::Failed("playbin was discarded".into())),
+            Some(playbin) => Ok(playbin),
+        }
+    }
+
+    fn next(&self) -> zbus::fdo::Result<()> {
+        // If CanGoNext is false, attempting to call this method should have no effect.
+        if !self.can_go_next()? {
+            return Ok(());
+        }
+
+        let playbin = self.playbin()?;
+        if playbin.current_entry().is_none()
+            || (playbin.current_entry().unwrap() + 1 > playbin.entries().len())
+        {
+            // If there is no next track (and endless playback and track repeat are both off), stop playback.
+            // (interpret this as something else than what Stop does)
+            todo!();
+        } else {
+            playbin.next_entry();
+        }
+
+        Ok(())
+    }
+
+    fn previous(&self) -> zbus::fdo::Result<()> {
+        let playbin = self.playbin()?;
+
+        // If CanGoPrevious is false, attempting to call this method should have no effect.
+        if !self.can_go_previous()? {
+            return Ok(());
+        }
+
+        match playbin.current_entry() {
+            None | Some(0) => {
+                // If there is no previous track (and endless playback and track repeat are both off), stop playback.
+                // (interpret this as something else than what Stop does)
+                playbin.stop();
+            }
+            _ => {
+                playbin.prev_entry();
+            }
+        }
+
+        Ok(())
+    }
+
+    fn pause(&self) -> zbus::fdo::Result<()> {
+        let playbin = self.playbin()?;
+
+        // If CanPause is false, attempting to call this method should have no effect.
+        if !self.can_pause() {
+            return Ok(());
+        }
+
+        // If playback is already paused, this has no effect.
+        if playbin.paused() {
+            return Ok(());
+        }
+
+        playbin.set_paused(true);
+        Ok(())
+    }
+
+    fn play_pause(&self) -> zbus::fdo::Result<()> {
+        let playbin = self.playbin()?;
+        playbin.set_paused(!playbin.paused());
+        Ok(())
+    }
+
+    fn stop(&self) -> zbus::fdo::Result<()> {
+        todo!()
+    }
+
+    fn play(&self) -> zbus::fdo::Result<()> {
+        todo!()
+    }
+
+    fn seek(&self, _offset: i64) -> zbus::fdo::Result<()> {
+        todo!()
+    }
+
+    fn set_position(&self, _track_id: ObjectPath<'_>, _position: i64) -> zbus::fdo::Result<()> {
+        todo!()
+    }
+
+    fn open_uri(&self, _s: &str) -> zbus::fdo::Result<()> {
+        Err(zbus::fdo::Error::NotSupported("OpenUri".into()))
+    }
+
+    fn playback_status(&self) -> zbus::fdo::Result<String> {
+        match self.playbin()?.paused() {
+            //crate::playbin::State::Stopped => Ok("Stopped".into()),
+            false => Ok("Playing".into()),
+            true => Ok("Paused".into()),
+        }
+    }
+
+    fn volume(&self) -> zbus::fdo::Result<f64> {
+        Ok(self.playbin()?.volume() as f64 / 100.0)
+    }
+
+    fn set_volume(&self, mut volume: f64) -> zbus::fdo::Result<()> {
+        // When setting, if a negative value is passed, the volume should be set to 0.0.
+        if volume < 0.0 {
+            volume = 0.0;
+        }
+        let playbin = self.playbin()?;
+        // FIXME: check if this is set by the notify callback: self.volume = volume;
+        playbin.set_volume((volume * 100.0) as i64);
+        Ok(())
+    }
+
+    fn position(&self) -> zbus::fdo::Result<i64> {
+        Ok(0) // TODO
+    }
+
     fn can_go_next(&self) -> zbus::fdo::Result<bool> {
         // same as can_play
-        Ok(self.playbin()?.play_queue_length() > 0)
+        Ok(self.playbin()?.entries().len() > 0)
     }
 
-    #[zbus(property)]
     fn can_go_previous(&self) -> zbus::fdo::Result<bool> {
         // same as can_play
-        Ok(self.playbin()?.play_queue_length() > 0)
+        Ok(self.playbin()?.entries().len() > 0)
     }
 
-    #[zbus(property)]
     fn can_play(&self) -> zbus::fdo::Result<bool> {
         // it only makes sense to disallow "play" when the play queue is empty
-        Ok(self.playbin()?.play_queue_length() > 0)
+        Ok(self.playbin()?.entries().len() > 0)
     }
 
-    #[zbus(property)]
     fn can_pause(&self) -> bool {
         // we don't play anything that can't be paused
         true
     }
 
-    #[zbus(property)]
     fn can_seek(&self) -> bool {
         // we don't play anything that can't be seeked
         true
     }
 
-    #[zbus(property(emits_changed_signal = "const"))]
     fn can_control(&self) -> bool {
         true
     }

src/playbin2.rs

@@ -24,9 +24,9 @@ pub struct Playbin<E> {
     paused_changed: SignalEmitter<Self, ()>,
     current_entry_changed: SignalEmitter<Self, ()>,
 
-    entry_inserted: SignalEmitter<Self, u32>,
+    entry_inserted: SignalEmitter<Self, usize>,
     stopped: SignalEmitter<Self, ()>,
-    entry_removed: SignalEmitter<Self, u32>,
+    entry_removed: SignalEmitter<Self, usize>,
 
     file_started: SignalEmitter<Self, ()>,
 }
@@ -98,7 +98,7 @@ where
         todo!()
     }
 
-    pub fn current_entry(&self) -> Option<u32> {
+    pub fn current_entry(&self) -> Option<usize> {
         self.mpv
             .get_property::<i64>("playlist-pos")
             .unwrap()
@@ -118,7 +118,7 @@ where
         self.mpv.command(["playlist-prev"]).unwrap();
     }
 
-    pub fn play_entry(&self, index: u32) {
+    pub fn play_entry(&self, index: usize) {
         self.mpv
             .command(["playlist-play-index", &index.to_string()])
             .unwrap();
@@ -137,10 +137,10 @@ where
         entries.push(entry);
 
         drop(entries);
-        self.entry_inserted.emit(self, index as u32);
+        self.entry_inserted.emit(self, index as usize);
     }
 
-    pub fn insert_entry(&self, index: u32, entry: E) {
+    pub fn insert_entry(&self, index: usize, entry: E) {
         let mut entries = self.entries.borrow_mut();
         self.mpv
             .command(["loadfile", entry.url().as_str(), "insert-at-play"])
@@ -161,7 +161,7 @@ where
         self.stopped.emit(self, ());
     }
 
-    pub fn remove_entry(&self, index: u32) {
+    pub fn remove_entry(&self, index: usize) {
         let mut entries = self.entries.borrow_mut();
         self.mpv.command(["remove", &index.to_string()]).unwrap();
         entries.remove(index as usize);
@@ -170,7 +170,7 @@ where
         self.entry_removed.emit(self, index);
     }
 
-    pub fn move_entry(&self, _from: u32, _to: u32) {
+    pub fn move_entry(&self, _from: usize, _to: usize) {
         todo!()
     }
 

src/ui/window.rs

@@ -96,7 +96,7 @@ mod imp {
                 crate::Mpris::setup(conn.object_server(), &window)
                     .await
                     .expect("could not serve mpris");
-                crate::mpris::Player::setup(conn.object_server(), &window.playbin())
+                crate::mpris::Player::setup(conn.object_server(), &window.imp().playbin2)
                     .await
                     .expect("could not serve mpris player");