equivalent exchange of boilerplate

src/main.vala

@@ -1,12 +0,0 @@
-int main (string[] args) {
-    // do it ourselves or it's broken
-    Gtk.disable_setlocale ();
-    Intl.bindtextdomain (Audrey.Config.GETTEXT_PACKAGE, Audrey.Config.LOCALEDIR);
-    Intl.bind_textdomain_codeset (Audrey.Config.GETTEXT_PACKAGE, "UTF-8");
-    Intl.textdomain (Audrey.Config.GETTEXT_PACKAGE);
-    Intl.setlocale (LocaleCategory.ALL, "");
-    Intl.setlocale (LocaleCategory.NUMERIC, "C.UTF-8");
-
-    var app = new Audrey.Application ();
-    return app.run (args);
-}

src/mpris/player.rs

@@ -11,7 +11,7 @@ struct MetadataMap {
     // mpris
     track_id: Option<OwnedObjectPath>,
     length: Option<i64>,
-    //art_url: Option<url::Url>,
+    art_url: Option<url::Url>,
     // xesam
     album: Option<String>,
     //album_artist: Option<Vec<String>>,
@@ -44,6 +44,7 @@ impl MetadataMap {
                     .unwrap()
             }),
             length: Some(song.duration() * MICROSECONDS as i64),
+            //art_url: Some(song.cover_art_url()), // FIXME: this would leak credentials
             album: Some(song.album().into()),
             artist: Some(vec![song.artist().into()]),
             //content_created: song.year().map(|year| chrono::NaiveDate::from_yo_opt(year, 1).unwrap()), // FIXME: replace this unwrap with Some(Err) -> None
@@ -62,6 +63,9 @@ impl MetadataMap {
         if let Some(track_id) = &self.track_id {
             map.insert("mpris:trackid", Value::new(track_id.as_ref()));
         }
+        if let Some(art_url) = &self.art_url {
+            map.insert("mpris:artUrl", Value::new(art_url.to_string()));
+        }
         if let Some(length) = &self.length {
             map.insert("mpris:length", Value::new(length));
         }
@@ -117,13 +121,10 @@ impl Player {
             .interface::<_, Self>("/org/mpris/MediaPlayer2")
             .await?;
 
-        playbin.connect_closure(
+        playbin.connect_new_track(glib::clone!(
-            "new-track",
-            false,
-            glib::closure_local!(
             #[strong]
             player_ref,
-                move |_playbin: &crate::Playbin, song: &crate::playbin::Song| {
+            move |_, song| {
                 let metadata = MetadataMap::from_playbin_song(Some(song));
 
                 let player_ref = player_ref.clone();
@@ -136,16 +137,12 @@ impl Player {
                         .unwrap();
                 });
             }
-            ),
+        ));
-        );
 
-        playbin.connect_closure(
+        playbin.connect_seeked(glib::clone!(
-            "seeked",
-            false,
-            glib::closure_local!(
             #[strong]
             player_ref,
-                move |_playbin: &crate::Playbin, position: f64| {
+            move |_, position| {
                 let player_ref = player_ref.clone();
                 glib::spawn_future_local(async move {
                     player_ref
@@ -154,15 +151,14 @@ impl Player {
                         .unwrap();
                 });
             }
-            ),
+        ));
-        );
 
         playbin.connect_notify_local(
             Some("play-queue-length"),
             glib::clone!(
                 #[strong]
                 player_ref,
-                move |_playbin: &crate::Playbin, _| {
+                move |_, _| {
                     let player_ref = player_ref.clone();
                     glib::spawn_future_local(async move {
                         let player = player_ref.get_mut().await;
@@ -189,7 +185,7 @@ impl Player {
             glib::clone!(
                 #[strong]
                 player_ref,
-                move |_playbin: &crate::Playbin, _| {
+                move |_, _| {
                     let player_ref = player_ref.clone();
                     glib::spawn_future_local(async move {
                         let player = player_ref.get_mut().await;
@@ -208,7 +204,7 @@ impl Player {
             glib::clone!(
                 #[strong]
                 player_ref,
-                move |_playbin: &crate::Playbin, _| {
+                move |_, _| {
                     let player_ref = player_ref.clone();
                     glib::spawn_future_local(async move {
                         let player = player_ref.get_mut().await;
@@ -223,17 +219,24 @@ impl Player {
 
         Ok(())
     }
+
+    fn playbin(&self) -> zbus::fdo::Result<crate::Playbin> {
+        match self.playbin.upgrade() {
+            None => Err(zbus::fdo::Error::Failed("playbin was discarded".into())),
+            Some(playbin) => Ok(playbin),
+        }
+    }
 }
 
 #[zbus::interface(name = "org.mpris.MediaPlayer2.Player")]
 impl Player {
-    fn next(&self) {
+    fn next(&self) -> zbus::fdo::Result<()> {
         // If CanGoNext is false, attempting to call this method should have no effect.
-        if !self.can_go_next() {
+        if !self.can_go_next()? {
-            return;
+            return Ok(());
         }
 
-        let playbin = self.playbin.upgrade().unwrap();
+        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)
@@ -241,14 +244,16 @@ impl Player {
         } else {
             playbin.go_to_next_track();
         }
+
+        Ok(())
     }
 
-    fn previous(&self) {
+    fn previous(&self) -> zbus::fdo::Result<()> {
-        let playbin = self.playbin.upgrade().unwrap();
+        let playbin = self.playbin()?;
 
         // If CanGoPrevious is false, attempting to call this method should have no effect.
-        if !self.can_go_previous() {
+        if !self.can_go_previous()? {
-            return;
+            return Ok(());
         }
 
         if playbin.play_queue_position() == 0 {
@@ -258,74 +263,79 @@ impl Player {
         } else {
             playbin.go_to_prev_track();
         }
+
+        Ok(())
     }
 
-    fn pause(&self) {
+    fn pause(&self) -> zbus::fdo::Result<()> {
-        let playbin = self.playbin.upgrade().unwrap();
+        let playbin = self.playbin()?;
 
         // If CanPause is false, attempting to call this method should have no effect.
         if !self.can_pause() {
-            return;
+            return Ok(());
         }
 
         // If playback is already paused, this has no effect.
         if playbin.state() != crate::playbin::State::Playing {
-            return;
+            return Ok(());
         }
 
-        self.playbin.upgrade().unwrap().pause();
+        playbin.pause();
+        Ok(())
     }
 
-    fn play_pause(&self) {
+    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.upgrade().unwrap().state() == crate::playbin::State::Paused {
+        if self.playbin()?.state() == crate::playbin::State::Paused {
-            self.play();
+            self.play()
         } else {
-            self.pause();
+            self.pause()
         }
     }
 
-    fn stop(&self) {
+    fn stop(&self) -> zbus::fdo::Result<()> {
-        let playbin = self.playbin.upgrade().unwrap();
+        let playbin = self.playbin()?;
 
         // If playback is already stopped, this has no effect.
         if playbin.state() != crate::playbin::State::Playing {
-            return;
+            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) {
+    fn play(&self) -> zbus::fdo::Result<()> {
-        let playbin = self.playbin.upgrade().unwrap();
+        let playbin = self.playbin()?;
 
         // If CanPlay is false, attempting to call this method should have no effect.
-        if !self.can_play() {
+        if !self.can_play()? {
-            return;
+            return Ok(());
         }
 
         // If already playing, this has no effect.
         if playbin.state() == crate::playbin::State::Playing {
-            return;
+            return Ok(());
         }
 
         // If there is no track to play, this has no effect.
         if playbin.play_queue_length() == 0 {
-            return;
+            return Ok(());
         }
 
         playbin.play();
+        Ok(())
     }
 
-    fn seek(&self, offset: i64) {
+    fn seek(&self, offset: i64) -> zbus::fdo::Result<()> {
         // If the CanSeek property is false, this has no effect.
         if !self.can_seek() {
-            return;
+            return Ok(());
         }
 
-        let playbin = self.playbin.upgrade().unwrap();
+        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;
 
@@ -340,30 +350,32 @@ impl Player {
         }
 
         playbin.seek(new_position as f64 / MICROSECONDS);
+        Ok(())
     }
 
-    fn set_position(&self, track_id: ObjectPath<'_>, position: i64) {
+    fn set_position(&self, track_id: ObjectPath<'_>, position: i64) -> zbus::fdo::Result<()> {
-        let playbin = self.playbin.upgrade().unwrap();
+        let playbin = self.playbin()?;
 
         // If the Position argument is less than 0, do nothing.
         if position < 0 {
-            return;
+            return Ok(());
         }
         // If the Position argument is greater than the track length, do nothing.
         if position > (playbin.duration() * MICROSECONDS) as i64 {
-            return;
+            return Ok(());
         }
         // If the CanSeek property is false, this has no effect.
         if !self.can_seek() {
-            return;
+            return Ok(());
         }
         // check if it's stale
         if self.metadata.track_id.as_deref() != Some(&track_id) {
             // TODO: warn of stale seek
-            return;
+            return Ok(());
         }
 
         playbin.seek(position as f64 / MICROSECONDS);
+        Ok(())
     }
 
     fn open_uri(&self, _s: &str) -> zbus::fdo::Result<()> {
@@ -374,47 +386,50 @@ impl Player {
     async fn seeked(signal_emitter: &SignalEmitter<'_>, position: i64) -> zbus::Result<()>;
 
     #[zbus(property)]
-    fn playback_status(&self) -> String {
+    fn playback_status(&self) -> zbus::fdo::Result<&str> {
-        match self.playbin.upgrade().unwrap().state() {
+        match self.playbin()?.state() {
-            crate::playbin::State::Stopped => "Stopped".into(),
+            crate::playbin::State::Stopped => Ok("Stopped"),
-            crate::playbin::State::Playing => "Playing".into(),
+            crate::playbin::State::Playing => Ok("Playing"),
-            crate::playbin::State::Paused => "Paused".into(),
+            crate::playbin::State::Paused => Ok("Paused"),
         }
     }
 
     #[zbus(property)]
-    fn loop_status(&self) -> String {
+    fn loop_status(&self) -> zbus::fdo::Result<String> {
-        "None".into() // TODO
+        Ok("None".into()) // TODO
     }
 
     #[zbus(property)]
     fn set_loop_status(&self, _loop_status: &str) -> zbus::Result<()> {
-        Err(zbus::Error::Unsupported) // TODO
+        Err(zbus::fdo::Error::NotSupported("setting LoopStatus".into()).into()) // TODO
     }
 
     #[zbus(property)]
-    fn rate(&self) -> f64 {
+    fn rate(&self) -> zbus::fdo::Result<f64> {
-        1.0
+        Ok(1.0)
     }
 
     #[zbus(property)]
-    fn set_rate(&self, rate: f64) {
+    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.pause()?;
         }
 
         // just ignore anything else
+        Ok(())
     }
 
     #[zbus(property)]
-    fn shuffle(&self) -> bool {
+    // FIXME: zbus bug (?): this getter can't be infallible
-        false
+    fn shuffle(&self) -> zbus::fdo::Result<bool> {
+        Ok(false)
     }
 
     #[zbus(property)]
+    // FIXME: zbus bug (?): this setter can't return zbus::fdo::Result
     fn set_shuffle(&self, _shuffle: bool) -> zbus::Result<()> {
-        Err(zbus::Error::Unsupported)
+        Err(zbus::fdo::Error::NotSupported("setting Shuffle".into()).into())
     }
 
     #[zbus(property)]
@@ -423,24 +438,25 @@ impl Player {
     }
 
     #[zbus(property)]
-    fn volume(&self) -> f64 {
+    fn volume(&self) -> zbus::fdo::Result<f64> {
-        self.playbin.upgrade().unwrap().volume() as f64 / 100.0
+        Ok(self.playbin()?.volume() as f64 / 100.0)
     }
 
     #[zbus(property)]
-    fn set_volume(&mut self, mut volume: f64) {
+    fn set_volume(&mut 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.upgrade().unwrap();
+        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) -> i64 {
+    fn position(&self) -> zbus::fdo::Result<i64> {
-        (self.playbin.upgrade().unwrap().position() * MICROSECONDS) as i64
+        Ok((self.playbin()?.position() * MICROSECONDS) as i64)
     }
 
     #[zbus(property)]
@@ -454,21 +470,21 @@ impl Player {
     }
 
     #[zbus(property)]
-    fn can_go_next(&self) -> bool {
+    fn can_go_next(&self) -> zbus::fdo::Result<bool> {
         // same as can_play
-        self.playbin.upgrade().unwrap().play_queue_length() > 0
+        Ok(self.playbin()?.play_queue_length() > 0)
     }
 
     #[zbus(property)]
-    fn can_go_previous(&self) -> bool {
+    fn can_go_previous(&self) -> zbus::fdo::Result<bool> {
         // same as can_play
-        self.playbin.upgrade().unwrap().play_queue_length() > 0
+        Ok(self.playbin()?.play_queue_length() > 0)
     }
 
     #[zbus(property)]
-    fn can_play(&self) -> bool {
+    fn can_play(&self) -> zbus::fdo::Result<bool> {
         // it only makes sense to disallow "play" when the play queue is empty
-        self.playbin.upgrade().unwrap().play_queue_length() > 0
+        Ok(self.playbin()?.play_queue_length() > 0)
     }
 
     #[zbus(property)]

src/playbin.rs

@@ -49,6 +49,7 @@ pub mod ffi {
     }
 }
 
+use adw::prelude::*;
 use glib::translate::{from_glib, from_glib_none, IntoGlib, ToGlibPtr};
 use gtk::{gio, glib};
 
@@ -136,4 +137,20 @@ impl Playbin {
     pub fn play_queue(&self) -> gio::ListModel {
         unsafe { from_glib_none(ffi::audrey_playbin_get_play_queue(self.to_glib_none().0)) }
     }
+
+    pub fn connect_new_track<F: Fn(&Self, &Song) + 'static>(&self, f: F) -> glib::SignalHandlerId {
+        self.connect_closure(
+            "new-track",
+            false,
+            glib::closure_local!(|playbin, song| f(playbin, song)),
+        )
+    }
+
+    pub fn connect_seeked<F: Fn(&Self, f64) + 'static>(&self, f: F) -> glib::SignalHandlerId {
+        self.connect_closure(
+            "seeked",
+            false,
+            glib::closure_local!(|playbin, position| f(playbin, position)),
+        )
+    }
 }

src/playbin.vala

@@ -23,6 +23,9 @@ public class Audrey.PlaybinSong : Object {
     public int64 track { get { return inner.track; } }
     public int64 play_count { get { return inner.play_count; } }
 
+    public string cover_art_url { owned get { return this.api.cover_art_uri (this.id); } }
+    public string stream_url { owned get { return this.api.stream_uri (this.id); } }
+
     public Gdk.Paintable? thumbnail { get; private set; }
 
     private Cancellable cancel_loading_thumbnail;
@@ -363,24 +366,26 @@ public class Audrey.Playbin : GLib.Object {
     }
 
     public void append_track (Subsonic.Song song) {
+        var pb_song = new PlaybinSong (this.api, song);
         assert (this.mpv.command ({
             "loadfile",
-            this.api.stream_uri (song.id),
+            pb_song.stream_url,
             "append",
         }) >= 0);
-        this._play_queue.append (new PlaybinSong (this.api, song));
+        this._play_queue.append (pb_song);
         this.play_queue_length += 1;
     }
 
     public async void append_track_async (Subsonic.Song song) {
+        var pb_song = new PlaybinSong (this.api, song);
         var err = yield this.mpv_command_async ({
             "loadfile",
-            this.api.stream_uri (song.id),
+            pb_song.stream_url,
             "append",
         });
         assert (err >= 0);
 
-        this._play_queue.append (new PlaybinSong (this.api, song));
+        this._play_queue.append (pb_song);
         this.play_queue_length += 1;
     }
 

src/playbin/song.rs

@@ -1,5 +1,6 @@
 pub mod ffi {
     use gtk::glib;
+    use std::ffi::c_char;
 
     #[repr(C)]
     pub struct AudreyPlaybinSong {
@@ -14,18 +15,16 @@ pub mod ffi {
     extern "C" {
         pub fn audrey_playbin_song_get_type() -> glib::ffi::GType;
         pub fn audrey_playbin_song_get_counter(self_: *mut AudreyPlaybinSong) -> i64;
-        pub fn audrey_playbin_song_get_id(self_: *mut AudreyPlaybinSong)
+        pub fn audrey_playbin_song_get_id(self_: *mut AudreyPlaybinSong) -> *const c_char;
-            -> *const std::ffi::c_char;
+        pub fn audrey_playbin_song_get_title(self_: *mut AudreyPlaybinSong) -> *const c_char;
-        pub fn audrey_playbin_song_get_title(
+        pub fn audrey_playbin_song_get_artist(self_: *mut AudreyPlaybinSong) -> *const c_char;
-            self_: *mut AudreyPlaybinSong,
+        pub fn audrey_playbin_song_get_album(self_: *mut AudreyPlaybinSong) -> *const c_char;
-        ) -> *const std::ffi::c_char;
-        pub fn audrey_playbin_song_get_artist(
-            self_: *mut AudreyPlaybinSong,
-        ) -> *const std::ffi::c_char;
-        pub fn audrey_playbin_song_get_album(
-            self_: *mut AudreyPlaybinSong,
-        ) -> *const std::ffi::c_char;
         pub fn audrey_playbin_song_get_duration(self_: *mut AudreyPlaybinSong) -> i64;
+
+        pub fn audrey_playbin_song_get_cover_art_url(
+            self_: *mut AudreyPlaybinSong,
+        ) -> *const c_char;
+        pub fn audrey_playbin_song_get_stream_url(self_: *mut AudreyPlaybinSong) -> *const c_char;
     }
 }
 
@@ -65,4 +64,22 @@ impl Song {
     pub fn duration(&self) -> i64 {
         unsafe { ffi::audrey_playbin_song_get_duration(self.to_glib_none().0) }
     }
+
+    pub fn cover_art_url(&self) -> url::Url {
+        let url: String = unsafe {
+            from_glib_none(ffi::audrey_playbin_song_get_cover_art_url(
+                self.to_glib_none().0,
+            ))
+        };
+        url::Url::parse(&url).expect("invalid url from vala side")
+    }
+
+    pub fn stream_url(&self) -> url::Url {
+        let url: String = unsafe {
+            from_glib_none(ffi::audrey_playbin_song_get_stream_url(
+                self.to_glib_none().0,
+            ))
+        };
+        url::Url::parse(&url).expect("invalid url from vala side")
+    }
 }