Shuffle tracks in place (#1445)

* connect: add shuffle_vec.rs

* connect: shuffle in place

* add shuffle with seed option

* reduce complexity to add new metadata fields

* add context index to metadata

* use seed for shuffle

When losing the connection and restarting the dealer, the seed is now stored in the context metadata. So on transfer we can pickup the seed again and shuffle the context as it was previously

* add log for shuffle seed

* connect: use small_rng, derive Default

* Apply suggestions from code review

Co-authored-by: Copilot <[email protected]>

---------

Co-authored-by: Copilot <[email protected]>

Author: photovoltex

connect/Cargo.toml

@@ -12,7 +12,7 @@ edition = "2021"
 futures-util = "0.3"
 log = "0.4"
 protobuf = "3.5"
-rand = "0.8"
+rand = { version = "0.8", default-features = false, features = ["small_rng"] }
 serde_json = "1.0"
 thiserror = "2.0"
 tokio = { version = "1", features = ["macros", "parking_lot", "sync"] }

connect/src/context_resolver.rs

@@ -4,13 +4,10 @@ use crate::{
         autoplay_context_request::AutoplayContextRequest, context::Context,
         transfer_state::TransferState,
     },
-    state::{
-        context::{ContextType, UpdateContext},
-        ConnectState,
-    },
+    state::{context::ContextType, ConnectState},
 };
-use std::cmp::PartialEq;
 use std::{
+    cmp::PartialEq,
     collections::{HashMap, VecDeque},
     fmt::{Display, Formatter},
     hash::Hash,
@@ -35,7 +32,7 @@ pub(super) enum ContextAction {
 pub(super) struct ResolveContext {
     resolve: Resolve,
     fallback: Option<String>,
-    update: UpdateContext,
+    update: ContextType,
     action: ContextAction,
 }
 
@@ -44,15 +41,15 @@ impl ResolveContext {
         Self {
             resolve: Resolve::Uri(uri.into()),
             fallback: None,
-            update: UpdateContext::Default,
+            update: ContextType::Default,
             action: ContextAction::Append,
         }
     }
 
     pub fn from_uri(
         uri: impl Into<String>,
         fallback: impl Into<String>,
-        update: UpdateContext,
+        update: ContextType,
         action: ContextAction,
     ) -> Self {
         let fallback_uri = fallback.into();
@@ -64,7 +61,7 @@ impl ResolveContext {
         }
     }
 
-    pub fn from_context(context: Context, update: UpdateContext, action: ContextAction) -> Self {
+    pub fn from_context(context: Context, update: ContextType, action: ContextAction) -> Self {
         Self {
             resolve: Resolve::Context(context),
             fallback: None,
@@ -214,7 +211,7 @@ impl ContextResolver {
         let (next, resolve_uri, _) = self.find_next().ok_or(ContextResolverError::NoNext)?;
 
         match next.update {
-            UpdateContext::Default => {
+            ContextType::Default => {
                 let mut ctx = self.session.spclient().get_context(resolve_uri).await;
                 if let Ok(ctx) = ctx.as_mut() {
                     ctx.uri = Some(next.context_uri().to_string());
@@ -223,7 +220,7 @@ impl ContextResolver {
 
                 ctx
             }
-            UpdateContext::Autoplay => {
+            ContextType::Autoplay => {
                 if resolve_uri.contains("spotify:show:") || resolve_uri.contains("spotify:episode:")
                 {
                     // autoplay is not supported for podcasts
@@ -304,13 +301,13 @@ impl ContextResolver {
         }
 
         match (next.update, state.active_context) {
-            (UpdateContext::Default, ContextType::Default) | (UpdateContext::Autoplay, _) => {
+            (ContextType::Default, ContextType::Default) | (ContextType::Autoplay, _) => {
                 debug!(
                     "last item of type <{:?}>, finishing state setup",
                     next.update
                 );
             }
-            (UpdateContext::Default, _) => {
+            (ContextType::Default, _) => {
                 debug!("skipped finishing default, because it isn't the active context");
                 return false;
             }
@@ -320,7 +317,7 @@ impl ContextResolver {
         let res = if let Some(transfer_state) = transfer_state.take() {
             state.finish_transfer(transfer_state)
         } else if state.shuffling_context() {
-            state.shuffle()
+            state.shuffle(None)
         } else if matches!(active_ctx, Ok(ctx) if ctx.index.track == 0) {
             // has context, and context is not touched
             // when the index is not zero, the next index was already evaluated elsewhere

connect/src/lib.rs

@@ -7,5 +7,6 @@ use librespot_protocol as protocol;
 
 mod context_resolver;
 mod model;
+pub mod shuffle_vec;
 pub mod spirc;
 pub mod state;

connect/src/shuffle_vec.rs

@@ -0,0 +1,117 @@
+use rand::{rngs::SmallRng, Rng, SeedableRng};
+use std::{
+    ops::{Deref, DerefMut},
+    vec::IntoIter,
+};
+
+#[derive(Debug, Clone, Default)]
+pub struct ShuffleVec<T> {
+    vec: Vec<T>,
+    indices: Option<Vec<usize>>,
+}
+
+impl<T: PartialEq> PartialEq for ShuffleVec<T> {
+    fn eq(&self, other: &Self) -> bool {
+        self.vec == other.vec
+    }
+}
+
+impl<T> Deref for ShuffleVec<T> {
+    type Target = Vec<T>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.vec
+    }
+}
+
+impl<T> DerefMut for ShuffleVec<T> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        self.vec.as_mut()
+    }
+}
+
+impl<T> IntoIterator for ShuffleVec<T> {
+    type Item = T;
+    type IntoIter = IntoIter<T>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.vec.into_iter()
+    }
+}
+
+impl<T> From<Vec<T>> for ShuffleVec<T> {
+    fn from(vec: Vec<T>) -> Self {
+        Self { vec, indices: None }
+    }
+}
+
+impl<T> ShuffleVec<T> {
+    pub fn new() -> Self {
+        Self {
+            vec: Vec::new(),
+            indices: None,
+        }
+    }
+
+    pub fn shuffle_with_seed(&mut self, seed: u64) {
+        self.shuffle_with_rng(SmallRng::seed_from_u64(seed))
+    }
+
+    pub fn shuffle_with_rng(&mut self, mut rng: impl Rng) {
+        if self.indices.is_some() {
+            self.unshuffle()
+        }
+
+        let indices = {
+            (1..self.vec.len())
+                .rev()
+                .map(|i| rng.gen_range(0..i + 1))
+                .collect()
+        };
+
+        for (i, &rnd_ind) in (1..self.vec.len()).rev().zip(&indices) {
+            self.vec.swap(i, rnd_ind);
+        }
+
+        self.indices = Some(indices)
+    }
+
+    pub fn unshuffle(&mut self) {
+        let indices = match self.indices.take() {
+            Some(indices) => indices,
+            None => return,
+        };
+
+        for i in 1..self.vec.len() {
+            let n = indices[self.vec.len() - i - 1];
+            self.vec.swap(n, i);
+        }
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use rand::Rng;
+
+    #[test]
+    fn test_shuffle_with_seed() {
+        let seed = rand::thread_rng().gen_range(0..10000000000000);
+
+        let vec = (0..100).collect::<Vec<_>>();
+        let base_vec: ShuffleVec<i32> = vec.into();
+
+        let mut shuffled_vec = base_vec.clone();
+        shuffled_vec.shuffle_with_seed(seed);
+
+        let mut different_shuffled_vec = base_vec.clone();
+        different_shuffled_vec.shuffle_with_seed(seed);
+
+        assert_eq!(shuffled_vec, different_shuffled_vec);
+
+        let mut unshuffled_vec = shuffled_vec.clone();
+        unshuffled_vec.unshuffle();
+
+        assert_eq!(base_vec, unshuffled_vec);
+    }
+}

connect/src/spirc.rs

@@ -25,9 +25,7 @@ use crate::{
         user_attributes::UserAttributesMutation,
     },
     state::{
-        context::{
-            ResetContext, {ContextType, UpdateContext},
-        },
+        context::{ContextType, ResetContext},
         metadata::Metadata,
         provider::IsProvider,
         {ConnectState, ConnectStateConfig},
@@ -37,7 +35,6 @@ use futures_util::StreamExt;
 use protobuf::MessageField;
 use std::{
     future::Future,
-    ops::Deref,
     sync::atomic::{AtomicUsize, Ordering},
     sync::Arc,
     time::{Duration, SystemTime, UNIX_EPOCH},
@@ -749,9 +746,6 @@ impl SpircTask {
 
         use protobuf::Message;
 
-        // todo: handle received pages from transfer, important to not always shuffle the first 10 tracks
-        //  also important when the dealer is restarted, currently we just shuffle again, but at least
-        //  the 10 tracks provided should be used and after that the new shuffle context
         match TransferState::parse_from_bytes(&cluster.transfer_data) {
             Ok(transfer_state) => self.handle_transfer(transfer_state)?,
             Err(why) => error!("failed to take over control: {why}"),
@@ -889,7 +883,7 @@ impl SpircTask {
                 } else {
                     self.context_resolver.add(ResolveContext::from_context(
                         update_context.context,
-                        super::state::context::UpdateContext::Default,
+                        ContextType::Default,
                         ContextAction::Replace,
                     ))
                 }
@@ -1007,7 +1001,7 @@ impl SpircTask {
         self.context_resolver.add(ResolveContext::from_uri(
             ctx_uri.clone(),
             &fallback,
-            UpdateContext::Default,
+            ContextType::Default,
             ContextAction::Replace,
         ));
 
@@ -1044,7 +1038,7 @@ impl SpircTask {
             self.context_resolver.add(ResolveContext::from_uri(
                 ctx_uri,
                 fallback,
-                UpdateContext::Autoplay,
+                ContextType::Autoplay,
                 ContextAction::Replace,
             ))
         }
@@ -1139,13 +1133,12 @@ impl SpircTask {
         };
 
         let update_context = if cmd.autoplay {
-            UpdateContext::Autoplay
+            ContextType::Autoplay
         } else {
-            UpdateContext::Default
+            ContextType::Default
         };
 
-        self.connect_state
-            .set_active_context(*update_context.deref());
+        self.connect_state.set_active_context(update_context);
 
         let current_context_uri = self.connect_state.context_uri();
         if current_context_uri == &cmd.context_uri && fallback == cmd.context_uri {
@@ -1209,7 +1202,7 @@ impl SpircTask {
             if self.context_resolver.has_next() {
                 self.connect_state.update_queue_revision()
             } else {
-                self.connect_state.shuffle()?;
+                self.connect_state.shuffle(None)?;
                 self.add_autoplay_resolving_when_required();
             }
         } else {
@@ -1366,7 +1359,7 @@ impl SpircTask {
         let resolve = ResolveContext::from_uri(
             current_context,
             fallback,
-            UpdateContext::Autoplay,
+            ContextType::Autoplay,
             if has_tracks {
                 ContextAction::Append
             } else {
@@ -1458,7 +1451,7 @@ impl SpircTask {
         self.context_resolver.add(ResolveContext::from_uri(
             uri,
             self.connect_state.current_track(|t| &t.uri),
-            UpdateContext::Default,
+            ContextType::Default,
             ContextAction::Replace,
         ));
 

connect/src/state.rs

@@ -7,12 +7,12 @@ mod restrictions;
 mod tracks;
 mod transfer;
 
-use crate::model::SpircPlayStatus;
 use crate::{
     core::{
         config::DeviceType, date::Date, dealer::protocol::Request, spclient::SpClientResult,
         version, Error, Session,
     },
+    model::SpircPlayStatus,
     protocol::{
         connect::{Capabilities, Device, DeviceInfo, MemberType, PutStateReason, PutStateRequest},
         media::AudioQuality,
@@ -26,7 +26,6 @@ use crate::{
         provider::{IsProvider, Provider},
     },
 };
-
 use log::LevelFilter;
 use protobuf::{EnumOrUnknown, MessageField};
 use std::{
@@ -118,10 +117,9 @@ pub struct ConnectState {
 
     /// the context from which we play, is used to top up prev and next tracks
     context: Option<StateContext>,
+    /// seed extracted in [ConnectState::handle_initial_transfer] and used in [ConnectState::finish_transfer]
+    transfer_shuffle_seed: Option<u64>,
 
-    /// a context to keep track of our shuffled context,
-    /// should be only available when `player.option.shuffling_context` is true
-    shuffle_context: Option<StateContext>,
     /// a context to keep track of the autoplay context
     autoplay_context: Option<StateContext>,
 }