pkgs/mpdpopmd: Don't add a new song to playlist for every event

This tries to keep the playlist size somewhat bounded, but is probably
not yet a perfect solution.

2 files changed, 35 insertions, 12 deletions

diff --git a/pkgs/by-name/mp/mpdpopm/src/lib.rs b/pkgs/by-name/mp/mpdpopm/src/lib.rs
index a69beeb2..2394b729 100644
--- a/pkgs/by-name/mp/mpdpopm/src/lib.rs
+++ b/pkgs/by-name/mp/mpdpopm/src/lib.rs
@@ -144,14 +144,14 @@ pub async fn mpdpopm(cfg: Config) -> std::result::Result<(), Error> {
                         Ok(subsys) => {
                             debug!("subsystem {} changed", subsys);
                             if subsys == IdleSubSystem::Player {
-                                state.update(&mut client)
+                                if state.update(&mut client)
                                     .await
-                                    .context("PlayState update failed")?;
-
+                                    .context("PlayState update failed")? {
                                 mqueue
                                     .advance_dj(&mut client)
                                     .await
                                     .context("MessageQueue tick failed")?;
+                                }
                             } else if subsys == IdleSubSystem::Message {
                                 msg_check_needed = true;
                             }
diff --git a/pkgs/by-name/mp/mpdpopm/src/playcounts.rs b/pkgs/by-name/mp/mpdpopm/src/playcounts.rs
index 417b3e7e..2ef4993f 100644
--- a/pkgs/by-name/mp/mpdpopm/src/playcounts.rs
+++ b/pkgs/by-name/mp/mpdpopm/src/playcounts.rs
@@ -71,13 +71,16 @@ impl PlayState {
 
     /// Poll the server-- update our status; maybe increment the current track's play count; the
     /// caller must arrange to have this method invoked periodically to keep our state fresh
-    pub async fn update(&mut self, client: &mut Client) -> Result<()> {
+    ///
+    /// Returns whether a song finished between the last call and this one.
+    /// That can be used to add a new song to the queue.
+    pub async fn update(&mut self, client: &mut Client) -> Result<bool> {
         let new_stat = client
             .status()
             .await
             .context("Failed to get client status")?;
 
-        match (&self.last_server_stat, &new_stat) {
+        let previous_song_finished = match (&self.last_server_stat, &new_stat) {
             (PlayerStatus::Play(last), PlayerStatus::Play(curr))
             | (PlayerStatus::Pause(last), PlayerStatus::Play(curr))
             | (PlayerStatus::Play(last), PlayerStatus::Pause(curr))
@@ -93,12 +96,23 @@ impl PlayState {
                     }
 
                     self.have_incr_play_count = false;
+
+                    // We are now playing something else, as such the previous one must have
+                    // finished or was skipped.
+                    true
                 } else if last.elapsed > curr.elapsed
                     && self.have_incr_play_count
                     && curr.elapsed / curr.duration <= 0.1
                 {
                     debug!("Re-play-- resetting PC incremented flag.");
                     self.have_incr_play_count = false;
+
+                    // We are still playing the same song, just skipped at the start again.
+                    // This means that we don't need a new one.
+                    false
+                } else {
+                    // We are still playing the same song, so nothing changed
+                    false
                 }
             }
             (PlayerStatus::Stopped, PlayerStatus::Play(_))
@@ -106,20 +120,29 @@ impl PlayState {
             | (PlayerStatus::Pause(_), PlayerStatus::Stopped)
             | (PlayerStatus::Play(_), PlayerStatus::Stopped) => {
                 self.have_incr_play_count = false;
+
+                // We played and stopped or stopped and play now so we did probably not change the
+                // song?
+                false
             }
-            (PlayerStatus::Stopped, PlayerStatus::Stopped) => (),
-        }
+            (PlayerStatus::Stopped, PlayerStatus::Stopped) => {
+                // We did not play before and we are still not playing, as such nothing really
+                // changed.
+                false
+            },
+        };
 
         match &new_stat {
             PlayerStatus::Play(curr) => {
                 let pct = curr.played_pct();
                 debug!("Updating status: {:.3}% complete.", 100.0 * pct);
+
                 if !self.have_incr_play_count && pct >= self.played_thresh {
                     info!(
-                        "Increment play count for '{}' (songid: {}) at {} played.",
+                        "Increment play count for '{}' (songid: {}) at {:.2}% played.",
                         curr.file.display(),
                         curr.songid,
-                        curr.elapsed / curr.duration
+                        (curr.elapsed / curr.duration) * 100.0
                     );
 
                     let file = curr.file.to_str().ok_or_else(|| {
@@ -150,10 +173,10 @@ impl PlayState {
                         .expect("To exist, as it was skipped");
 
                     info!(
-                        "Marking '{}' (songid: {}) as skipped at {}.",
+                        "Marking '{}' (songid: {}) as skipped at {:.2}%.",
                         last.file.display(),
                         last.songid,
-                        last.elapsed / last.duration
+                        (last.elapsed / last.duration) * 100.0
                     );
 
                     let file = last.file.to_str().ok_or_else(|| {
@@ -168,7 +191,7 @@ impl PlayState {
         };
 
         self.last_server_stat = new_stat;
-        Ok(()) // No need to update the DB
+        Ok(previous_song_finished)
     }
 }