Improve song duration reporting, change to fork of playback library with duration fix

Cargo.lock

@@ -1898,8 +1898,6 @@ dependencies = [
 [[package]]
 name = "rodio"
 version = "0.17.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "bdf1d4dea18dff2e9eb6dca123724f8b60ef44ad74a9ad283cdfe025df7e73fa"
 dependencies = [
  "cpal",
  "symphonia",

interpreter/Cargo.toml

@@ -10,9 +10,7 @@ rust-version = "1.59"
 [dependencies]
 rusqlite = { version = "0.27", features = ["bundled"], optional = true }
 sqlparser = { version = "0.23", optional = true }
-symphonia = { version = "0.5", optional = true, features = [
+symphonia = { version = "0.5", optional = true, features = ["all"] }
-    "aac", "alac", "flac", "mp3", "pcm", "vorbis", "isomp4", "ogg", "wav"
-] }
 dirs = { version = "4" }
 regex = { version = "1" }
 rand = { version = "0.8" }

player/Cargo.toml

@@ -7,7 +7,7 @@ repository = "https://git.ngni.us/NGnius/muss"
 readme = "README.md"
 
 [dependencies]
-rodio = { version = "^0.17", features = ["symphonia-all"], default-features = false}
+rodio = { version = "0.17", features = ["symphonia-all"], path = "../../rodio", default-features = false}
 m3u8-rs = { version = "^3.0" }
 mpd = { version = "0.1", optional = true }
 

player/src/os_controls.rs

@@ -49,7 +49,8 @@ impl SystemControlWrapper {
             state: std::sync::Mutex::new(
                 State {
                     playback_time: 0,
-                    duration_cache: None,
+                    duration_cache: std::collections::HashMap::new(),
+                    current_item: None,
                 }
             ),
         }
@@ -164,7 +165,7 @@ impl SystemControlWrapper {
         }*/
     }
 
-    fn build_metadata(item: Item) -> Metadata {
+    fn build_metadata(item: Item, duration: Option<&std::time::Duration>) -> Metadata {
         let file_uri = item.field("filename").and_then(|x| x.to_owned().to_str());
         let cover_art = item.field("cover").and_then(|x| x.to_owned().to_str());
         let cover_url = if let Some(art) = &cover_art {
@@ -200,7 +201,7 @@ impl SystemControlWrapper {
             None
         };
         Metadata {
-            length: None, // populated separately
+            length: duration.map(|duration| duration.as_secs_f64().round() as i64 * 1_000_000),
             art_url: cover_url,
             album: item.field("album").and_then(|x| x.to_owned().to_str()),
             album_artist: item
@@ -232,10 +233,10 @@ impl SystemControlWrapper {
         }
     }
 
-    fn enqueued(item: Item, dbus_ctrl: &Sender<DbusControl>) {
+    fn enqueued(item: Item, dbus_ctrl: &Sender<DbusControl>, duration: Option<&std::time::Duration>) {
         //println!("Got enqueued item {}", &item.title);
         dbus_ctrl
-            .send(DbusControl::SetMetadata(Self::build_metadata(item)))
+            .send(DbusControl::SetMetadata(Self::build_metadata(item, duration)))
             .unwrap_or(());
     }
 
@@ -257,16 +258,15 @@ impl SystemControlWrapper {
             .unwrap_or(());
     }
 
-    fn time(item: Item, duration: std::time::Duration, dbus_ctrl: &Sender<DbusControl>) {
+    fn time(item: Item, duration: &std::time::Duration, dbus_ctrl: &Sender<DbusControl>) {
-        let mut meta = Self::build_metadata(item);
+        let meta = Self::build_metadata(item, Some(duration));
-        meta.length = Some(duration.as_secs_f64().round() as i64 * 1_000_000);
         dbus_ctrl.send(DbusControl::SetMetadata(meta)).unwrap_or(());
     }
 
     fn time_update(
         _item: Item,
         new_time: i64,
-        duration: &Option<std::time::Duration>,
+        duration: Option<&std::time::Duration>,
         dbus_ctrl: &Sender<DbusControl>,
     ) {
         //println!("Position update tick");
@@ -286,7 +286,8 @@ impl SystemControlWrapper {
 #[cfg(all(target_os = "linux", feature = "os-controls", feature = "mpris-player"))]
 struct State {
     playback_time: i64,
-    duration_cache: Option<std::time::Duration>,
+    duration_cache: std::collections::HashMap<Item, std::time::Duration>,
+    current_item: Option<Item>,
 }
 
 #[cfg(all(target_os = "linux", feature = "os-controls", feature = "mpris-player"))]
@@ -299,24 +300,29 @@ impl super::EventTap for SystemControlWrapper {
             PlaybackAction::Enqueued(item) => {
                 let mut state = self.state.lock().unwrap();
                 state.playback_time = 0;
-                state.duration_cache = None;
+                state.current_item = Some(item.to_owned());
-                Self::enqueued(item.to_owned(), self.dbus_ctrl.as_ref().unwrap());
+                Self::enqueued(item.to_owned(), self.dbus_ctrl.as_ref().unwrap(), state.duration_cache.get(item));
+            },
+            PlaybackAction::Empty => {
+                Self::empty(self.dbus_ctrl.as_ref().unwrap());
+                let mut state = self.state.lock().unwrap();
+                state.playback_time = 0;
+                state.current_item = None;
             },
-            PlaybackAction::Empty => Self::empty(self.dbus_ctrl.as_ref().unwrap()),
             PlaybackAction::Time(item, duration) => {
                 let mut state = self.state.lock().unwrap();
-                state.duration_cache = Some(duration.to_owned());
+                state.duration_cache.insert(item.to_owned(), duration.to_owned());
-                Self::time(item.to_owned(), duration.to_owned(), self.dbus_ctrl.as_ref().unwrap());
+                Self::time(item.to_owned(), duration, self.dbus_ctrl.as_ref().unwrap());
             }
-            PlaybackAction::UpdateTick(item) => {
+            PlaybackAction::UpdateTick(item, tick) => {
                 let mut state = self.state.lock().unwrap();
+                state.playback_time = *tick as i64;
                 Self::time_update(
                     item.to_owned(),
                     state.playback_time,
-                    &state.duration_cache,
+                    state.duration_cache.get(item),
                     self.dbus_ctrl.as_ref().unwrap(),
                 );
-                state.playback_time += 1;
             }
         }
         None

player/src/player_wrapper.rs

@@ -47,7 +47,9 @@ impl<I: std::iter::Iterator<Item = Result<Item, InterpreterError>>> PlayerServer
         let event = std::sync::Arc::new(std::sync::Mutex::new(self.playback.clone()));
         move |source_in, item| {
             let event2 = event.clone();
+            let mut update_tick_num = 0;
             if let Some(duration) = source_in.total_duration() {
+                //println!("Got duration {:?}", duration);
                 event
                     .lock()
                     .map(|event| {
@@ -62,8 +64,9 @@ impl<I: std::iter::Iterator<Item = Result<Item, InterpreterError>>> PlayerServer
                         event2
                             .lock()
                             .map(|x| {
-                                x.send(PlaybackAction::UpdateTick(item.clone()))
+                                x.send(PlaybackAction::UpdateTick(item.clone(), update_tick_num))
-                                    .unwrap_or(())
+                                    .unwrap_or(());
+                                update_tick_num += 1;
                             })
                             .unwrap_or(());
                     }),
@@ -96,8 +99,9 @@ impl<I: std::iter::Iterator<Item = Result<Item, InterpreterError>>> PlayerServer
                         event2
                             .lock()
                             .map(|x| {
-                                x.send(PlaybackAction::UpdateTick(item.clone()))
+                                x.send(PlaybackAction::UpdateTick(item.clone(), update_tick_num))
-                                    .unwrap_or(())
+                                    .unwrap_or(());
+                                update_tick_num += 1;
                             })
                             .unwrap_or(());
                     }),
@@ -316,7 +320,7 @@ pub enum PlaybackAction {
     Empty,
     Enqueued(Item),
     Time(Item, std::time::Duration),
-    UpdateTick(Item), // tick sent once every second
+    UpdateTick(Item, u64), // tick sent once every second
     Exit,
 }
 

src/debug_state.rs

@@ -3,26 +3,46 @@ use muss_interpreter::Item;
 use muss_player::EventTap;
 use muss_player::{PlaybackAction, PlayerAction, ControlAction};
 
+
+#[derive(Clone)]
+pub struct InterpreterDebugState {
+    pub debug_flag: InterpreterDebugFlag,
+    pub verbose: bool,
+}
+
+#[derive(Copy, Clone)]
+pub enum InterpreterDebugFlag {
+    Skip,
+    List,
+    Normal,
+}
+
+#[derive(Clone)]
 pub struct DebugState {
-    pub now_playing: Option<Item>,
+    pub now_playing: std::sync::Arc<std::sync::RwLock<Option<Item>>>,
-    pub control_tx: Option<std::sync::Mutex<std::sync::mpsc::Sender<ControlAction>>>,
+    pub control_tx: std::sync::Arc<std::sync::Mutex<Option<std::sync::mpsc::Sender<ControlAction>>>>,
+    pub interpreter: std::sync::Arc<std::sync::RwLock<InterpreterDebugState>>,
 }
 
 impl DebugState {
-    pub fn new() -> std::sync::Arc<std::sync::RwLock<Self>> {
+    pub fn new() -> Self {
-        std::sync::Arc::new(std::sync::RwLock::new(Self {
+        Self {
-            now_playing: None,
+            now_playing: std::sync::Arc::new(std::sync::RwLock::new(None)),
-            control_tx: None,
+            control_tx: std::sync::Arc::new(std::sync::Mutex::new(None)),
-        }))
+            interpreter: std::sync::Arc::new(std::sync::RwLock::new(InterpreterDebugState {
+                debug_flag: InterpreterDebugFlag::Normal,
+                verbose: false,
+            })),
+        }
     }
 }
 
 pub struct DebugEventHandler {
-    state: std::sync::Arc<std::sync::RwLock<DebugState>>,
+    state: DebugState,
 }
 
 impl DebugEventHandler {
-    pub fn new(debug_state: std::sync::Arc<std::sync::RwLock<DebugState>>) -> Self {
+    pub fn new(debug_state: DebugState) -> Self {
         Self {
             state: debug_state,
         }
@@ -33,15 +53,15 @@ impl EventTap for DebugEventHandler {
     fn on_playback(&self, playback: &PlaybackAction) -> Option<ControlAction> {
         match playback {
             PlaybackAction::Enqueued(item) => {
-                let mut state = self.state.write().unwrap();
+                let mut now_playing = self.state.now_playing.write().unwrap();
-                state.now_playing = Some(item.to_owned());
+                *now_playing = Some(item.to_owned());
             },
             PlaybackAction::Empty | PlaybackAction::Exit => {
-                let mut state = self.state.write().unwrap();
+                let mut now_playing = self.state.now_playing.write().unwrap();
-                state.now_playing = None;
+                *now_playing = None;
             },
             PlaybackAction::Time(_item, _dur) => {},
-            PlaybackAction::UpdateTick(_item) => {},
+            PlaybackAction::UpdateTick(_item, _tick) => {},
         }
         None
     }
@@ -51,6 +71,6 @@ impl EventTap for DebugEventHandler {
     }
 
     fn init_control(&mut self, control: &std::sync::mpsc::Sender<ControlAction>) {
-        self.state.write().expect("Failed to get write lock on controller debug state").control_tx = Some(std::sync::Mutex::new(control.clone()));
+        *self.state.control_tx.lock().expect("Failed to get write lock on controller debug state") = Some(control.clone());
     }
 }

src/playlists.rs

@@ -84,6 +84,6 @@ fn music_filename(item: &Item) -> Option<String> {
 }
 
 pub fn get_current_item(state: &mut ReplState) -> Option<muss_interpreter::Item> {
-    let data = state.controller_debug.read().expect("Failed to get read lock for debug player data");
+    let now_playing = state.controller_debug.now_playing.read().expect("Failed to get read lock for debug now_playing data");
-    data.now_playing.clone()
+    now_playing.clone()
 }

src/repl.rs

@@ -2,9 +2,6 @@
 #![allow(clippy::single_match)]
 use std::io::{self, Write};
 use std::sync::mpsc::{self, Receiver};
-use std::sync::RwLock;
-
-use lazy_static::lazy_static;
 
 use console::{Key, Term};
 
@@ -15,13 +12,6 @@ use muss_player::{Controller, Player};
 use super::channel_io::{channel_io, ChannelWriter};
 use super::cli::CliArgs;
 
-lazy_static! {
-    static ref DEBUG_STATE: RwLock<DebugState> = RwLock::new(DebugState {
-        debug_flag: DebugFlag::Normal,
-        verbose: false,
-    });
-}
-
 pub const TERMINAL_WRITE_ERROR: &str = "Failed to write to terminal output";
 const INTERPRETER_WRITE_ERROR: &str = "Failed to write to interpreter";
 
@@ -41,20 +31,7 @@ pub struct ReplState {
     cursor_rightward_position: usize,
     //debug: Arc<RwLock<DebugState>>,
     list_rx: Receiver<DebugItem>,
-    pub controller_debug: std::sync::Arc<std::sync::RwLock<crate::debug_state::DebugState>>,
+    pub controller_debug: crate::debug_state::DebugState,
-}
-
-#[derive(Clone)]
-struct DebugState {
-    debug_flag: DebugFlag,
-    verbose: bool,
-}
-
-#[derive(Copy, Clone)]
-enum DebugFlag {
-    Skip,
-    List,
-    Normal,
 }
 
 impl ReplState {
@@ -80,19 +57,22 @@ impl ReplState {
     }
 }
 
-fn interpreter_event_callback<T: muss_interpreter::tokens::TokenReader>(
+fn interpreter_event_callback<'a, T: muss_interpreter::tokens::TokenReader>(
-    _interpreter: &mut Interpreter<'_, T>,
+    debug_state: crate::debug_state::DebugState,
-    event: InterpreterEvent,
+) -> impl Fn(
+    &mut Interpreter<'a, T>, InterpreterEvent,
 ) -> Result<(), InterpreterError> {
-    match event {
+    move |_interpreter, event| {
-        InterpreterEvent::StatementComplete => {
+        match event {
-            if let Ok(mut d_state) = DEBUG_STATE.write() {
+            InterpreterEvent::StatementComplete => {
-                d_state.debug_flag = DebugFlag::Normal;
+                if let Ok(mut d_state) = debug_state.interpreter.write() {
+                    d_state.debug_flag = crate::debug_state::InterpreterDebugFlag::Normal;
+                }
             }
+            _ => {}
         }
-        _ => {}
+        Ok(())
     }
-    Ok(())
 }
 
 #[inline]
@@ -148,7 +128,7 @@ fn pretty_print_item(item: &Item, terminal: &mut Term, args: &CliArgs, verbose:
 
 fn handle_list_rx(state: &mut ReplState, args: &CliArgs) {
     //let items = state.list_rx.try_iter().collect::<Vec<_>>();
-    let d_state = DEBUG_STATE
+    let d_state = state.controller_debug.interpreter
         .read()
         .expect("Failed to get read lock for debug state info")
         .clone();
@@ -162,12 +142,14 @@ fn handle_list_rx(state: &mut ReplState, args: &CliArgs) {
         }
     }
     let flag = d_state.debug_flag;
+    drop(d_state);
     match flag {
-        DebugFlag::List => {
+        crate::debug_state::InterpreterDebugFlag::List => {
             while let Ok(item) = state.list_rx.recv() {
                 match item {
                     Ok(item) => {
-                        pretty_print_item(&item, &mut state.terminal, args, d_state.verbose)
+                        let verbose = state.controller_debug.interpreter.read().map(|x| x.verbose).unwrap_or(false);
+                        pretty_print_item(&item, &mut state.terminal, args, verbose)
                     }
                     Err(e) => error_prompt(
                         muss_player::PlayerError::Playback(muss_player::PlaybackError::from_err(e)),
@@ -175,13 +157,13 @@ fn handle_list_rx(state: &mut ReplState, args: &CliArgs) {
                     ),
                 }
                 // stop listing if no longer in list mode
-                let flag = if let Ok(d_state) = DEBUG_STATE.read() {
+                let flag = if let Ok(d_state) = state.controller_debug.interpreter.read() {
                     d_state.debug_flag
                 } else {
-                    DebugFlag::Normal
+                    crate::debug_state::InterpreterDebugFlag::Normal
                 };
                 match flag {
-                    DebugFlag::List => {}
+                    crate::debug_state::InterpreterDebugFlag::List => {}
                     _ => break,
                 }
             }
@@ -213,23 +195,27 @@ pub fn repl(args: CliArgs) {
     };
     let (list_tx, list_rx) = mpsc::channel();
     let mut state = ReplState::new(writer, term, list_rx);
+    let controller_debug_clone = state.controller_debug.clone();
     let player_builder = move || {
-        let runner = Interpreter::with_stream_and_callback(reader, &interpreter_event_callback);
+        let callback_handler = Box::new(interpreter_event_callback(controller_debug_clone.clone()));
+        // FIXME don't leak memory for callback handler
+        // (this is fine since it'll only get called once, it just looks bad)
+        let runner = Interpreter::with_stream_and_callback(reader, Box::leak(callback_handler));
         let debugger = Debugger::new(runner, move |interpretor, item| {
-            let flag = if let Ok(d_state) = DEBUG_STATE.read() {
+            let flag = if let Ok(d_state) = controller_debug_clone.interpreter.read() {
                 d_state.debug_flag
             } else {
-                DebugFlag::Normal
+                crate::debug_state::InterpreterDebugFlag::Normal
             };
             match flag {
-                DebugFlag::Normal => item,
+                crate::debug_state::InterpreterDebugFlag::Normal => item,
-                DebugFlag::Skip => {
+                crate::debug_state::InterpreterDebugFlag::Skip => {
                     for _ in interpretor.by_ref() {
                         // NOTE: recursion occurs here
                     }
                     None
                 }
-                DebugFlag::List => {
+                crate::debug_state::InterpreterDebugFlag::List => {
                     if let Some(x) = item {
                         list_tx.send(x.map_err(|e| e.to_string())).unwrap_or(());
                         for x in interpretor.by_ref() {
@@ -713,34 +699,34 @@ fn repl_commands(command_str: &str, state: &mut ReplState, args: &CliArgs) {
         }
         "?list" => {
             {
-                let mut debug_state = DEBUG_STATE
+                let mut debug_state = state.controller_debug.interpreter
                     .write()
                     .expect("Failed to get write lock for debug state");
-                debug_state.debug_flag = DebugFlag::List;
+                debug_state.debug_flag = crate::debug_state::InterpreterDebugFlag::List;
             }
             writeln!(state.terminal, "Listing upcoming items").expect(TERMINAL_WRITE_ERROR);
         }
         "?skip" => {
             {
-                let mut debug_state = DEBUG_STATE
+                let mut debug_state = state.controller_debug.interpreter
                     .write()
                     .expect("Failed to get write lock for debug state");
-                debug_state.debug_flag = DebugFlag::Skip;
+                debug_state.debug_flag = crate::debug_state::InterpreterDebugFlag::Skip;
             }
             writeln!(state.terminal, "Skipping upcoming items").expect(TERMINAL_WRITE_ERROR);
         }
         "?normal" => {
             {
-                let mut debug_state = DEBUG_STATE
+                let mut debug_state = state.controller_debug.interpreter
                     .write()
                     .expect("Failed to get write lock for debug state");
-                debug_state.debug_flag = DebugFlag::Normal;
+                debug_state.debug_flag = crate::debug_state::InterpreterDebugFlag::Normal;
             }
             writeln!(state.terminal, "Resuming normal operation").expect(TERMINAL_WRITE_ERROR);
         }
         "?verbose" => {
             let verbose = {
-                let mut debug_state = DEBUG_STATE
+                let mut debug_state = state.controller_debug.interpreter
                     .write()
                     .expect("Failed to get write lock for debug state");
                 debug_state.verbose = !debug_state.verbose;
@@ -754,7 +740,7 @@ fn repl_commands(command_str: &str, state: &mut ReplState, args: &CliArgs) {
         }
         "?now" => {
             if let Some(item) = crate::playlists::get_current_item(state) {
-                let verbose = DEBUG_STATE
+                let verbose = state.controller_debug.interpreter
                     .read()
                     .expect("Failed to get read lock for debug state")
                     .verbose;
@@ -764,46 +750,38 @@ fn repl_commands(command_str: &str, state: &mut ReplState, args: &CliArgs) {
             }
         }
         "?next" => {
-            state.controller_debug
+            state.controller_debug.control_tx
-                .read()
-                .expect("Failed to get read lock for debug player data")
-                .control_tx.as_ref()
-                .expect("Control action sender shouldn't be None")
                 .lock()
                 .expect("Failed to get lock for control action sender")
+                .as_ref()
+                .expect("Control action sender shouldn't be None")
                 .send(muss_player::ControlAction::Next { ack: false })
                 .expect("Failed to send control action");
         },
         "?previous" => {
-            state.controller_debug
+            state.controller_debug.control_tx
-                .read()
-                .expect("Failed to get read lock for debug player data")
-                .control_tx.as_ref()
-                .expect("Control action sender shouldn't be None")
                 .lock()
                 .expect("Failed to get lock for control action sender")
+                .as_ref()
+                .expect("Control action sender shouldn't be None")
                 .send(muss_player::ControlAction::Previous { ack: false })
                 .expect("Failed to send control action");
         },
         "?pause" => {
-            state.controller_debug
+            state.controller_debug.control_tx
-                .read()
-                .expect("Failed to get read lock for debug player data")
-                .control_tx.as_ref()
-                .expect("Control action sender shouldn't be None")
                 .lock()
                 .expect("Failed to get lock for control action sender")
+                .as_ref()
+                .expect("Control action sender shouldn't be None")
                 .send(muss_player::ControlAction::Pause { ack: false })
                 .expect("Failed to send control action");
         }
         "?play" => {
-            state.controller_debug
+            state.controller_debug.control_tx
-                .read()
-                .expect("Failed to get read lock for debug player data")
-                .control_tx.as_ref()
-                .expect("Control action sender shouldn't be None")
                 .lock()
                 .expect("Failed to get lock for control action sender")
+                .as_ref()
+                .expect("Control action sender shouldn't be None")
                 .send(muss_player::ControlAction::Play { ack: false })
                 .expect("Failed to send control action");
         }
@@ -817,13 +795,11 @@ fn volume_cmd(cmd_args: &[&str], state: &mut ReplState) {
     if let Some(volume_arg) = cmd_args.get(1) {
         let volume_result: Result<f32, _> = volume_arg.parse();
         match volume_result {
-            Ok(vol) => state.controller_debug
+            Ok(vol) => state.controller_debug.control_tx
-                .read()
-                .expect("Failed to get read lock for debug player data")
-                .control_tx.as_ref()
-                .expect("Control action sender shouldn't be None")
                 .lock()
                 .expect("Failed to get lock for control action sender")
+                .as_ref()
+                .expect("Control action sender shouldn't be None")
                 .send(muss_player::ControlAction::SetVolume { ack: false, volume: (vol * 100.0).round() as _ })
                 .expect("Failed to send control action"),
             Err(e) => writeln!(state.terminal, "Error parsing ?volume number parameter: {}", e).expect(TERMINAL_WRITE_ERROR)