connect: load entire context at once

connect/src/model.rs

@@ -64,12 +64,6 @@ pub(super) struct ResolveContext {
     context: Context,
     fallback: Option<String>,
     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::<Vec<&str>>()
-            .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);
     }
 }
 

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_context(resolve_uri, resolve.context_uri(), resolve.autoplay())
-                        resolve_uri,
-                        resolve.context_uri(),
-                        resolve.autoplay(),
-                        resolve.update(),
-                    )
                     .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?;
-
-            if update {
             ctx.uri = context_uri.to_string();
             ctx.url = format!("context://{context_uri}");
 
-                self.connect_state
+            if let Some(remaining) = self
+                .connect_state
                 .update_context(ctx, UpdateContext::Default)?
-            } else if matches!(ctx.pages.first(), Some(p) if !p.tracks.is_empty()) {
+            {
-                debug!(
+                self.try_resolve_remaining(remaining).await;
-                    "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:#?}");
-            };
 
             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
+        if let Some(remaining) = self
-            .update_context(context, UpdateContext::Autoplay)
+            .connect_state
+            .update_context(context, UpdateContext::Autoplay)?
+        {
+            self.try_resolve_remaining(remaining).await;
+        }
+
+        Ok(())
+    }
+
+    async fn try_resolve_remaining(&mut self, remaining: Vec<String>) {
+        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);
 
-                        self.add_resolve_context(resolve)
+        self.add_resolve_context(resolve);
-                    }
-                    LoadNext::Empty => {
-                        debug!("next context is empty and autoplay isn't enabled, no preloading required")
-                    }
-                }
-            }
-        }
     }
 
     fn is_playing(&self) -> bool {

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,
+    ContextIndex, ContextPlayerOptions, PlayOrigin, PlayerState, ProvidedTrack, Suppressions,
-    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<StateContext>,
-    /// upcoming contexts, directly provided by the context-resolver
-    next_contexts: Vec<ContextPage>,
 
     /// a context to keep track of our shuffled context,
     /// should be only available when `player.option.shuffling_context` is true

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::<Vec<&str>>()
+        .join(":")
+}
+
 impl ConnectState {
     pub fn find_index_in_context<F: Fn(&ProvidedTrack) -> 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<Option<Vec<String>>, 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) {
+        let mut next_contexts = Vec::new();
-            self.next_contexts.clear();
-        }
-
         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<LoadNext, Error> {
-        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)
-    }
 }