Repository: aome510/spotify-player (language: rust)

Issue:
Fake track repeat
**Is your feature request related to a problem? Please describe.**
I want to play a song on loop.

**Describe the solution you'd like**
(when using the local device) to fake repeat the track by re-playing it after it finishes when the loop on single track is enabled.

**Describe alternatives you've considered**
A clear and concise description of any alternative solutions or features you've considered.

**Additional context**
Currently, Librespot does not support looping a single track once.


Discussion / hints:
I believe to make this possible, it would require a custom new 'playback' mode that is not available to Spotify API, as 'repeated once track' behavior is going to be inferred from spotify_player AFAIK.
For what it's worth, `ncspot` (another Rust/`librespot` based Spotify TUI client) seems to have figured out a hack, although what distinguishes it from this project isn't immediately obvious. I'll try and dig deeper if I can find the time.
> For what it's worth, `ncspot` (another Rust/`librespot` based Spotify TUI client) seems to have figured out a hack, although what distinguishes it from this project isn't immediately obvious. I'll try and dig deeper if I can find the time.

The main difference is that `ncspot` itself is an Spotify client (for streaming), while `spotify_player` uses an integrated client from [librespot](https://github.com/librespot-org/librespot). In other words, it's easier for `ncspot` to implement the hack as they own the client implementation. 
Is it that difficult to just check if the user has 'repeat once' and just replay the song after it ends? Or am I underestimating how difficult it is?
`spotify_player` operates on API boundary and streaming client is just an "add-on" compared to `ncspot` with streaming client as the main component. As a result, the app cannot have any assumption regarding the streaming client's behaviour.

> Is it that difficult to just check if the user has 'repeat once' and just replay the song after it ends?

The problem is that you don't know when user has 'repeat once'. The integrated streaming client will automatically switch to `repeat context` mode as `repeat once` is not supported. As a result, when you query `get-repeat-mode` API, the returned mode will be `repeat context`.

There are different ways to implement the "hack". The easiest one is to start a new "one-track" playback if user wants to change the repeat mode to `repeat once`. The drawback of this approach is that you will lose the previous context, or you cannot play the previous context when switching to different repeat mode.

Another approach is to add a separate `fake track repeat` mode and a command to enable/disable the mode. The integrated client can be updated to queue the current song when it is about to end. I would prefer this approach as a workaround for the integrated client's [limitation](https://github.com/librespot-org/librespot/issues/19). Can start working on this when I'm free.



Fix patch:
diff --git a/README.md b/README.md
index c975d2e4..4fda4667 100644
--- a/README.md
+++ b/README.md
@@ -309,6 +309,7 @@ List of supported commands:
 | `ResumePause`                  | resume/pause based on the current playback                              | `space`            |
 | `PlayRandom`                   | play a random track in the current context                              | `.`                |
 | `Repeat`                       | cycle the repeat mode                                                   | `C-r`              |
+| `ToggleFakeTrackRepeatMode`    | toggle fake track repeat mode                                           | `M-r`              |
 | `Shuffle`                      | toggle the shuffle mode                                                 | `C-s`              |
 | `VolumeUp`                     | increase playback volume by 5%                                          | `+`                |
 | `VolumeDown`                   | decrease playback volume by 5%                                          | `-`                |
diff --git a/spotify_player/src/client/handlers.rs b/spotify_player/src/client/handlers.rs
index 4c4ec2c2..91574338 100644
--- a/spotify_player/src/client/handlers.rs
+++ b/spotify_player/src/client/handlers.rs
@@ -1,3 +1,4 @@
+use anyhow::Context;
 use rspotify::model::PlayableItem;
 use tracing::Instrument;
 
@@ -6,6 +7,10 @@ use crate::{event::ClientRequest, state::*};
 #[cfg(feature = "lyric-finder")]
 use crate::utils::map_join;
 
+struct PlayerEventHandlerState {
+    add_track_to_queue_req_timer: std::time::Instant,
+}
+
 /// starts the client's request handler
 pub async fn start_client_handler(
     state: SharedState,
@@ -36,60 +41,146 @@ pub async fn start_client_handler(
     }
 }
 
-async fn handle_track_end_event(
+fn handle_playback_change_event(
     state: &SharedState,
     client_pub: &flume::Sender<ClientRequest>,
+    handler_state: &mut PlayerEventHandlerState,
 ) -> anyhow::Result<()> {
-    let mut needs_update = false;
-    {
-        let player = state.player.read();
-        if let (Some(playback), Some(track)) =
-            (player.playback.as_ref(), player.current_playing_track())
-        {
-            if let Some(progress) = player.playback_progress() {
-                needs_update = progress >= track.duration && playback.is_playing;
+    let player = state.player.read();
+    let (playback, track) = match (
+        player.buffered_playback.as_ref(),
+        player.current_playing_track(),
+    ) {
+        (Some(playback), Some(track)) => (playback, track),
+        _ => return Ok(()),
+    };
+
+    if let Some(progress) = player.playback_progress() {
+        // update the playback when the current track ends
+        if progress >= track.duration && playback.is_playing {
+            client_pub.send(ClientRequest::GetCurrentPlayback)?;
+        }
+    }
+
+    if let Some(queue) = player.queue.as_ref() {
+        // queue needs to be updated if its playing track is different from actual playback's playing track
+        if let Some(PlayableItem::Track(queue_track)) = queue.currently_playing.as_ref() {
+            if queue_track.id != track.id {
+                client_pub.send(ClientRequest::GetCurrentUserQueue)?;
             }
         }
+    } else {
+        client_pub.send(ClientRequest::GetCurrentUserQueue)?;
     }
-    if needs_update {
-        client_pub
-            .send_async(ClientRequest::GetCurrentPlayback)
-            .await?;
+
+    // handle fake track repeat mode
+    if playback.fake_track_repeat_state {
+        if let Some(progress) = player.playback_progress() {
+            // re-queue the current track if it's about to end while
+            // ensuring that only one `AddTrackToQueue` request is made
+            if progress + chrono::TimeDelta::seconds(5) >= track.duration
+                && playback.is_playing
+                && handler_state.add_track_to_queue_req_timer.elapsed()
+                    > std::time::Duration::from_secs(10)
+            {
+                tracing::info!(
+                    "fake track repeat mode is enabled, add the current track ({}) to queue",
+                    track.name
+                );
+                client_pub.send(ClientRequest::AddTrackToQueue(track.id.clone().unwrap()))?;
+                handler_state.add_track_to_queue_req_timer = std::time::Instant::now();
+            }
+        }
     }
 
     Ok(())
 }
 
-async fn handle_queue_change_event(
+fn handle_page_change_event(
     state: &SharedState,
     client_pub: &flume::Sender<ClientRequest>,
 ) -> anyhow::Result<()> {
-    let mut needs_update = false;
-    {
-        let player = state.player.read();
-        if let (Some(track), Some(queue)) = (player.current_playing_track(), player.queue.as_ref())
-        {
-            if let Some(PlayableItem::Track(queue_track)) = queue.currently_playing.as_ref() {
-                // if the currently playing track in queue is different from the actual
-                // currently playing track stored inside player's playback, update the queue
-                needs_update = queue_track.id != track.id;
+    match state.ui.lock().current_page_mut() {
+        PageState::Context {
+            id,
+            context_page_type,
+            state: page_state,
+        } => {
+            let expected_id = match context_page_type {
+                ContextPageType::Browsing(context_id) => Some(context_id.clone()),
+                ContextPageType::CurrentPlaying => state.player.read().playing_context_id(),
+            };
+
+            // update the context state and request new data when moving to a new context page
+            if *id != expected_id {
+                tracing::info!("Current context ID ({:?}) is different from the expected ID ({:?}), update the context state", id, expected_id);
+
+                *id = expected_id;
+
+                // update the UI page state based on the context's type
+                match id {
+                    Some(id) => {
+                        *page_state = Some(match id {
+                            ContextId::Album(_) => ContextPageUIState::new_album(),
+                            ContextId::Artist(_) => ContextPageUIState::new_artist(),
+                            ContextId::Playlist(_) => ContextPageUIState::new_playlist(),
+                            ContextId::Tracks(_) => ContextPageUIState::new_tracks(),
+                        });
+                    }
+                    None => {
+                        *page_state = None;
+                    }
+                }
+            }
+
+            // request new context's data if not found in memory
+            if let Some(id) = id {
+                if !matches!(id, ContextId::Tracks(_))
+                    && !state.data.read().caches.context.contains_key(&id.uri())
+                {
+                    client_pub.send(ClientRequest::GetContext(id.clone()))?;
+                }
             }
         }
-    }
-    if needs_update {
-        // In addition to `GetCurrentUserQueue` request, also update the current playback
-        // as there can be a mismatch between the current playback and the current queue.
-        client_pub
-            .send_async(ClientRequest::GetCurrentPlayback)
-            .await?;
-        client_pub
-            .send_async(ClientRequest::GetCurrentUserQueue)
-            .await?;
+
+        #[cfg(feature = "lyric-finder")]
+        PageState::Lyric {
+            track,
+            artists,
+            scroll_offset,
+        } => {
+            if let Some(current_track) = state.player.read().current_playing_track() {
+                if current_track.name != *track {
+                    tracing::info!("Current playing track \"{}\" is different from the track \"{track}\" shown up in the lyric page. Updating the track and fetching its lyric...", current_track.name);
+                    *track = current_track.name.clone();
+                    *artists = map_join(&current_track.artists, |a| &a.name, ", ");
+                    *scroll_offset = 0;
+
+                    client_pub.send(ClientRequest::GetLyric {
+                        track: track.clone(),
+                        artists: artists.clone(),
+                    })?;
+                }
+            }
+        }
+        _ => {}
     }
 
     Ok(())
 }
 
+fn handle_player_event(
+    state: &SharedState,
+    client_pub: &flume::Sender<ClientRequest>,
+    handler_state: &mut PlayerEventHandlerState,
+) -> anyhow::Result<()> {
+    handle_page_change_event(state, client_pub).context("handle page change event")?;
+    handle_playback_change_event(state, client_pub, handler_state)
+        .context("handle playback change event")?;
+
+    Ok(())
+}
+
 /// Starts multiple event watchers listening to events and
 /// notifying the client to make update requests if needed
 pub async fn start_player_event_watchers(
@@ -116,109 +207,15 @@ pub async fn start_player_event_watchers(
         });
     }
 
-    // Start the first task for handling "low-frequency" events.
-    // An event can be categorized as "low-frequency" when
-    // - we don't need to handle it "immediately"
-    // - we want to avoid handling it more than once within a period of time
-    tokio::task::spawn({
-        let client_pub = client_pub.clone();
-        let state = state.clone();
-        async move {
-            let refresh_duration = std::time::Duration::from_millis(1000); // frequency = 1Hz
-            loop {
-                tokio::time::sleep(refresh_duration).await;
-
-                if let Err(err) = handle_track_end_event(&state, &client_pub).await {
-                    tracing::error!("Encountered error when handling track end event: {err:#}");
-                }
-                if let Err(err) = handle_queue_change_event(&state, &client_pub).await {
-                    tracing::error!("Encountered error when handling queue change event: {err:#}");
-                }
-            }
-        }
-    });
-
-    // Start the second task (main blocking task) for handling "high-frequency" events.
-    // An event is categorized as "high-frequency" when
-    // - we want to handle it "immediately" to prevent users from experiencing a noticeable delay
-    let refresh_duration = std::time::Duration::from_millis(200); // frequency = 5Hz
-
-    // This timer is used to avoid making multiple `GetContext` requests in a short period of time.
-    let mut last_request_timer = std::time::Instant::now();
+    let refresh_duration = std::time::Duration::from_secs(1);
+    let mut handler_state = PlayerEventHandlerState {
+        add_track_to_queue_req_timer: std::time::Instant::now(),
+    };
 
     loop {
         tokio::time::sleep(refresh_duration).await;
-
-        match state.ui.lock().current_page_mut() {
-            PageState::Context {
-                id,
-                context_page_type,
-                state: page_state,
-            } => {
-                let expected_id = match context_page_type {
-                    ContextPageType::Browsing(context_id) => Some(context_id.clone()),
-                    ContextPageType::CurrentPlaying => state.player.read().playing_context_id(),
-                };
-
-                // update the context state and request new data when moving to a new context page
-                if *id != expected_id {
-                    tracing::info!("Current context ID ({:?}) is different from the expected ID ({:?}), update the context state", id, expected_id);
-
-                    *id = expected_id;
-
-                    // update the UI page state based on the context's type
-                    match id {
-                        Some(id) => {
-                            *page_state = Some(match id {
-                                ContextId::Album(_) => ContextPageUIState::new_album(),
-                                ContextId::Artist(_) => ContextPageUIState::new_artist(),
-                                ContextId::Playlist(_) => ContextPageUIState::new_playlist(),
-                                ContextId::Tracks(_) => ContextPageUIState::new_tracks(),
-                            });
-                        }
-                        None => {
-                            *page_state = None;
-                        }
-                    }
-                }
-
-                // request new context's data if not found in memory
-                if let Some(id) = id {
-                    if !matches!(id, ContextId::Tracks(_))
-                        && !state.data.read().caches.context.contains_key(&id.uri())
-                        && last_request_timer.elapsed() >= std::time::Duration::from_secs(1)
-                    {
-                        last_request_timer = std::time::Instant::now();
-                        client_pub
-                            .send(ClientRequest::GetContext(id.clone()))
-                            .unwrap_or_default();
-                    }
-                }
-            }
-
-            #[cfg(feature = "lyric-finder")]
-            PageState::Lyric {
-                track,
-                artists,
-                scroll_offset,
-            } => {
-                if let Some(current_track) = state.player.read().current_playing_track() {
-                    if current_track.name != *track {
-                        tracing::info!("Current playing track \"{}\" is different from the track \"{track}\" shown up in the lyric page. Updating the track and fetching its lyric...", current_track.name);
-                        *track = current_track.name.clone();
-                        *artists = map_join(&current_track.artists, |a| &a.name, ", ");
-                        *scroll_offset = 0;
-
-                        client_pub
-                            .send(ClientRequest::GetLyric {
-                                track: track.clone(),
-                                artists: artists.clone(),
-                            })
-                            .unwrap_or_default();
-                    }
-                }
-            }
-            _ => {}
+        if let Err(err) = handle_player_event(&state, &client_pub, &mut handler_state) {
+            tracing::error!("Encounter error when handling player event: {err:#}");
         }
     }
 }
diff --git a/spotify_player/src/client/mod.rs b/spotify_player/src/client/mod.rs
index 2765a0af..854c1493 100644
--- a/spotify_player/src/client/mod.rs
+++ b/spotify_player/src/client/mod.rs
@@ -209,9 +209,6 @@ impl Client {
                 self.spotify
                     .shuffle(playback.shuffle_state, device_id)
                     .await?;
-
-                // after handling `StartPlayback` request, reset the buffered playback
-                return Ok(None);
             }
             PlayerRequest::TransferPlayback(..) => {
                 anyhow::bail!("`TransferPlayback` should be handled earlier")
@@ -439,9 +436,7 @@ impl Client {
             }
             ClientRequest::GetCurrentUserQueue => {
                 let queue = self.spotify.current_user_queue().await?;
-                {
-                    state.player.write().queue = Some(queue);
-                }
+                state.player.write().queue = Some(queue);
             }
             ClientRequest::ReorderPlaylistItems {
                 playlist_id,
@@ -541,7 +536,8 @@ impl Client {
         // After handling a request that updates the player's playback,
         // update the playback state by making additional refresh requests.
         //
-        // # Why needs more than one request to update the playback?
+        // Why needs more than one request to update the playback?
+        //
         // It may take a while for Spotify to update the new change,
         // making additional requests can help ensure that
         // the playback state is always in sync with the latest change.
@@ -1374,15 +1370,18 @@ impl Client {
             };
 
             if reset_buffered_playback || needs_update {
-                // new playback updates, the buffered playback becomes invalid and needs to be updated
                 player.buffered_playback = player.playback.as_ref().map(|p| {
                     let mut playback = SimplifiedPlayback::from_playback(p);
-                    // update buffered playback's mute state and volume
-                    let mute_state = player.buffered_playback.as_ref().and_then(|p| p.mute_state);
-                    if let Some(volume) = mute_state {
-                        playback.volume = Some(volume);
+
+                    // handle additional data from the previous buffered state
+                    // that is not available in a standard Spotify playback's state
+                    if let Some(bp) = &player.buffered_playback {
+                        if let Some(volume) = bp.mute_state {
+                            playback.volume = Some(volume);
+                        }
+                        playback.mute_state = bp.mute_state;
+                        playback.fake_track_repeat_state = bp.fake_track_repeat_state;
                     }
-                    playback.mute_state = mute_state;
                     playback
                 });
             }
diff --git a/spotify_player/src/command.rs b/spotify_player/src/command.rs
index f644a318..887a0426 100644
--- a/spotify_player/src/command.rs
+++ b/spotify_player/src/command.rs
@@ -11,6 +11,7 @@ pub enum Command {
     ResumePause,
     PlayRandom,
     Repeat,
+    ToggleFakeTrackRepeatMode,
     Shuffle,
     VolumeUp,
     VolumeDown,
@@ -200,6 +201,7 @@ impl Command {
             Self::ResumePause => "resume/pause based on the current playback",
             Self::PlayRandom => "play a random track in the current context",
             Self::Repeat => "cycle the repeat mode",
+            Self::ToggleFakeTrackRepeatMode => "toggle fake track repeat mode",
             Self::Shuffle => "toggle the shuffle mode",
             Self::VolumeUp => "increase playback volume by 5%",
             Self::VolumeDown => "decrease playback volume by 5%",
diff --git a/spotify_player/src/config/keymap.rs b/spotify_player/src/config/keymap.rs
index d7abfb76..c525ab6b 100644
--- a/spotify_player/src/config/keymap.rs
+++ b/spotify_player/src/config/keymap.rs
@@ -43,6 +43,10 @@ impl Default for KeymapConfig {
                     key_sequence: "C-r".into(),
                     command: Command::Repeat,
                 },
+                Keymap {
+                    key_sequence: "M-r".into(),
+                    command: Command::ToggleFakeTrackRepeatMode,
+                },
                 Keymap {
                     key_sequence: "C-s".into(),
                     command: Command::Shuffle,
diff --git a/spotify_player/src/event/mod.rs b/spotify_player/src/event/mod.rs
index cb769e11..a870615d 100644
--- a/spotify_player/src/event/mod.rs
+++ b/spotify_player/src/event/mod.rs
@@ -233,6 +233,12 @@ fn handle_global_command(
         Command::Repeat => {
             client_pub.send(ClientRequest::Player(PlayerRequest::Repeat))?;
         }
+        Command::ToggleFakeTrackRepeatMode => {
+            let mut player = state.player.write();
+            if let Some(playback) = &mut player.buffered_playback {
+                playback.fake_track_repeat_state = !playback.fake_track_repeat_state;
+            }
+        }
         Command::Shuffle => {
             client_pub.send(ClientRequest::Player(PlayerRequest::Shuffle))?;
         }
diff --git a/spotify_player/src/state/model.rs b/spotify_player/src/state/model.rs
index 05013a99..1de9a890 100644
--- a/spotify_player/src/state/model.rs
+++ b/spotify_player/src/state/model.rs
@@ -103,6 +103,11 @@ pub struct SimplifiedPlayback {
     pub repeat_state: rspotify_model::RepeatState,
     pub shuffle_state: bool,
     pub mute_state: Option<u32>,
+    /// Indicate if fake track repeat mode is enabled.
+    /// This mode is a workaround for a librespot's [limitation] that doesn't support `track` repeat.
+    ///
+    /// [limitation]: https://github.com/librespot-org/librespot/issues/19
+    pub fake_track_repeat_state: bool,
 }
 
 #[derive(Debug, Clone)]
@@ -482,6 +487,7 @@ impl SimplifiedPlayback {
             repeat_state: p.repeat_state,
             shuffle_state: p.shuffle_state,
             mute_state: None,
+            fake_track_repeat_state: false,
         }
     }
 }
diff --git a/spotify_player/src/ui/playback.rs b/spotify_player/src/ui/playback.rs
index 258a1add..090c20cc 100644
--- a/spotify_player/src/ui/playback.rs
+++ b/spotify_player/src/ui/playback.rs
@@ -161,12 +161,6 @@ fn render_playback_text(
     // this regex is to handle a format argument or a newline
     let re = regex::Regex::new(r"\{.*?\}|\n").unwrap();
 
-    // build the volume string (vol% when unmuted, old_vol% (muted) if currently muted)
-    let volume = match playback.mute_state {
-        Some(volume) => format!("{volume}% (muted)"),
-        None => format!("{}%", playback.volume.unwrap_or_default()),
-    };
-
     let mut ptr = 0;
     for m in re.find_iter(format_str) {
         let s = m.start();
@@ -208,9 +202,16 @@ fn render_playback_text(
             "{metadata}" => (
                 format!(
                     "repeat: {} | shuffle: {} | volume: {} | device: {}",
-                    <&'static str>::from(playback.repeat_state),
+                    if playback.fake_track_repeat_state {
+                        "track (fake)"
+                    } else {
+                        <&'static str>::from(playback.repeat_state)
+                    },
                     playback.shuffle_state,
-                    volume,
+                    match playback.mute_state {
+                        Some(volume) => format!("{volume}% (muted)"),
+                        None => format!("{}%", playback.volume.unwrap_or_default()),
+                    },
                     playback.device_name,
                 ),
                 ui.theme.playback_metadata(),


Test patch: