From 28588c4a8ee68abcda9cca0c3a1c7d88a7d151bc Mon Sep 17 00:00:00 2001 From: Felix Prillwitz Date: Tue, 10 Dec 2024 21:34:54 +0100 Subject: [PATCH] connect: load entire context at once --- connect/src/model.rs | 47 +--------------- connect/src/spirc.rs | 102 ++++++++++++++++++----------------- connect/src/state.rs | 5 +- connect/src/state/context.rs | 84 +++++++++++++++++------------ 4 files changed, 105 insertions(+), 133 deletions(-) diff --git a/connect/src/model.rs b/connect/src/model.rs index f9165eae..a7ff6e22 100644 --- a/connect/src/model.rs +++ b/connect/src/model.rs @@ -64,12 +64,6 @@ pub(super) struct ResolveContext { context: Context, fallback: Option, autoplay: bool, - /// if `true` updates the entire context, otherwise only fills the context from the next - /// retrieve page, it is usually used when loading the next page of an already established context - /// - /// like for example: - /// - playing an artists profile - update: bool, } impl ResolveContext { @@ -82,7 +76,6 @@ impl ResolveContext { }, fallback: (!fallback_uri.is_empty()).then_some(fallback_uri), autoplay, - update: true, } } @@ -91,35 +84,6 @@ impl ResolveContext { context, fallback: None, autoplay, - update: true, - } - } - - // expected page_url: hm://artistplaycontext/v1/page/spotify/album/5LFzwirfFwBKXJQGfwmiMY/km_artist - pub fn from_page_url(page_url: String) -> Self { - let split = if let Some(rest) = page_url.strip_prefix("hm://") { - rest.split('/') - } else { - warn!("page_url didn't started with hm://. got page_url: {page_url}"); - page_url.split('/') - }; - - let uri = split - .skip_while(|s| s != &"spotify") - .take(3) - .collect::>() - .join(":"); - - trace!("created an ResolveContext from page_url <{page_url}> as uri <{uri}>"); - - Self { - context: Context { - uri, - ..Default::default() - }, - fallback: None, - update: false, - autoplay: false, } } @@ -140,21 +104,16 @@ impl ResolveContext { pub fn autoplay(&self) -> bool { self.autoplay } - - pub fn update(&self) -> bool { - self.update - } } impl Display for ResolveContext { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!( f, - "resolve_uri: <{:?}>, context_uri: <{}>, autoplay: <{}>, update: <{}>", + "resolve_uri: <{:?}>, context_uri: <{}>, autoplay: <{}>", self.resolve_uri(), self.context.uri, self.autoplay, - self.update ) } } @@ -164,9 +123,8 @@ impl PartialEq for ResolveContext { let eq_context = self.context_uri() == other.context_uri(); let eq_resolve = self.resolve_uri() == other.resolve_uri(); let eq_autoplay = self.autoplay == other.autoplay; - let eq_update = self.update == other.update; - eq_context && eq_resolve && eq_autoplay && eq_update + eq_context && eq_resolve && eq_autoplay } } @@ -177,7 +135,6 @@ impl Hash for ResolveContext { self.context_uri().hash(state); self.resolve_uri().hash(state); self.autoplay.hash(state); - self.update.hash(state); } } diff --git a/connect/src/spirc.rs b/connect/src/spirc.rs index 0f819c8f..150b1331 100644 --- a/connect/src/spirc.rs +++ b/connect/src/spirc.rs @@ -27,7 +27,7 @@ use crate::{ use crate::{ model::{ResolveContext, SpircPlayStatus}, state::{ - context::{ContextType, LoadNext, UpdateContext}, + context::{ContextType, UpdateContext}, provider::IsProvider, {ConnectState, ConnectStateConfig}, }, @@ -488,12 +488,7 @@ impl SpircTask { // the autoplay endpoint can return a 404, when it tries to retrieve an // autoplay context for an empty playlist as it seems if let Err(why) = self - .resolve_context( - resolve_uri, - resolve.context_uri(), - resolve.autoplay(), - resolve.update(), - ) + .resolve_context(resolve_uri, resolve.context_uri(), resolve.autoplay()) .await { error!("failed resolving context <{resolve}>: {why}"); @@ -537,37 +532,29 @@ impl SpircTask { resolve_uri: &str, context_uri: &str, autoplay: bool, - update: bool, ) -> Result<(), Error> { if !autoplay { let mut ctx = self.session.spclient().get_context(resolve_uri).await?; + ctx.uri = context_uri.to_string(); + ctx.url = format!("context://{context_uri}"); - if update { - ctx.uri = context_uri.to_string(); - ctx.url = format!("context://{context_uri}"); - - self.connect_state - .update_context(ctx, UpdateContext::Default)? - } else if matches!(ctx.pages.first(), Some(p) if !p.tracks.is_empty()) { - debug!( - "update context from single page, context {} had {} pages", - ctx.uri, - ctx.pages.len() - ); - self.connect_state - .fill_context_from_page(ctx.pages.remove(0))?; - } else { - error!("resolving context should only update the tracks, but had no page, or track. {ctx:#?}"); - }; + if let Some(remaining) = self + .connect_state + .update_context(ctx, UpdateContext::Default)? + { + self.try_resolve_remaining(remaining).await; + } return Ok(()); } + // refuse resolve of not supported autoplay context if resolve_uri.contains("spotify:show:") || resolve_uri.contains("spotify:episode:") { // autoplay is not supported for podcasts Err(SpircError::NotAllowedContext(resolve_uri.to_string()))? } + // resolve autoplay let previous_tracks = self.connect_state.prev_autoplay_track_uris(); debug!( @@ -587,8 +574,40 @@ impl SpircTask { .get_autoplay_context(&ctx_request) .await?; - self.connect_state - .update_context(context, UpdateContext::Autoplay) + if let Some(remaining) = self + .connect_state + .update_context(context, UpdateContext::Autoplay)? + { + self.try_resolve_remaining(remaining).await; + } + + Ok(()) + } + + async fn try_resolve_remaining(&mut self, remaining: Vec) { + for resolve_uri in remaining { + let mut ctx = match self.session.spclient().get_context(&resolve_uri).await { + Ok(ctx) => ctx, + Err(why) => { + warn!("failed to retrieve context for remaining <{resolve_uri}>: {why}"); + continue; + } + }; + + if ctx.pages.len() > 1 { + warn!("context contained more page then expected: {ctx:#?}"); + continue; + } + + debug!("appending context from single page, adding: <{}>", ctx.uri); + + if let Err(why) = self + .connect_state + .fill_context_from_page(ctx.pages.remove(0)) + { + warn!("failed appending context <{resolve_uri}>: {why}"); + } + } } fn add_resolve_context(&mut self, resolve: ResolveContext) { @@ -1193,7 +1212,7 @@ impl SpircTask { debug!("context <{current_context_uri}> didn't change, no resolving required") } else { debug!("resolving context for load command"); - self.resolve_context(&fallback, &cmd.context_uri, false, true) + self.resolve_context(&fallback, &cmd.context_uri, false) .await?; } @@ -1366,33 +1385,18 @@ impl SpircTask { fn preload_autoplay_when_required(&mut self) { let require_load_new = !self .connect_state - .has_next_tracks(Some(CONTEXT_FETCH_THRESHOLD)); + .has_next_tracks(Some(CONTEXT_FETCH_THRESHOLD)) + && self.session.autoplay(); if !require_load_new { return; } - match self.connect_state.try_load_next_context() { - Err(why) => error!("failed loading next context: {why}"), - Ok(next) => { - match next { - LoadNext::Done => info!("loaded next context"), - LoadNext::PageUrl(page_url) => { - self.add_resolve_context(ResolveContext::from_page_url(page_url)) - } - LoadNext::Empty if self.session.autoplay() => { - let current_context = self.connect_state.context_uri(); - let fallback = self.connect_state.current_track(|t| &t.uri); - let resolve = ResolveContext::from_uri(current_context, fallback, true); + let current_context = self.connect_state.context_uri(); + let fallback = self.connect_state.current_track(|t| &t.uri); + let resolve = ResolveContext::from_uri(current_context, fallback, true); - self.add_resolve_context(resolve) - } - LoadNext::Empty => { - debug!("next context is empty and autoplay isn't enabled, no preloading required") - } - } - } - } + self.add_resolve_context(resolve); } fn is_playing(&self) -> bool { diff --git a/connect/src/state.rs b/connect/src/state.rs index 8bfea2a3..b9cf37d6 100644 --- a/connect/src/state.rs +++ b/connect/src/state.rs @@ -20,8 +20,7 @@ use librespot_protocol::connect::{ Capabilities, Device, DeviceInfo, MemberType, PutStateReason, PutStateRequest, }; use librespot_protocol::player::{ - ContextIndex, ContextPage, ContextPlayerOptions, PlayOrigin, PlayerState, ProvidedTrack, - Suppressions, + ContextIndex, ContextPlayerOptions, PlayOrigin, PlayerState, ProvidedTrack, Suppressions, }; use log::LevelFilter; use protobuf::{EnumOrUnknown, MessageField}; @@ -112,8 +111,6 @@ pub struct ConnectState { /// the context from which we play, is used to top up prev and next tracks pub context: Option, - /// upcoming contexts, directly provided by the context-resolver - next_contexts: Vec, /// a context to keep track of our shuffled context, /// should be only available when `player.option.shuffling_context` is true diff --git a/connect/src/state/context.rs b/connect/src/state/context.rs index 3e9d720e..378da66c 100644 --- a/connect/src/state/context.rs +++ b/connect/src/state/context.rs @@ -27,12 +27,6 @@ pub enum ContextType { Autoplay, } -pub enum LoadNext { - Done, - PageUrl(String), - Empty, -} - #[derive(Debug)] pub enum UpdateContext { Default, @@ -45,6 +39,27 @@ pub enum ResetContext<'s> { WhenDifferent(&'s str), } +/// Extracts the spotify uri from a given page_url +/// +/// Just extracts "spotify/album/5LFzwirfFwBKXJQGfwmiMY" and replaces the slash's with colon's +/// +/// Expected `page_url` should look something like the following: +/// `hm://artistplaycontext/v1/page/spotify/album/5LFzwirfFwBKXJQGfwmiMY/km_artist` +fn page_url_to_uri(page_url: &str) -> String { + let split = if let Some(rest) = page_url.strip_prefix("hm://") { + rest.split('/') + } else { + warn!("page_url didn't started with hm://. got page_url: {page_url}"); + page_url.split('/') + }; + + split + .skip_while(|s| s != &"spotify") + .take(3) + .collect::>() + .join(":") +} + impl ConnectState { pub fn find_index_in_context bool>( context: Option<&StateContext>, @@ -86,7 +101,6 @@ impl ConnectState { ResetContext::Completely => { self.context = None; self.autoplay_context = None; - self.next_contexts.clear(); } ResetContext::WhenDifferent(_) => debug!("context didn't change, no reset"), ResetContext::DefaultIndex => { @@ -142,7 +156,11 @@ impl ConnectState { } } - pub fn update_context(&mut self, mut context: Context, ty: UpdateContext) -> Result<(), Error> { + pub fn update_context( + &mut self, + mut context: Context, + ty: UpdateContext, + ) -> Result>, Error> { if context.pages.iter().all(|p| p.tracks.is_empty()) { error!("context didn't have any tracks: {context:#?}"); return Err(StateError::ContextHasNoTracks.into()); @@ -150,16 +168,13 @@ impl ConnectState { return Err(StateError::UnsupportedLocalPlayBack.into()); } - if matches!(ty, UpdateContext::Default) { - self.next_contexts.clear(); - } - + let mut next_contexts = Vec::new(); let mut first_page = None; for page in context.pages { if first_page.is_none() && !page.tracks.is_empty() { first_page = Some(page); } else { - self.next_contexts.push(page) + next_contexts.push(page) } } @@ -234,7 +249,27 @@ impl ConnectState { } } - Ok(()) + if next_contexts.is_empty() { + return Ok(None); + } + + // load remaining contexts + let next_contexts = next_contexts + .into_iter() + .flat_map(|page| { + if !page.tracks.is_empty() { + self.fill_context_from_page(page).ok()?; + None + } else if !page.page_url.is_empty() { + Some(page_url_to_uri(&page.page_url)) + } else { + warn!("unhandled context page: {page:#?}"); + None + } + }) + .collect(); + + Ok(Some(next_contexts)) } fn state_context_from_page( @@ -391,25 +426,4 @@ impl ConnectState { Ok(()) } - - pub fn try_load_next_context(&mut self) -> Result { - let next = match self.next_contexts.first() { - None => return Ok(LoadNext::Empty), - Some(_) => self.next_contexts.remove(0), - }; - - if next.tracks.is_empty() { - if next.page_url.is_empty() { - Err(StateError::NoContext(ContextType::Default))? - } - - self.update_current_index(|i| i.page += 1); - return Ok(LoadNext::PageUrl(next.page_url)); - } - - self.fill_context_from_page(next)?; - self.fill_up_next_tracks()?; - - Ok(LoadNext::Done) - } }