src/music_controller/controller.rs

@@ -6,7 +6,6 @@ use crossbeam_channel;
 use crossbeam_channel::{Receiver, Sender};
 use std::path::PathBuf;
 use std::sync::{Arc, RwLock};
-use std::thread::spawn;
 use thiserror::Error;
 
 use crossbeam_channel::unbounded;
@@ -14,18 +13,18 @@ use std::error::Error;
 use uuid::Uuid;
 
 use crate::config::config::ConfigError;
-use crate::music_player::player::{Player, PlayerCommand, PlayerError};
+use crate::music_player::player::{Player, PlayerError};
 use crate::{
     config::config::Config, music_controller::queue::Queue, music_storage::library::MusicLibrary,
 };
 
 use super::queue::QueueError;
 
-pub struct Controller<P: Player + Send + Sync> {
+pub struct Controller<P: Player> {
     pub queue: Queue,
     pub config: Arc<RwLock<Config>>,
     pub library: MusicLibrary,
-    pub player: Arc<RwLock<Box<P>>>,
+    pub player: Box<P>,
 }
 
 #[derive(Error, Debug)]
@@ -70,7 +69,7 @@ impl<T: Send, U: Send> MailMan<T, U> {
 }
 
 #[allow(unused_variables)]
-impl<P: Player + Send + Sync> Controller<P> {
+impl<P: Player> Controller<P> {
     pub fn start<T>(config_path: T) -> Result<Self, Box<dyn Error>>
     where
         std::path::PathBuf: std::convert::From<T>,
@@ -84,28 +83,12 @@ impl<P: Player + Send + Sync> Controller<P> {
         let config_ = Arc::new(RwLock::from(config));
         let library = MusicLibrary::init(config_.clone(), uuid)?;
 
-        let controller = Controller {
+        Ok(Controller {
             queue: Queue::default(),
             config: config_.clone(),
             library,
-            player: Arc::new(RwLock::new(Box::new(P::new()?))),
+            player: Box::new(P::new()?),
-        };
+        })
-
-
-        let player = controller.player.clone();
-        let controler_thread = spawn(move || {
-            match player.read().unwrap().message_channel().recv().unwrap() {
-                PlayerCommand::AboutToFinish => {},
-                PlayerCommand::EndOfStream => {
-
-                    player.write().unwrap().enqueue_next(todo!());
-                },
-                _ => {}
-            }
-        });
-
-
-        Ok(controller)
     }
 
     pub fn q_add(&mut self, item: &Uuid, source: super::queue::PlayerLocation, by_human: bool) {

src/music_controller/queue.rs

@@ -59,7 +59,7 @@ pub struct Queue {
     pub items: Vec<QueueItem>,
     pub played: Vec<QueueItem>,
     pub loop_: bool,
-    pub shuffle: Option<Vec<usize>>,
+    pub shuffle: bool,
 }
 
 // TODO: HAndle the First QueueState[looping] and shuffle

src/music_player/gstreamer.rs

@@ -94,7 +94,6 @@ impl GStreamer {
         }
 
         // Make sure the playback tracker knows the stuff is stopped
-        println!("Beginning switch");
         self.playback_tx.send(PlaybackInfo::Switching).unwrap();
 
         let uri = self.playbin.read().unwrap().property_value("current-uri");
@@ -134,11 +133,12 @@ impl GStreamer {
                     .unwrap()
                     .set_property("uri", source.as_uri());
 
-                if self.state() != PlayerState::Playing {
                 self.play().unwrap();
-                }
 
-                while self.raw_duration().is_none() {
+                while uri.get::<&str>().unwrap_or("")
+                    == self.property("current-uri").get::<&str>().unwrap_or("")
+                    || self.position().is_none()
+                {
                     std::thread::sleep(std::time::Duration::from_millis(10));
                 }
 
@@ -213,11 +213,6 @@ impl GStreamer {
     fn property(&self, property: &str) -> glib::Value {
         self.playbin().unwrap().property_value(property)
     }
-
-    fn ready(&mut self) -> Result<(), PlayerError> {
-        self.set_state(gst::State::Ready)?;
-        Ok(())
-    }
 }
 
 impl Player for GStreamer {
@@ -254,7 +249,7 @@ impl Player for GStreamer {
             .ok_or(PlayerError::Build)?;
 
         playbin.write().unwrap().set_property_from_value("flags", &flags);
-        //playbin.write().unwrap().set_property("instant-uri", true);
+        playbin.write().unwrap().set_property("instant-uri", true);
 
         let position = Arc::new(RwLock::new(None));
 
@@ -263,6 +258,7 @@ impl Player for GStreamer {
         let (status_tx, status_rx) = unbounded::<PlaybackInfo>();
         let position_update = Arc::clone(&position);
 
+        let _playback_monitor =
             std::thread::spawn(|| playback_monitor(playbin_arc, status_rx, playback_tx, position_update));
 
         // Set up the thread to monitor bus messages
@@ -334,46 +330,62 @@ impl Player for GStreamer {
         &self.source
     }
 
+    /// Insert a new track to be played. This method should be called at the
+    /// beginning to start playback of something, and once the [PlayerCommand]
+    /// indicates the track is about to finish to enqueue gaplessly.
     fn enqueue_next(&mut self, next_track: &URI) -> Result<(), PlayerError> {
         self.set_source(next_track)
     }
 
+    /// Set the playback volume, accepts a float from 0 to 1
     fn set_volume(&mut self, volume: f64) {
         self.volume = volume.clamp(0.0, 1.0);
         self.set_gstreamer_volume(self.volume);
     }
 
-    fn volume(&self) -> f64 {
+    /// Returns the current volume level, a float from 0 to 1
+    fn volume(&mut self) -> f64 {
         self.volume
     }
 
-    fn play(&mut self) -> Result<(), PlayerError> {
+    fn ready(&mut self) -> Result<(), PlayerError> {
-        if self.state() == PlayerState::Playing {
+        self.set_state(gst::State::Ready)?;
-            return Ok(())
+        Ok(())
     }
+
+    /// If the player is paused or stopped, starts playback
+    fn play(&mut self) -> Result<(), PlayerError> {
         *self.paused.write().unwrap() = false;
         self.set_state(gst::State::Playing)?;
         Ok(())
     }
 
+    /// Pause, if playing
     fn pause(&mut self) -> Result<(), PlayerError> {
-        if self.state() == PlayerState::Paused || *self.paused.read().unwrap() {
-            return Ok(())
-        }
         *self.paused.write().unwrap() = true;
         self.set_state(gst::State::Paused)?;
         Ok(())
     }
 
-    fn is_paused(&self) -> bool {
+    /// Resume from being paused
+    fn resume(&mut self) -> Result<(), PlayerError> {
+        *self.paused.write().unwrap() = false;
+        self.set_state(gst::State::Playing)?;
+        Ok(())
+    }
+
+    /// Check if playback is paused
+    fn is_paused(&mut self) -> bool {
         self.playbin().unwrap().current_state() == gst::State::Paused
     }
 
-    fn position(&self) -> Option<Duration> {
+    /// Get the current playback position of the player
+    fn position(&mut self) -> Option<Duration> {
         *self.position.read().unwrap()
     }
 
-    fn duration(&self) -> Option<Duration> {
+    /// Get the duration of the currently playing track
+    fn duration(&mut self) -> Option<Duration> {
         if self.end.is_some() && self.start.is_some() {
             Some(self.end.unwrap() - self.start.unwrap())
         } else {
@@ -381,6 +393,7 @@ impl Player for GStreamer {
         }
     }
 
+    /// Seek relative to the current position
     fn seek_by(&mut self, seek_amount: Duration) -> Result<(), PlayerError> {
         let time_pos = match *self.position.read().unwrap() {
             Some(pos) => pos,
@@ -392,6 +405,7 @@ impl Player for GStreamer {
         Ok(())
     }
 
+    /// Seek absolutely
     fn seek_to(&mut self, target_pos: Duration) -> Result<(), PlayerError> {
         let start = if self.start.is_none() {
             return Err(PlayerError::Seek("No START time".into()));
@@ -419,6 +433,7 @@ impl Player for GStreamer {
         Ok(())
     }
 
+    /// Stop the playback entirely
     fn stop(&mut self) -> Result<(), PlayerError> {
         self.pause()?;
         self.ready()?;
@@ -433,6 +448,7 @@ impl Player for GStreamer {
         Ok(())
     }
 
+    /// Return a reference to the player message channel
     fn message_channel(&self) -> &crossbeam::channel::Receiver<PlayerCommand> {
         &self.message_rx
     }
@@ -461,7 +477,7 @@ fn playback_monitor(
     let mut sent_atf = false;
     loop {
         // Check for new messages to decide how to proceed
-        if let Ok(result) = status_rx.recv_timeout(std::time::Duration::from_millis(50)) {
+        if let Ok(result) = status_rx.recv_timeout(std::time::Duration::from_millis(10)) {
             stats = result
         }
 
@@ -474,9 +490,8 @@ fn playback_monitor(
         match stats {
             PlaybackInfo::Playing{start, end} if pos_temp.is_some() => {
                 // Check if the current playback position is close to the end
-                let finish_point = end - Duration::milliseconds(2000);
+                let finish_point = end - Duration::milliseconds(250);
                 if pos_temp.unwrap().num_microseconds() >= end.num_microseconds() {
-                    println!("MONITOR: End of stream");
                     let _ = playback_tx.try_send(PlayerCommand::EndOfStream);
                     playbin
                         .write()
@@ -485,9 +500,7 @@ fn playback_monitor(
                         .expect("Unable to set the pipeline state");
                     sent_atf = false
                 } else if pos_temp.unwrap().num_microseconds() >= finish_point.num_microseconds()
-                    && !sent_atf
+                    && !sent_atf {
-                {
-                    println!("MONITOR: About to finish");
                     let _ = playback_tx.try_send(PlayerCommand::AboutToFinish);
                     sent_atf = true;
                 }
@@ -497,7 +510,6 @@ fn playback_monitor(
                 pos_temp = Some(pos_temp.unwrap() - start)
             },
             PlaybackInfo::Finished => {
-                println!("MONITOR: Shutting down");
                 *position.write().unwrap() = None;
                 break
             },

src/music_player/player.rs

@@ -40,58 +40,36 @@ pub enum PlayerCommand {
 }
 
 pub trait Player {
-    /// Create a new player.
+    /// Create a new player
     fn new() -> Result<Self, PlayerError> where Self: Sized;
 
-    /// Get the currently playing [URI] from the player.
     fn source(&self) -> &Option<URI>;
 
-    /// Insert a new [`URI`] to be played. This method should be called at the
-    /// beginning to start playback of something, and once the [`PlayerCommand`]
-    /// indicates the track is about to finish to enqueue gaplessly.
-    ///
-    /// For backends which do not support gapless playback, `AboutToFinish`
-    /// will not be called, and the next [`URI`] should be enqueued once `Eos`
-    /// occurs.
     fn enqueue_next(&mut self, next_track: &URI) -> Result<(), PlayerError>;
 
-    /// Set the playback volume, accepts a float from `0` to `1`.
-    ///
-    /// Values outside the range of `0` to `1` will be capped.
     fn set_volume(&mut self, volume: f64);
 
-    /// Returns the current volume level, a float from `0` to `1`.
+    fn volume(&mut self) -> f64;
-    fn volume(&self) -> f64;
+
+    fn ready(&mut self) -> Result<(), PlayerError>;
 
-    /// If the player is paused or stopped, starts playback.
     fn play(&mut self) -> Result<(), PlayerError>;
 
-    /// If the player is playing, pause playback.
+    fn resume(&mut self) -> Result<(), PlayerError>;
+
     fn pause(&mut self) -> Result<(), PlayerError>;
 
-    /// Stop the playback entirely, removing the current [`URI`] from the player.
     fn stop(&mut self) -> Result<(), PlayerError>;
 
-    /// Convenience function to check if playback is paused.
+    fn is_paused(&mut self) -> bool;
-    fn is_paused(&self) -> bool;
 
-    /// Get the current playback position of the player.
+    fn position(&mut self) -> Option<Duration>;
-    fn position(&self) -> Option<Duration>;
 
-    /// Get the duration of the currently playing track.
+    fn duration(&mut self) -> Option<Duration>;
-    fn duration(&self) -> Option<Duration>;
 
-    /// Seek relative to the current position.
-    ///
-    /// The position is capped at the duration of the song, and zero.
     fn seek_by(&mut self, seek_amount: Duration) -> Result<(), PlayerError>;
 
-    /// Seek absolutely within the song.
-    ///
-    /// The position is capped at the duration of the song, and zero.
     fn seek_to(&mut self, target_pos: Duration) -> Result<(), PlayerError>;
 
-    /// Return a reference to the player message channel, which can be cloned
-    /// in order to monitor messages from the player.
     fn message_channel(&self) -> &crossbeam::channel::Receiver<PlayerCommand>;
 }

src/music_storage/db_reader/itunes/reader.rs

@@ -353,6 +353,6 @@ mod tests {
         songs.iter().for_each(|song| library.add_song(song.to_owned()).unwrap());
 
         config.write_file().unwrap();
-        library.save(Arc::new(RwLock::from(config))).unwrap();
+        library.save(config).unwrap();
     }
 }

src/music_storage/library.rs

@@ -283,8 +283,8 @@ impl Song {
         }
 
         // Find images around the music file that can be used
-        let found_images = find_images(target_file.as_ref()).unwrap();
+        let mut found_images = find_images(target_file.as_ref()).unwrap();
-        album_art.extend_from_slice(&found_images);
+        album_art.append(&mut found_images);
 
         // Get the format as a string
         let format: Option<FileFormat> = match FileFormat::from_file(target_file) {
@@ -624,6 +624,8 @@ impl Album<'_> {
     }
 }
 
+const BLOCKED_EXTENSIONS: [&str; 4] = ["vob", "log", "txt", "sf2"];
+
 #[derive(Debug, Serialize, Deserialize)]
 pub struct MusicLibrary {
     pub name: String,
@@ -634,8 +636,6 @@ pub struct MusicLibrary {
 }
 
 impl MusicLibrary {
-    const BLOCKED_EXTENSIONS: &'static [&'static str] = &["vob", "log", "txt", "sf2"];
-
     /// Create a new library from a name and [Uuid]
     fn new(name: String, uuid: Uuid) -> Self {
         MusicLibrary {
@@ -654,7 +654,7 @@ impl MusicLibrary {
     /// the [MusicLibrary] Vec
     pub fn init(config: Arc<RwLock<Config>>, uuid: Uuid) -> Result<Self, Box<dyn Error>> {
         let global_config = &*config.read().unwrap();
-        let path = global_config.libraries.get_library(&uuid)?.path.clone();
+        let path = global_config.libraries.get_library(&uuid)?.path;
 
         let library: MusicLibrary = match path.exists() {
             true => read_file(path)?,
@@ -671,8 +671,7 @@ impl MusicLibrary {
     }
 
     //#[cfg(debug_assertions)] // We probably wouldn't want to use this for real, but maybe it would have some utility?
-    pub fn from_path<P: ?Sized + AsRef<Path>>(path: &P) -> Result<Self, Box<dyn Error>> {
+    pub fn from_path(path: PathBuf) -> Result<Self, Box<dyn Error>> {
-        let path: PathBuf = path.as_ref().to_path_buf();
         let library: MusicLibrary = match path.exists() {
             true => read_file(path)?,
             false => {
@@ -685,8 +684,8 @@ impl MusicLibrary {
     }
 
     /// Serializes the database out to the file specified in the config
-    pub fn save(&self, config: Arc<RwLock<Config>>) -> Result<(), Box<dyn Error>> {
+    pub fn save(&self, config: Config) -> Result<(), Box<dyn Error>> {
-        let path = config.read().unwrap().libraries.get_library(&self.uuid)?.path.clone();
+        let path = config.libraries.get_library(&self.uuid)?.path;
         match path.try_exists() {
             Ok(_) => write_file(self, path)?,
             Err(error) => return Err(error.into()),
@@ -716,7 +715,7 @@ impl MusicLibrary {
                 for location in &track.location {
                     //TODO: check that this works
                     if path == location {
-                        return Break((track, i));
+                        return std::ops::ControlFlow::Break((track, i));
                     }
                 }
                 Continue(())
@@ -766,7 +765,7 @@ impl MusicLibrary {
     }
 
     /// Finds all the audio files within a specified folder
-    pub fn scan_folder<P: ?Sized + AsRef<Path>>(&mut self, target_path: &P) -> Result<i32, Box<dyn std::error::Error>> {
+    pub fn scan_folder(&mut self, target_path: &str) -> Result<i32, Box<dyn std::error::Error>> {
         let mut total = 0;
         let mut errors = 0;
         for target_file in WalkDir::new(target_path)
@@ -804,29 +803,27 @@ impl MusicLibrary {
             // If it's a normal file, add it to the database
             // if it's a cuesheet, do a bunch of fancy stuff
             if (format.kind() == Kind::Audio || format.kind() == Kind::Video)
-                && !Self::BLOCKED_EXTENSIONS.contains(&extension.as_str())
+                && !BLOCKED_EXTENSIONS.contains(&extension.as_str())
             {
                 match self.add_file(target_file.path()) {
                     Ok(_) => total += 1,
                     Err(_error) => {
                         errors += 1;
-                        println!("{:?}: {}", target_file.file_name(), _error)
+                        //println!("{}, {:?}: {}", format, target_file.file_name(), _error)
                     } // TODO: Handle more of these errors
                 };
             } else if extension == "cue" {
                 total += match self.add_cuesheet(target_file.path()) {
                     Ok(added) => added,
-                    Err(_error) => {
+                    Err(error) => {
                         errors += 1;
-                        println!("{:?}: {}", target_file.file_name(), _error);
+                        //println!("{}", error);
                         0
                     }
                 }
             }
         }
 
-        println!("Total scanning errors: {}", errors);
-
         Ok(total)
     }