resources/play_queue.blp

@@ -5,7 +5,7 @@ template $AudreyUiPlayQueue: Adw.Bin {
   name: "play-queue";
 
   child: Stack {
-    visible-child-name: bind $visible_child_name (template.model as <$GListStore>.n-items) as <string>;
+    visible-child-name: bind $visible_child_name (template.playbin as <$AudreyPlaybin>.play-queue-length) as <string>;
 
     StackPage {
       name: "empty";
@@ -31,7 +31,7 @@ template $AudreyUiPlayQueue: Adw.Bin {
           activate => $on_row_activated () swapped;
 
           model: NoSelection {
-            model: bind template.model;
+            model: bind template.playbin as <$AudreyPlaybin>.play_queue;
           };
 
           factory: SignalListItemFactory {

resources/play_queue_song.blp

@@ -35,7 +35,7 @@ template $AudreyUiPlayQueueSong: Box {
     margin-top: 1;
     margin-bottom: 1;
     pixel-size: 50;
-    //paintable: bind template.song as <$AudreyPlaybinSong>.thumbnail;
+    paintable: bind template.song as <$AudreyPlaybinSong>.thumbnail;
   }
 
   Box title_box {

resources/window.blp

@@ -119,7 +119,7 @@ template $AudreyUiWindow: Adw.ApplicationWindow {
             margin-end: 24;
 
             styles [ "frame" ]
-            //playbin: bind template.playbin;
+            playbin: bind template.playbin;
           }
         };
       }

src/mpris/player.rs

@@ -1,10 +1,8 @@
-use gtk::glib::spawn_future_local;
+use glib::SendWeakRef;
+use gtk::glib;
 use std::collections::HashMap;
-use std::rc::{Rc, Weak};
-use zbus::zvariant::{ObjectPath, OwnedObjectPath, OwnedValue, Value};
-
-use crate::playbin::Song as PlaybinSong;
-type Playbin = crate::playbin2::Playbin<PlaybinSong>;
+use zbus::object_server::SignalEmitter;
+use zbus::zvariant::{ObjectPath, OwnedObjectPath, Value};
 
 const MICROSECONDS: f64 = 1e6; // in a second
 
@@ -59,91 +57,70 @@ impl MetadataMap {
         .unwrap_or_default()
     }
 
-    fn as_hash_map(&self) -> HashMap<&'static str, OwnedValue> {
+    fn as_hash_map(&self) -> HashMap<&'static str, Value> {
         let mut map = HashMap::new();
 
         if let Some(track_id) = &self.track_id {
-            map.insert(
-                "mpris:trackid",
-                Value::new(track_id.as_ref()).try_into().unwrap(),
-            );
+            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()).try_into().unwrap(),
-            );
+            map.insert("mpris:artUrl", Value::new(art_url.to_string()));
         }
         if let Some(length) = &self.length {
-            map.insert("mpris:length", Value::new(length).try_into().unwrap());
+            map.insert("mpris:length", Value::new(length));
         }
         if let Some(album) = &self.album {
-            map.insert("xesam:album", Value::new(album).try_into().unwrap());
+            map.insert("xesam:album", Value::new(album));
         }
         if let Some(artist) = &self.artist {
-            map.insert("xesam:artist", Value::new(artist).try_into().unwrap());
+            map.insert("xesam:artist", Value::new(artist));
         }
         if let Some(content_created) = &self.content_created {
             map.insert(
                 "xesam:contentCreated",
-                Value::new(content_created.format("%+").to_string())
-                    .try_into()
-                    .unwrap(),
+                Value::new(content_created.format("%+").to_string()),
             );
         }
         if let Some(genre) = &self.genre {
-            map.insert("xesam:genre", Value::new(genre).try_into().unwrap());
+            map.insert("xesam:genre", Value::new(genre));
         }
         if let Some(track_number) = self.track_number {
-            map.insert(
-                "xesam:trackNumber",
-                Value::new(track_number).try_into().unwrap(),
-            );
+            map.insert("xesam:trackNumber", Value::new(track_number));
         }
         if let Some(title) = &self.title {
-            map.insert("xesam:title", Value::new(title).try_into().unwrap());
+            map.insert("xesam:title", Value::new(title));
         }
         if let Some(user_rating) = self.user_rating {
-            map.insert(
-                "xesam:userRating",
-                Value::new(user_rating).try_into().unwrap(),
-            );
+            map.insert("xesam:userRating", Value::new(user_rating));
         }
 
         map
     }
 }
 
-pub struct Player(async_channel::Sender<Box<dyn FnOnce(Rc<LocalPlayer>) + Send>>);
+pub struct Player {
+    playbin: SendWeakRef<crate::Playbin>,
+    metadata: MetadataMap,
+}
 
 impl Player {
     pub async fn setup(
         object_server: &zbus::ObjectServer,
-        playbin: &Rc<Playbin>,
+        playbin: &crate::Playbin,
     ) -> Result<(), zbus::Error> {
-        let local = LocalPlayer {
+        use adw::prelude::*;
+
+        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,
@@ -239,46 +216,182 @@ impl Player {
                 }
             ),
         );
-        */
 
         Ok(())
     }
 
-    async fn with_local<T: Send + 'static, F: std::future::Future<Output = T>>(
-        &self,
-        f: impl FnOnce(Rc<LocalPlayer>) -> F + Send + 'static,
-    ) -> 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()
+    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),
+        }
     }
 }
 
-// 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 {
-    async fn next(&self) -> zbus::fdo::Result<()> {
-        self.with_local(move |local| async move { local.next() })
-            .await
+    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.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)]
-    async fn playback_status(&self) -> zbus::fdo::Result<String> {
-        self.with_local(|local| async move { local.playback_status() })
-            .await
+    fn playback_status(&self) -> zbus::fdo::Result<&str> {
+        match self.playbin()?.state() {
+            crate::playbin::State::Stopped => Ok("Stopped"),
+            crate::playbin::State::Playing => Ok("Playing"),
+            crate::playbin::State::Paused => Ok("Paused"),
+        }
     }
 
     #[zbus(property)]
@@ -297,11 +410,10 @@ impl Player {
     }
 
     #[zbus(property)]
-    async fn set_rate(&self, rate: f64) -> zbus::Result<()> {
+    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.with_local(|local| async move { local.pause() })
-                .await?;
+            self.pause()?;
         }
 
         // just ignore anything else
@@ -309,39 +421,42 @@ impl Player {
     }
 
     #[zbus(property)]
-    // FIXME: https://github.com/dbus2/zbus/issues/992
+    // FIXME: zbus bug (?): this getter can't be infallible
     fn shuffle(&self) -> zbus::fdo::Result<bool> {
         Ok(false)
     }
 
     #[zbus(property)]
-    // FIXME: see above
+    // FIXME: zbus bug (?): this setter can't return zbus::fdo::Result
     fn set_shuffle(&self, _shuffle: bool) -> zbus::Result<()> {
         Err(zbus::fdo::Error::NotSupported("setting Shuffle".into()).into())
     }
 
     #[zbus(property)]
-    async fn metadata(&self) -> zbus::fdo::Result<HashMap<&'static str, OwnedValue>> {
-        self.with_local(move |local| async move { Ok(local.metadata.as_hash_map()) })
-            .await
+    fn metadata(&self) -> HashMap<&'static str, Value> {
+        self.metadata.as_hash_map()
     }
 
     #[zbus(property)]
-    async fn volume(&self) -> zbus::fdo::Result<f64> {
-        self.with_local(|local| async move { local.volume() }).await
+    fn volume(&self) -> zbus::fdo::Result<f64> {
+        Ok(self.playbin()?.volume() as f64 / 100.0)
     }
 
     #[zbus(property)]
-    async fn set_volume(&self, volume: f64) -> zbus::Result<()> {
-        self.with_local(move |local| async move { local.set_volume(volume) })
-            .await?;
+    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()?;
+        // 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"))]
-    async fn position(&self) -> zbus::fdo::Result<i64> {
-        self.with_local(|local| async move { local.position() })
-            .await
+    fn position(&self) -> zbus::fdo::Result<i64> {
+        Ok((self.playbin()?.position() * MICROSECONDS) as i64)
     }
 
     #[zbus(property)]
@@ -355,186 +470,36 @@ 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()?.entries().len() > 0)
+        Ok(self.playbin()?.play_queue_length() > 0)
     }
 
+    #[zbus(property)]
     fn can_go_previous(&self) -> zbus::fdo::Result<bool> {
         // same as can_play
-        Ok(self.playbin()?.entries().len() > 0)
+        Ok(self.playbin()?.play_queue_length() > 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()?.entries().len() > 0)
+        Ok(self.playbin()?.play_queue_length() > 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/mpv/format.rs

@@ -1,5 +1,5 @@
 use super::{ffi, Error};
-use std::ffi::{c_char, c_int, c_void, CStr, CString};
+use std::ffi::{c_char, c_int, c_void, CString};
 
 pub trait SetProperty {
     /// # Safety
@@ -121,18 +121,3 @@ impl GetProperty for i64 {
         Ok(value)
     }
 }
-
-impl GetProperty for String {
-    unsafe fn get_property(ctx: *mut ffi::mpv_handle, name: *const c_char) -> Result<Self, Error> {
-        let mut value: *mut c_char = std::ptr::null_mut();
-        Error::from_return_code(ffi::mpv_get_property(
-            ctx,
-            name,
-            ffi::mpv_format_MPV_FORMAT_STRING,
-            std::ptr::from_mut::<*mut c_char>(&mut value) as *mut c_void,
-        ))?;
-        let result = CStr::from_ptr(value).to_string_lossy().into_owned();
-        ffi::mpv_free(value as *mut c_void);
-        Ok(result)
-    }
-}

src/playbin.vala

@@ -13,35 +13,58 @@ public class Audrey.PlaybinSong : Object {
     private static int64 next_counter = 0;
     public int64 counter { get; private set; }
 
-    public string id { get; set; }
-    public string title { get; set; }
-    public string artist { get; set; }
-    public string album { get; set; }
-    public string? genre { get; set; }
-    public int64 duration { get; set; }
-    public int64 track { get; set; }
-    public int64 play_count { get; set; }
+    private Subsonic.Song inner;
+    public string id { get { return inner.id; } }
+    public string title { get { return inner.title; } }
+    public string artist { get { return inner.artist; } }
+    public string album { get { return inner.album; } }
+    public string? genre { get { return inner.genre; } }
+    public int64 duration { get { return inner.duration; } }
+    public int64 track { get { return inner.track; } }
+    public int64 play_count { get { return inner.play_count; } }
 
-    public string cover_art_url { get; set; }
-    public string stream_url { get; set; }
+    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;
+
+    public PlaybinSong (Subsonic.Client api, Subsonic.Song song) {
+        this.api = api;
+        this.inner = song;
 
-    construct {
         this.counter = next_counter;
         next_counter += 1;
     }
 
-    public PlaybinSong (Subsonic.Client api, Subsonic.Song song) {
-        this.id = song.id;
-        this.title = song.title;
-        this.artist = song.artist;
-        this.album = song.album;
-        this.genre = song.genre;
-        this.duration = song.duration;
-        this.track = song.track;
-        this.play_count = song.play_count;
+    private Subsonic.Client api;
 
-        this.cover_art_url = api.cover_art_uri(this.id);
-        this.stream_url = api.stream_uri(this.id);
+    public void need_cover_art () {
+        /* TODO
+        if (this.cancel_loading_thumbnail != null) return;
+        if (this.thumbnail != null) return;
+
+        this.cancel_loading_thumbnail = new Cancellable ();
+        // TODO: dpi scaling maybe?? probably
+        api.cover_art.begin (this.id, 50, Priority.LOW, this.cancel_loading_thumbnail, (obj, res) => {
+            try {
+                var pixbuf = api.cover_art.end (res);
+                this.thumbnail = Gdk.Texture.for_pixbuf (pixbuf);
+            } catch (Error e) {
+                if (!(e is IOError.CANCELLED)) {
+                    warning ("could not fetch cover art for song %s: %s", this.id, e.message);
+                }
+            }
+            this.cancel_loading_thumbnail = null;
+        });
+        */
+    }
+
+    ~PlaybinSong () {
+        if (this.cancel_loading_thumbnail != null) {
+            this.cancel_loading_thumbnail.cancel ();
+        }
     }
 }
 

src/playbin/song.rs

@@ -82,19 +82,4 @@ impl Song {
         };
         url::Url::parse(&url).expect("invalid url from vala side")
     }
-
-    pub fn from_child(
-        api: &crate::subsonic::Client,
-        child: &crate::subsonic::schema::Child,
-    ) -> Self {
-        glib::Object::builder()
-            .property("id", &child.id)
-            .property("title", &child.title)
-            .property("artist", &child.artist)
-            .property("album", &child.album)
-            .property("duration", child.duration as i64)
-            .property("cover-art-url", api.cover_art_url(&child.id).as_str())
-            .property("stream-url", api.stream_url(&child.id).as_str())
-            .build()
-    }
 }

src/playbin2.rs

@@ -24,9 +24,9 @@ pub struct Playbin<E> {
     paused_changed: SignalEmitter<Self, ()>,
     current_entry_changed: SignalEmitter<Self, ()>,
 
-    entry_inserted: SignalEmitter<Self, usize>,
+    entry_inserted: SignalEmitter<Self, u32>,
     stopped: SignalEmitter<Self, ()>,
-    entry_removed: SignalEmitter<Self, usize>,
+    entry_removed: SignalEmitter<Self, u32>,
 
     file_started: SignalEmitter<Self, ()>,
 }
@@ -98,7 +98,7 @@ where
         todo!()
     }
 
-    pub fn current_entry(&self) -> Option<usize> {
+    pub fn current_entry(&self) -> Option<u32> {
         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: usize) {
+    pub fn play_entry(&self, index: u32) {
         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 usize);
+        self.entry_inserted.emit(self, index as u32);
     }
 
-    pub fn insert_entry(&self, index: usize, entry: E) {
+    pub fn insert_entry(&self, index: u32, entry: E) {
         let mut entries = self.entries.borrow_mut();
         self.mpv
             .command(["loadfile", entry.url().as_str(), "insert-at-play"])
@@ -161,18 +161,16 @@ where
         self.stopped.emit(self, ());
     }
 
-    pub fn remove_entry(&self, index: usize) {
+    pub fn remove_entry(&self, index: u32) {
         let mut entries = self.entries.borrow_mut();
-        self.mpv
-            .command(["playlist-remove", &index.to_string()])
-            .unwrap();
+        self.mpv.command(["remove", &index.to_string()]).unwrap();
         entries.remove(index as usize);
 
         drop(entries);
         self.entry_removed.emit(self, index);
     }
 
-    pub fn move_entry(&self, _from: usize, _to: usize) {
+    pub fn move_entry(&self, _from: u32, _to: u32) {
         todo!()
     }
 
@@ -190,25 +188,25 @@ where
                 0 => {
                     assert_eq!(&event.name, "volume");
                     self.volume_changed.emit(self, ());
-                    dbg!(self.volume());
+                    println!("new volume! {:?}", self.volume());
                 }
 
                 1 => {
                     assert_eq!(&event.name, "mute");
                     self.muted_changed.emit(self, ());
-                    dbg!(self.muted());
+                    println!("new muted! {:?}", self.muted());
                 }
 
                 2 => {
                     assert_eq!(&event.name, "pause");
                     self.paused_changed.emit(self, ());
-                    dbg!(self.paused());
+                    println!("new paused! {:?}", self.paused());
                 }
 
                 3 => {
                     assert_eq!(&event.name, "playlist-pos");
                     self.current_entry_changed.emit(self, ());
-                    dbg!(self.current_entry());
+                    println!("new current_entry! {:?}", self.current_entry());
                 }
 
                 _ => unreachable!(),
@@ -229,12 +227,6 @@ where
                 // since we set up the hook before, the current song is guaranteed not to change
                 // under our feet
                 self.file_started.emit(self, ());
-
-                // sanity check
-                assert_eq!(
-                    self.entries()[self.current_entry().unwrap()].url().as_str(),
-                    &self.mpv.get_property::<String>("path").unwrap()
-                );
             }
 
             _ => println!("mpv event {:?}", event),
@@ -249,22 +241,6 @@ where
         self.muted_changed.signal()
     }
 
-    pub fn current_entry_changed(&self) -> Signal<'_, Self, ()> {
-        self.current_entry_changed.signal()
-    }
-
-    pub fn entry_inserted(&self) -> Signal<'_, Self, usize> {
-        self.entry_inserted.signal()
-    }
-
-    pub fn stopped(&self) -> Signal<'_, Self, ()> {
-        self.stopped.signal()
-    }
-
-    pub fn entry_removed(&self) -> Signal<'_, Self, usize> {
-        self.entry_removed.signal()
-    }
-
     pub fn file_started(&self) -> Signal<'_, Self, ()> {
         self.file_started.signal()
     }

src/signal.rs

@@ -25,7 +25,7 @@ pub struct Signal<'a, E, T> {
     just_connected: &'a RefCell<Vec<SignalHandlerBox<E, T>>>,
 }
 
-#[derive(Clone, Default)]
+#[derive(Clone)]
 pub struct SignalHandler(Weak<Cell<bool>>);
 
 impl SignalHandler {
@@ -101,16 +101,28 @@ impl<E, T> SignalEmitter<E, T> {
             .expect("tried to re-emit signal during emission");
         handlers.append(self.just_connected.borrow_mut().as_mut());
 
+        if handlers.is_empty() {
+            return;
+        }
+
         let mut i = 0;
         let mut skip = 0;
-        // FIXME: does not preserve ordering
-        while i < handlers.len() {
-            if handlers[i](emitter, f()) {
+        loop {
+            if handlers[i + skip](emitter, f()) {
                 i += 1;
             } else {
-                drop(handlers.swap_remove(i));
+                skip += 1;
             }
+
+            if i + skip == handlers.len() {
+                break;
+            }
+
+            handlers.swap(i, i + skip);
         }
+
+        println!("emitted to {i} listeners");
+        handlers.truncate(i);
     }
 }
 

src/subsonic.rs

@@ -176,10 +176,6 @@ impl Client {
         .map(|response| response.random_songs.song)
     }
 
-    pub fn cover_art_url(&self, id: &str) -> url::Url {
-        self.url(&["rest", "coverArt"], &[("id", id)])
-    }
-
     pub fn stream_url(&self, id: &str) -> url::Url {
         self.url(&["rest", "stream"], &[("id", id)])
     }

src/ui/play_queue.rs

@@ -2,22 +2,15 @@ pub mod song;
 pub use song::Song;
 
 mod imp {
-    use crate::playbin::Song as PlaybinSong;
-    use adw::{gio, glib, prelude::*, subclass::prelude::*};
+    use adw::{glib, prelude::*, subclass::prelude::*};
     use glib::{subclass::InitializingObject, WeakRef};
-    use std::cell::{Cell, RefCell};
-    use std::rc::Rc;
-
-    type Playbin = crate::playbin2::Playbin<PlaybinSong>;
 
     #[derive(gtk::CompositeTemplate, glib::Properties, Default)]
     #[template(resource = "/eu/callcc/audrey/play_queue.ui")]
     #[properties(wrapper_type = super::PlayQueue)]
     pub struct PlayQueue {
         #[property(get, set)]
-        pub(super) model: RefCell<Option<gio::ListStore>>,
-
-        pub(super) playbin: RefCell<Option<Rc<Playbin>>>,
+        playbin: WeakRef<crate::Playbin>,
     }
 
     #[glib::object_subclass]
@@ -37,13 +30,7 @@ mod imp {
     }
 
     #[glib::derived_properties]
-    impl ObjectImpl for PlayQueue {
-        fn constructed(&self) {
-            self.parent_constructed();
-
-            self.obj().set_model(gio::ListStore::new::<PlaybinSong>());
-        }
-    }
+    impl ObjectImpl for PlayQueue {}
 
     impl WidgetImpl for PlayQueue {}
 
@@ -62,7 +49,7 @@ mod imp {
 
         #[template_callback]
         fn on_song_list_setup(&self, item: &gtk::ListItem, _factory: &gtk::SignalListItemFactory) {
-            let child = super::Song::new(self.playbin.borrow().as_ref().unwrap());
+            let child = super::Song::new(&self.playbin.upgrade().unwrap());
 
             child.set_draggable(true);
             child.set_show_position(true);
@@ -94,11 +81,7 @@ mod imp {
 
         #[template_callback]
         fn on_row_activated(&self, position: u32) {
-            self.playbin
-                .borrow()
-                .as_ref()
-                .unwrap()
-                .play_entry(position as usize);
+            self.obj().playbin().unwrap().select_track(position);
         }
     }
 
@@ -109,47 +92,10 @@ mod imp {
     }
 }
 
-use crate::playbin::Song as PlaybinSong;
-use adw::subclass::prelude::*;
 use gtk::glib;
-use std::rc::Rc;
-
-type Playbin = crate::playbin2::Playbin<PlaybinSong>;
 
 glib::wrapper! {
     pub struct PlayQueue(ObjectSubclass<imp::PlayQueue>)
         @extends adw::Bin, gtk::Widget,
         @implements gtk::Accessible, gtk::Buildable, gtk::ConstraintTarget;
 }
-
-impl PlayQueue {
-    pub fn set_playbin(&self, playbin: &Rc<Playbin>) {
-        assert!(self
-            .imp()
-            .playbin
-            .replace(Some(Rc::clone(playbin)))
-            .is_none()); // only set once
-
-        playbin
-            .entry_inserted()
-            .connect_object(self, |playbin, play_queue, index| {
-                play_queue
-                    .model()
-                    .unwrap()
-                    .insert(index as u32, &playbin.entries()[index]);
-                true
-            });
-        playbin
-            .stopped()
-            .connect_object(self, |playbin, play_queue, ()| {
-                play_queue.model().unwrap().remove_all();
-                true
-            });
-        playbin
-            .entry_removed()
-            .connect_object(self, |playbin, play_queue, index| {
-                play_queue.model().unwrap().remove(index as u32);
-                true
-            });
-    }
-}

src/ui/play_queue/song.rs

@@ -1,17 +1,14 @@
 mod imp {
-    use crate::playbin::Song as PlaybinSong;
-    use crate::signal::SignalHandler;
     use glib::{subclass::InitializingObject, WeakRef};
     use gtk::{gdk, gio, glib, prelude::*, subclass::prelude::*};
     use std::cell::{Cell, RefCell};
-    use std::rc::Rc;
-
-    type Playbin = crate::playbin2::Playbin<PlaybinSong>;
 
     #[derive(gtk::CompositeTemplate, glib::Properties, Default)]
     #[template(resource = "/eu/callcc/audrey/play_queue_song.ui")]
     #[properties(wrapper_type = super::Song)]
     pub struct Song {
+        pub(super) playbin: WeakRef<crate::Playbin>,
+
         #[property(set, get)]
         draggable: Cell<bool>,
         #[property(set, get)]
@@ -27,13 +24,12 @@ mod imp {
         #[property(set, get)]
         displayed_position: Cell<u32>,
         #[property(get, set)]
-        song: RefCell<Option<PlaybinSong>>,
+        song: RefCell<Option<crate::playbin::Song>>,
+
+        pub(super) connection: Cell<Option<glib::SignalHandlerId>>,
 
         drag_pos: Cell<(i32, i32)>,
         drag_widget: Cell<Option<gtk::ListBox>>,
-
-        pub(super) playbin: RefCell<Option<Rc<Playbin>>>,
-        pub(super) connection: Cell<SignalHandler>,
     }
 
     #[glib::object_subclass]
@@ -65,7 +61,7 @@ mod imp {
                         self_
                             .obj()
                             .playbin()
-                            .remove_entry(self_.obj().displayed_position() as usize - 1)
+                            .remove_track(self_.obj().displayed_position() - 1)
                     }
                 ))
                 .build();
@@ -165,16 +161,18 @@ mod imp {
             false
         }
     }
+
+    impl Drop for Song {
+        fn drop(&mut self) {
+            println!("dropping AudreyUiPlayQueueSong");
+        }
+    }
 }
 
-use crate::playbin::Song as PlaybinSong;
 use adw::prelude::*;
 use adw::subclass::prelude::*;
 use glib::Object;
 use gtk::glib;
-use std::rc::Rc;
-
-type Playbin = crate::playbin2::Playbin<PlaybinSong>;
 
 glib::wrapper! {
     pub struct Song(ObjectSubclass<imp::Song>)
@@ -183,38 +181,36 @@ glib::wrapper! {
 }
 
 impl Song {
-    pub fn new(playbin: &Rc<Playbin>) -> Self {
+    pub fn new(playbin: &crate::Playbin) -> Self {
         let song: Self = Object::new();
-
-        assert!(song
-            .imp()
-            .playbin
-            .replace(Some(Rc::clone(playbin)))
-            .is_none()); // only set once
-
+        song.imp().playbin.set(Some(playbin));
         song
     }
 
-    fn playbin(&self) -> Rc<Playbin> {
-        Rc::clone(self.imp().playbin.borrow().as_ref().unwrap())
+    fn playbin(&self) -> crate::Playbin {
+        self.imp().playbin.upgrade().unwrap()
     }
 
     pub fn bind(&self, position: u32, song: &crate::playbin::Song) {
         self.set_displayed_position(position + 1);
         self.set_song(song);
-        self.set_current(self.playbin().current_entry() == Some(position as usize));
+        self.set_current(self.playbin().play_queue_position() == position);
         self.imp()
             .connection
-            .replace(self.playbin().current_entry_changed().connect_object(
-                self,
-                move |playbin, song, ()| {
-                    song.set_current(playbin.current_entry() == Some(position as usize));
-                    true
-                },
-            ));
+            .replace(Some(self.playbin().connect_notify_local(
+                Some("play-queue-position"),
+                glib::clone!(
+                    #[weak(rename_to = self_)]
+                    self,
+                    move |playbin: &crate::Playbin, _| {
+                        self_.set_current(playbin.play_queue_position() == position)
+                    }
+                ),
+            )));
     }
 
     pub fn unbind(&self) {
-        self.imp().connection.take().disconnect();
+        self.playbin()
+            .disconnect(self.imp().connection.take().unwrap());
     }
 }

src/ui/window.rs

@@ -6,12 +6,6 @@ mod imp {
     use std::cell::{Cell, RefCell};
     use std::rc::Rc;
 
-    impl crate::playbin2::PlaybinEntry for crate::playbin::Song {
-        fn url(&self) -> url::Url {
-            self.stream_url()
-        }
-    }
-
     #[derive(gtk::CompositeTemplate, glib::Properties, Default)]
     #[template(resource = "/eu/callcc/audrey/window.ui")]
     #[properties(wrapper_type = super::Window)]
@@ -19,9 +13,6 @@ mod imp {
         #[template_child]
         pub(super) playbar: TemplateChild<crate::ui::Playbar>,
 
-        #[template_child]
-        pub(super) play_queue: TemplateChild<crate::ui::PlayQueue>,
-
         #[property(get, set)]
         playbin: RefCell<crate::Playbin>,
 
@@ -36,7 +27,7 @@ mod imp {
 
         pub(super) setup: crate::ui::Setup,
 
-        pub(super) playbin2: Rc<crate::playbin2::Playbin<crate::playbin::Song>>,
+        pub(super) playbin2: Rc<crate::playbin2::Playbin<url::Url>>,
         pub(super) api2: RefCell<Option<Rc<crate::subsonic::Client>>>,
     }
 
@@ -75,8 +66,6 @@ mod imp {
                 }
             }));
 
-            self.play_queue.set_playbin(&self.playbin2);
-
             // set up mpris
             let window = self.obj().clone();
             glib::spawn_future_local(async move {
@@ -101,7 +90,7 @@ mod imp {
                 crate::Mpris::setup(conn.object_server(), &window)
                     .await
                     .expect("could not serve mpris");
-                crate::mpris::Player::setup(conn.object_server(), &window.imp().playbin2)
+                crate::mpris::Player::setup(conn.object_server(), &window.playbin())
                     .await
                     .expect("could not serve mpris player");
 
@@ -137,8 +126,8 @@ mod imp {
             let api = self.api2.borrow();
             let api = api.as_ref().unwrap();
             for song in api.get_random_songs(10).await.unwrap().into_iter() {
-                self.playbin2
-                    .push_entry(crate::playbin::Song::from_child(api, &song));
+                println!("{song:?}");
+                self.playbin2.push_entry(api.stream_url(&song.id));
             }
             self.obj().set_can_click_shuffle_all(true);
         }
@@ -200,10 +189,7 @@ impl Window {
         let window: Self = glib::Object::builder().property("application", app).build();
 
         // manual bidirectional sync
-        window
-            .imp()
-            .playbar
-            .set_volume(window.imp().playbin2.volume() as i32);
+        window.imp().playbar.set_volume(window.imp().playbin2.volume() as i32);
         window.imp().playbin2.volume_changed().connect_object(
             &*window.imp().playbar,
             |playbin, playbar, ()| {