From b63199743aef01868a67be451e6d46a288264235 Mon Sep 17 00:00:00 2001 From: ashthespy Date: Sat, 9 May 2020 13:59:28 +0200 Subject: [PATCH 1/8] Skip unplayable tracks instead of stopping --- playback/src/player.rs | 65 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/playback/src/player.rs b/playback/src/player.rs index a26d358..0a6fd41 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -378,6 +378,7 @@ impl PlayerState { } } + #[allow(dead_code)] fn is_stopped(&self) -> bool { use self::PlayerState::*; match *self { @@ -386,6 +387,14 @@ impl PlayerState { } } + fn is_loading(&self) -> bool { + use self::PlayerState::*; + match *self { + Loading { .. } => true, + _ => false, + } + } + fn decoder(&mut self) -> Option<&mut Decoder> { use self::PlayerState::*; match *self { @@ -728,8 +737,14 @@ impl Future for PlayerInternal { } Ok(Async::NotReady) => (), Err(_) => { - self.handle_player_stop(); - assert!(self.state.is_stopped()); + warn!("Unable to load <{:?}>", track_id); + warn!("Skipping to next track"); + trace!("State: {:?}", self.state); + assert!(self.state.is_loading()); + self.send_event(PlayerEvent::EndOfTrack { + track_id, + play_request_id, + }) } } } @@ -749,6 +764,7 @@ impl Future for PlayerInternal { } Ok(Async::NotReady) => (), Err(_) => { + warn!("Unable to preload {:?}", track_id); self.preload = PlayerPreload::None; } } @@ -1523,6 +1539,51 @@ impl ::std::fmt::Debug for PlayerCommand { } } +impl ::std::fmt::Debug for PlayerState { + fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result { + use PlayerState::*; + match *self { + Stopped => f.debug_struct("Stopped").finish(), + Loading { + track_id, + play_request_id, + .. + } => f + .debug_struct("Loading") + .field("track_id", &track_id) + .field("play_request_id", &play_request_id) + .finish(), + Paused { + track_id, + play_request_id, + .. + } => f + .debug_struct("Paused") + .field("track_id", &track_id) + .field("play_request_id", &play_request_id) + .finish(), + Playing { + track_id, + play_request_id, + .. + } => f + .debug_struct("Playing") + .field("track_id", &track_id) + .field("play_request_id", &play_request_id) + .finish(), + EndOfTrack { + track_id, + play_request_id, + .. + } => f + .debug_struct("EndOfTrack") + .field("track_id", &track_id) + .field("play_request_id", &play_request_id) + .finish(), + Invalid => f.debug_struct("Invalid").finish(), + } + } +} struct Subfile { stream: T, offset: u64, From 902440925d2c4b334d7b72fe64b198ef377692f8 Mon Sep 17 00:00:00 2001 From: ashthespy Date: Sun, 10 May 2020 14:31:43 +0200 Subject: [PATCH 2/8] Handle unplayable tracks during prefetch --- connect/src/spirc.rs | 46 ++++++++++++++++++++++++------------------ playback/src/player.rs | 42 ++++++++++++++++++++++++++++++++++---- 2 files changed, 64 insertions(+), 24 deletions(-) diff --git a/connect/src/spirc.rs b/connect/src/spirc.rs index 453327c..06bf37f 100644 --- a/connect/src/spirc.rs +++ b/connect/src/spirc.rs @@ -621,24 +621,26 @@ impl SpircTask { self.play_status = SpircPlayStatus::Stopped; } }, - PlayerEvent::TimeToPreloadNextTrack { .. } => match self.play_status { - SpircPlayStatus::Paused { - ref mut preloading_of_next_track_triggered, - .. - } - | SpircPlayStatus::Playing { - ref mut preloading_of_next_track_triggered, - .. - } => { - *preloading_of_next_track_triggered = true; - if let Some(track_id) = self.preview_next_track() { - self.player.preload(track_id); + PlayerEvent::TimeToPreloadNextTrack { preload_index, .. } => { + match self.play_status { + SpircPlayStatus::Paused { + ref mut preloading_of_next_track_triggered, + .. } + | SpircPlayStatus::Playing { + ref mut preloading_of_next_track_triggered, + .. + } => { + *preloading_of_next_track_triggered = true; + if let Some(track_id) = self.preview_next_track(preload_index) { + self.player.preload(track_id); + } + } + SpircPlayStatus::LoadingPause { .. } + | SpircPlayStatus::LoadingPlay { .. } + | SpircPlayStatus::Stopped => (), } - SpircPlayStatus::LoadingPause { .. } - | SpircPlayStatus::LoadingPlay { .. } - | SpircPlayStatus::Stopped => (), - }, + } _ => (), } } @@ -777,7 +779,8 @@ impl SpircTask { } = self.play_status { if preloading_of_next_track_triggered { - if let Some(track_id) = self.preview_next_track() { + // Get the next track_id in the playlist + if let Some(track_id) = self.preview_next_track(1) { self.player.preload(track_id); } } @@ -899,9 +902,12 @@ impl SpircTask { } } - fn preview_next_track(&mut self) -> Option { - self.get_track_id_to_play_from_playlist(self.state.get_playing_track_index() + 1) - .and_then(|(track_id, _)| Some(track_id)) + fn preview_next_track(&mut self, preview_index: u32) -> Option { + trace!("Previewing {:}", preview_index); + self.get_track_id_to_play_from_playlist( + self.state.get_playing_track_index() + preview_index, + ) + .and_then(|(track_id, _)| Some(track_id)) } fn handle_next(&mut self) { diff --git a/playback/src/player.rs b/playback/src/player.rs index 0a6fd41..4ea5041 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -99,6 +99,7 @@ pub enum PlayerEvent { TimeToPreloadNextTrack { play_request_id: u64, track_id: SpotifyId, + preload_index: u32, }, EndOfTrack { play_request_id: u64, @@ -320,6 +321,10 @@ enum PlayerPreload { Loading { track_id: SpotifyId, loader: Box>, + preload_index: u32, + }, + Missing { + preload_index: u32, }, Ready { track_id: SpotifyId, @@ -753,6 +758,7 @@ impl Future for PlayerInternal { if let PlayerPreload::Loading { ref mut loader, track_id, + preload_index, } = self.preload { match loader.poll() { @@ -764,8 +770,26 @@ impl Future for PlayerInternal { } Ok(Async::NotReady) => (), Err(_) => { - warn!("Unable to preload {:?}", track_id); - self.preload = PlayerPreload::None; + warn!("Unable to preload {:?}[{}]", track_id, preload_index,); + // Preemptively fetch next track? + self.preload = PlayerPreload::Missing { preload_index }; + if let PlayerState::Playing { + play_request_id, .. + } + | PlayerState::Paused { + play_request_id, .. + } = self.state + { + debug!( + "Requesting track_id for preload_index: {}", + preload_index + 1 + ); + self.send_event(PlayerEvent::TimeToPreloadNextTrack { + track_id, + play_request_id, + preload_index: preload_index + 1, + }); + } } } } @@ -853,6 +877,7 @@ impl Future for PlayerInternal { self.send_event(PlayerEvent::TimeToPreloadNextTrack { track_id, play_request_id, + preload_index: 1, }); } } @@ -1294,7 +1319,12 @@ impl PlayerInternal { fn handle_command_preload(&mut self, track_id: SpotifyId) { debug!("Preloading track"); let mut preload_track = true; - + let preload_index = match self.preload { + PlayerPreload::Loading { preload_index, .. } => preload_index, + // The last preload was missing, so increment it + PlayerPreload::Missing { preload_index, .. } => preload_index + 1, + _ => 1, + }; // check whether the track is already loaded somewhere or being loaded. if let PlayerPreload::Loading { track_id: currently_loading, @@ -1336,7 +1366,11 @@ impl PlayerInternal { // schedule the preload if the current track if desired. if preload_track { let loader = self.load_track(track_id, 0); - self.preload = PlayerPreload::Loading { track_id, loader } + self.preload = PlayerPreload::Loading { + track_id, + loader, + preload_index, + } } } From 14709b9f8deeba75f7de808dc3421f962a1a49c9 Mon Sep 17 00:00:00 2001 From: ashthespy Date: Wed, 13 May 2020 11:49:26 +0200 Subject: [PATCH 3/8] Let spirc handle unavailable tracks --- connect/src/spirc.rs | 80 ++++++++++++++++++++++++++++-------------- playback/src/player.rs | 43 +++++++---------------- 2 files changed, 66 insertions(+), 57 deletions(-) diff --git a/connect/src/spirc.rs b/connect/src/spirc.rs index 06bf37f..76ce456 100644 --- a/connect/src/spirc.rs +++ b/connect/src/spirc.rs @@ -621,26 +621,8 @@ impl SpircTask { self.play_status = SpircPlayStatus::Stopped; } }, - PlayerEvent::TimeToPreloadNextTrack { preload_index, .. } => { - match self.play_status { - SpircPlayStatus::Paused { - ref mut preloading_of_next_track_triggered, - .. - } - | SpircPlayStatus::Playing { - ref mut preloading_of_next_track_triggered, - .. - } => { - *preloading_of_next_track_triggered = true; - if let Some(track_id) = self.preview_next_track(preload_index) { - self.player.preload(track_id); - } - } - SpircPlayStatus::LoadingPause { .. } - | SpircPlayStatus::LoadingPlay { .. } - | SpircPlayStatus::Stopped => (), - } - } + PlayerEvent::TimeToPreloadNextTrack { .. } => self.handle_preload_next_track(), + PlayerEvent::Unavailable { track_id, .. } => self.handle_unavalable(track_id), _ => (), } } @@ -780,7 +762,7 @@ impl SpircTask { { if preloading_of_next_track_triggered { // Get the next track_id in the playlist - if let Some(track_id) = self.preview_next_track(1) { + if let Some(track_id) = self.preview_next_track() { self.player.preload(track_id); } } @@ -902,12 +884,44 @@ impl SpircTask { } } - fn preview_next_track(&mut self, preview_index: u32) -> Option { - trace!("Previewing {:}", preview_index); - self.get_track_id_to_play_from_playlist( - self.state.get_playing_track_index() + preview_index, - ) - .and_then(|(track_id, _)| Some(track_id)) + fn preview_next_track(&mut self) -> Option { + self.get_track_id_to_play_from_playlist(self.state.get_playing_track_index() + 1) + .and_then(|(track_id, _)| Some(track_id)) + } + + fn handle_preload_next_track(&mut self) { + // Requests the player thread to preload the next track + match self.play_status { + SpircPlayStatus::Paused { + ref mut preloading_of_next_track_triggered, + .. + } + | SpircPlayStatus::Playing { + ref mut preloading_of_next_track_triggered, + .. + } => { + *preloading_of_next_track_triggered = true; + if let Some(track_id) = self.preview_next_track() { + self.player.preload(track_id); + } + } + SpircPlayStatus::LoadingPause { .. } + | SpircPlayStatus::LoadingPlay { .. } + | SpircPlayStatus::Stopped => (), + } + } + + fn handle_unavalable(&mut self, track_id: SpotifyId) { + let unavalable_index = self.get_track_index_for_spotify_id( + &track_id, + self.state.get_playing_track_index() as usize, + ); + if let Some(index) = unavalable_index { + // TODO: Or mark it as NonPlayable? + debug!("Removing unavailable track_ref at {:?}", index); + self.state.mut_track().remove(index); + } + self.handle_preload_next_track(); } fn handle_next(&mut self) { @@ -1141,6 +1155,18 @@ impl SpircTask { }) } + fn get_track_index_for_spotify_id( + &self, + track_id: &SpotifyId, + start_index: usize, + ) -> Option { + let index = self.state.get_track()[start_index..] + .iter() + .position(|track_ref| self.get_spotify_id_for_track(track_ref) == Ok(*track_id)); + + index + } + fn get_track_id_to_play_from_playlist(&self, index: u32) -> Option<(SpotifyId, u32)> { let tracks_len = self.state.get_track().len() as u32; diff --git a/playback/src/player.rs b/playback/src/player.rs index 4ea5041..ef57607 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -99,7 +99,10 @@ pub enum PlayerEvent { TimeToPreloadNextTrack { play_request_id: u64, track_id: SpotifyId, - preload_index: u32, + }, + Unavailable { + play_request_id: u64, + track_id: SpotifyId, }, EndOfTrack { play_request_id: u64, @@ -117,6 +120,9 @@ impl PlayerEvent { Loading { play_request_id, .. } + | Unavailable { + play_request_id, .. + } | Started { play_request_id, .. } @@ -321,10 +327,6 @@ enum PlayerPreload { Loading { track_id: SpotifyId, loader: Box>, - preload_index: u32, - }, - Missing { - preload_index: u32, }, Ready { track_id: SpotifyId, @@ -742,9 +744,7 @@ impl Future for PlayerInternal { } Ok(Async::NotReady) => (), Err(_) => { - warn!("Unable to load <{:?}>", track_id); - warn!("Skipping to next track"); - trace!("State: {:?}", self.state); + warn!("Unable to load <{:?}>\nSkipping to next track", track_id); assert!(self.state.is_loading()); self.send_event(PlayerEvent::EndOfTrack { track_id, @@ -758,7 +758,6 @@ impl Future for PlayerInternal { if let PlayerPreload::Loading { ref mut loader, track_id, - preload_index, } = self.preload { match loader.poll() { @@ -770,9 +769,9 @@ impl Future for PlayerInternal { } Ok(Async::NotReady) => (), Err(_) => { - warn!("Unable to preload {:?}[{}]", track_id, preload_index,); - // Preemptively fetch next track? - self.preload = PlayerPreload::Missing { preload_index }; + debug!("Unable to preload {:?}", track_id); + self.preload = PlayerPreload::None; + // Let Spirc know that the track was unavailable. if let PlayerState::Playing { play_request_id, .. } @@ -780,14 +779,9 @@ impl Future for PlayerInternal { play_request_id, .. } = self.state { - debug!( - "Requesting track_id for preload_index: {}", - preload_index + 1 - ); - self.send_event(PlayerEvent::TimeToPreloadNextTrack { + self.send_event(PlayerEvent::Unavailable { track_id, play_request_id, - preload_index: preload_index + 1, }); } } @@ -877,7 +871,6 @@ impl Future for PlayerInternal { self.send_event(PlayerEvent::TimeToPreloadNextTrack { track_id, play_request_id, - preload_index: 1, }); } } @@ -1319,12 +1312,6 @@ impl PlayerInternal { fn handle_command_preload(&mut self, track_id: SpotifyId) { debug!("Preloading track"); let mut preload_track = true; - let preload_index = match self.preload { - PlayerPreload::Loading { preload_index, .. } => preload_index, - // The last preload was missing, so increment it - PlayerPreload::Missing { preload_index, .. } => preload_index + 1, - _ => 1, - }; // check whether the track is already loaded somewhere or being loaded. if let PlayerPreload::Loading { track_id: currently_loading, @@ -1366,11 +1353,7 @@ impl PlayerInternal { // schedule the preload if the current track if desired. if preload_track { let loader = self.load_track(track_id, 0); - self.preload = PlayerPreload::Loading { - track_id, - loader, - preload_index, - } + self.preload = PlayerPreload::Loading { track_id, loader } } } From 9cacc2d09fd08bb132a250df51d1f77f9315f15f Mon Sep 17 00:00:00 2001 From: ashthespy Date: Wed, 13 May 2020 13:24:30 +0200 Subject: [PATCH 4/8] Fix regression in track cycling --- connect/src/spirc.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/connect/src/spirc.rs b/connect/src/spirc.rs index aa7aae6..ececdba 100644 --- a/connect/src/spirc.rs +++ b/connect/src/spirc.rs @@ -1170,9 +1170,9 @@ impl SpircTask { } fn get_track_id_to_play_from_playlist(&self, index: u32) -> Option<(SpotifyId, u32)> { - let tracks_len = self.state.get_track().len() as u32; + let tracks_len = self.state.get_track().len(); - let mut new_playlist_index = index; + let mut new_playlist_index = index as usize; if new_playlist_index >= tracks_len { new_playlist_index = 0; @@ -1184,7 +1184,7 @@ impl SpircTask { // tracks in each frame either have a gid or uri (that may or may not be a valid track) // E.g - context based frames sometimes contain tracks with - let mut track_ref = self.state.get_track()[new_playlist_index as usize].clone(); + let mut track_ref = self.state.get_track()[new_playlist_index].clone(); let mut track_id = self.get_spotify_id_for_track(&track_ref); while track_id.is_err() || track_id.unwrap().audio_type == SpotifyAudioType::NonPlayable { warn!( @@ -1203,12 +1203,12 @@ impl SpircTask { warn!("No playable track found in state: {:?}", self.state); return None; } - track_ref = self.state.get_track()[index as usize].clone(); + track_ref = self.state.get_track()[new_playlist_index].clone(); track_id = self.get_spotify_id_for_track(&track_ref); } match track_id { - Ok(track_id) => Some((track_id, new_playlist_index)), + Ok(track_id) => Some((track_id, new_playlist_index as u32)), Err(_) => None, } } From 01813cf7c95c8e75b5fce39f6d99d22ca2f8fef2 Mon Sep 17 00:00:00 2001 From: ashthespy Date: Wed, 13 May 2020 13:30:30 +0200 Subject: [PATCH 5/8] Tweak unavailable track handling Flag them as `NonPlayable` instead of removing them from the queue --- connect/src/spirc.rs | 55 ++++++++++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/connect/src/spirc.rs b/connect/src/spirc.rs index ececdba..b6de59f 100644 --- a/connect/src/spirc.rs +++ b/connect/src/spirc.rs @@ -454,8 +454,8 @@ impl SpircTask { Ok(dur) => dur, Err(err) => err.duration(), }; - ((dur.as_secs() as i64 + self.session.time_delta()) * 1000 - + (dur.subsec_nanos() / 1000_000) as i64) + (dur.as_secs() as i64 + self.session.time_delta()) * 1000 + + (dur.subsec_nanos() / 1000_000) as i64 } fn ensure_mixer_started(&mut self) { @@ -622,7 +622,7 @@ impl SpircTask { } }, PlayerEvent::TimeToPreloadNextTrack { .. } => self.handle_preload_next_track(), - PlayerEvent::Unavailable { track_id, .. } => self.handle_unavalable(track_id), + PlayerEvent::Unavailable { track_id, .. } => self.handle_unavailable(track_id), _ => (), } } @@ -910,16 +910,28 @@ impl SpircTask { | SpircPlayStatus::Stopped => (), } } - - fn handle_unavalable(&mut self, track_id: SpotifyId) { - let unavalable_index = self.get_track_index_for_spotify_id( + // Mark unavailable tracks so we can skip them later + fn handle_unavailable(&mut self, track_id: SpotifyId) { + let unavailables = self.get_track_index_for_spotify_id( &track_id, self.state.get_playing_track_index() as usize, ); - if let Some(index) = unavalable_index { - // TODO: Or mark it as NonPlayable? - debug!("Removing unavailable track_ref at {:?}", index); - self.state.mut_track().remove(index); + + for &index in unavailables.iter() { + debug_assert_eq!(self.state.get_track()[index].get_gid(), track_id.to_raw()); + let mut unplayable_track_ref = TrackRef::new(); + unplayable_track_ref.set_gid(self.state.get_track()[index].get_gid().to_vec()); + // Misuse context field to flag the track + unplayable_track_ref.set_context(String::from("NonPlayable")); + std::mem::swap( + &mut self.state.mut_track()[index], + &mut unplayable_track_ref, + ); + debug!( + "Marked <{:?}> at {:?} as NonPlayable", + self.state.get_track()[index], + index, + ); } self.handle_preload_next_track(); } @@ -1157,18 +1169,28 @@ impl SpircTask { }) } + // Helper to find corresponding index(s) for track_id fn get_track_index_for_spotify_id( &self, track_id: &SpotifyId, start_index: usize, - ) -> Option { - let index = self.state.get_track()[start_index..] + ) -> Vec { + let index: Vec = self.state.get_track()[start_index..] .iter() - .position(|track_ref| self.get_spotify_id_for_track(track_ref) == Ok(*track_id)); - + .enumerate() + .filter(|&(_, track_ref)| track_ref.get_gid() == track_id.to_raw()) + .map(|(idx, _)| start_index + idx) + .collect(); + // Sanity check + debug_assert!(!index.is_empty()); index } + // Broken out here so we can refactor this later when we move to SpotifyObjectID or similar + fn track_ref_is_unavailable(&self, track_ref: &TrackRef) -> bool { + track_ref.get_context() == "NonPlayable" + } + fn get_track_id_to_play_from_playlist(&self, index: u32) -> Option<(SpotifyId, u32)> { let tracks_len = self.state.get_track().len(); @@ -1186,7 +1208,10 @@ impl SpircTask { let mut track_ref = self.state.get_track()[new_playlist_index].clone(); let mut track_id = self.get_spotify_id_for_track(&track_ref); - while track_id.is_err() || track_id.unwrap().audio_type == SpotifyAudioType::NonPlayable { + while self.track_ref_is_unavailable(&track_ref) + || track_id.is_err() + || track_id.unwrap().audio_type == SpotifyAudioType::NonPlayable + { warn!( "Skipping track <{:?}> at position [{}] of {}", track_ref.get_uri(), From 1da80ce480a4e398c2e0c6bd896da6f1da38835e Mon Sep 17 00:00:00 2001 From: ashthespy Date: Wed, 27 May 2020 16:24:21 +0200 Subject: [PATCH 6/8] Handle unplayable track at start of playlist --- connect/src/spirc.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/connect/src/spirc.rs b/connect/src/spirc.rs index b6de59f..c0bcfcf 100644 --- a/connect/src/spirc.rs +++ b/connect/src/spirc.rs @@ -910,13 +910,17 @@ impl SpircTask { | SpircPlayStatus::Stopped => (), } } + // Mark unavailable tracks so we can skip them later fn handle_unavailable(&mut self, track_id: SpotifyId) { - let unavailables = self.get_track_index_for_spotify_id( - &track_id, - self.state.get_playing_track_index() as usize, - ); - + let playing_index = self.state.get_playing_track_index() as usize; + let search_from = if playing_index == self.state.get_track().len() - 1 { + trace!("Cycling back to 0 instead of {:?}", playing_index); + 0 + } else { + playing_index + }; + let unavailables = self.get_track_index_for_spotify_id(&track_id, search_from); for &index in unavailables.iter() { debug_assert_eq!(self.state.get_track()[index].get_gid(), track_id.to_raw()); let mut unplayable_track_ref = TrackRef::new(); From a28199f07341f3437cb8d6959c6c3f79f11b4a51 Mon Sep 17 00:00:00 2001 From: ashthespy Date: Wed, 27 May 2020 18:14:31 +0200 Subject: [PATCH 7/8] Tweak handling unavailables at start of playlists --- connect/src/spirc.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/connect/src/spirc.rs b/connect/src/spirc.rs index c0bcfcf..c6feb72 100644 --- a/connect/src/spirc.rs +++ b/connect/src/spirc.rs @@ -914,13 +914,19 @@ impl SpircTask { // Mark unavailable tracks so we can skip them later fn handle_unavailable(&mut self, track_id: SpotifyId) { let playing_index = self.state.get_playing_track_index() as usize; - let search_from = if playing_index == self.state.get_track().len() - 1 { - trace!("Cycling back to 0 instead of {:?}", playing_index); - 0 - } else { - playing_index - }; - let unavailables = self.get_track_index_for_spotify_id(&track_id, search_from); + let mut unavailables = self.get_track_index_for_spotify_id(&track_id, playing_index); + if unavailables.is_empty() { + trace!( + "Couldn't find unavailables searching from {:?} -- {:?}, cycling through entire playlist", + playing_index, + self.state.get_track().len() + ); + // We could just do this everytime, but for most cases it's needless overhead. + // we are still repeating the serach for (playing_index..) in this case + unavailables = self.get_track_index_for_spotify_id(&track_id, 0); + } + // Sanity check + debug_assert!(!unavailables.is_empty()); for &index in unavailables.iter() { debug_assert_eq!(self.state.get_track()[index].get_gid(), track_id.to_raw()); let mut unplayable_track_ref = TrackRef::new(); @@ -1185,8 +1191,6 @@ impl SpircTask { .filter(|&(_, track_ref)| track_ref.get_gid() == track_id.to_raw()) .map(|(idx, _)| start_index + idx) .collect(); - // Sanity check - debug_assert!(!index.is_empty()); index } From 16112d71b968d3de4e45554f075970cf91ada39d Mon Sep 17 00:00:00 2001 From: ashthespy Date: Thu, 28 May 2020 16:18:41 +0200 Subject: [PATCH 8/8] Search through full playlist for unplayable tracks --- connect/src/spirc.rs | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/connect/src/spirc.rs b/connect/src/spirc.rs index c6feb72..bb214ad 100644 --- a/connect/src/spirc.rs +++ b/connect/src/spirc.rs @@ -913,20 +913,7 @@ impl SpircTask { // Mark unavailable tracks so we can skip them later fn handle_unavailable(&mut self, track_id: SpotifyId) { - let playing_index = self.state.get_playing_track_index() as usize; - let mut unavailables = self.get_track_index_for_spotify_id(&track_id, playing_index); - if unavailables.is_empty() { - trace!( - "Couldn't find unavailables searching from {:?} -- {:?}, cycling through entire playlist", - playing_index, - self.state.get_track().len() - ); - // We could just do this everytime, but for most cases it's needless overhead. - // we are still repeating the serach for (playing_index..) in this case - unavailables = self.get_track_index_for_spotify_id(&track_id, 0); - } - // Sanity check - debug_assert!(!unavailables.is_empty()); + let unavailables = self.get_track_index_for_spotify_id(&track_id, 0); for &index in unavailables.iter() { debug_assert_eq!(self.state.get_track()[index].get_gid(), track_id.to_raw()); let mut unplayable_track_ref = TrackRef::new(); @@ -1191,6 +1178,8 @@ impl SpircTask { .filter(|&(_, track_ref)| track_ref.get_gid() == track_id.to_raw()) .map(|(idx, _)| start_index + idx) .collect(); + // Sanity check + debug_assert!(!index.is_empty()); index } @@ -1222,9 +1211,7 @@ impl SpircTask { { warn!( "Skipping track <{:?}> at position [{}] of {}", - track_ref.get_uri(), - new_playlist_index, - tracks_len + track_ref, new_playlist_index, tracks_len ); new_playlist_index += 1;