From 77bea07c4f13f7f3f41ae723e2052cabe61d027f Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Wed, 14 Feb 2018 23:39:44 +0100 Subject: [PATCH 01/13] Run onstart/onstop when a new song is loaded --- playback/src/player.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/playback/src/player.rs b/playback/src/player.rs index e549736..bbcdb37 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -288,14 +288,13 @@ impl PlayerInternal { PlayerCommand::Load(track_id, play, position, end_of_track) => { if self.state.is_playing() { self.stop_sink_if_running(); + self.run_onstop(); } match self.load_track(track_id, position as i64) { Some(decoder) => { if play { - if !self.state.is_playing() { - self.run_onstart(); - } + self.run_onstart(); self.start_sink(); self.state = PlayerState::Playing { @@ -303,10 +302,6 @@ impl PlayerInternal { end_of_track: end_of_track, }; } else { - if self.state.is_playing() { - self.run_onstop(); - } - self.state = PlayerState::Paused { decoder: decoder, end_of_track: end_of_track, @@ -316,9 +311,6 @@ impl PlayerInternal { None => { let _ = end_of_track.send(()); - if self.state.is_playing() { - self.run_onstop(); - } } } } From b0ee03112fdb54eec2b9e0b91cd8558b48a77cb0 Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Fri, 16 Feb 2018 00:16:38 +0100 Subject: [PATCH 02/13] First attempt at a better playback event system. --- playback/src/config.rs | 24 ++++++++-- playback/src/player.rs | 87 ++++++++++++++++++++----------------- src/main.rs | 9 +++- src/player_event_handler.rs | 50 +++++++++++++++++++++ 4 files changed, 125 insertions(+), 45 deletions(-) create mode 100644 src/player_event_handler.rs diff --git a/playback/src/config.rs b/playback/src/config.rs index d44e937..f0fd13d 100644 --- a/playback/src/config.rs +++ b/playback/src/config.rs @@ -1,4 +1,6 @@ use std::str::FromStr; +use core::spotify_id::SpotifyId; +use std::sync::mpsc::Sender; #[derive(Clone, Copy, Debug, Hash, PartialOrd, Ord, PartialEq, Eq)] pub enum Bitrate { @@ -25,19 +27,33 @@ impl Default for Bitrate { } } +#[derive(Debug, Clone)] +pub enum PlayerEvent { + Started { + track_id: SpotifyId, + }, + + Changed { + old_track_id: SpotifyId, + new_track_id: SpotifyId, + }, + + Stopped { + track_id: SpotifyId, + } +} + #[derive(Clone, Debug)] pub struct PlayerConfig { pub bitrate: Bitrate, - pub onstart: Option, - pub onstop: Option, + pub event_sender : Option>, } impl Default for PlayerConfig { fn default() -> PlayerConfig { PlayerConfig { bitrate: Bitrate::default(), - onstart: None, - onstop: None, + event_sender: None, } } } diff --git a/playback/src/player.rs b/playback/src/player.rs index bbcdb37..593e7f1 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -4,12 +4,11 @@ use std; use std::borrow::Cow; use std::io::{Read, Seek, SeekFrom, Result}; use std::mem; -use std::process::Command; use std::sync::mpsc::{RecvError, TryRecvError, RecvTimeoutError}; use std::thread; use std::time::Duration; -use config::{Bitrate, PlayerConfig}; +use config::{Bitrate, PlayerConfig, PlayerEvent}; use core::session::Session; use core::spotify_id::SpotifyId; @@ -121,14 +120,16 @@ type Decoder = VorbisDecoder>>; enum PlayerState { Stopped, Paused { + track_id: SpotifyId, decoder: Decoder, end_of_track: oneshot::Sender<()>, }, Playing { + track_id: SpotifyId, decoder: Decoder, end_of_track: oneshot::Sender<()>, }, - + EndOfTrack { track_id: SpotifyId }, Invalid, } @@ -136,7 +137,7 @@ impl PlayerState { fn is_playing(&self) -> bool { use self::PlayerState::*; match *self { - Stopped | Paused { .. } => false, + Stopped | EndOfTrack { .. } | Paused { .. } => false, Playing { .. } => true, Invalid => panic!("invalid state"), } @@ -145,7 +146,7 @@ impl PlayerState { fn decoder(&mut self) -> Option<&mut Decoder> { use self::PlayerState::*; match *self { - Stopped => None, + Stopped | EndOfTrack { .. } => None, Paused { ref mut decoder, .. } | Playing { ref mut decoder, .. } => Some(decoder), Invalid => panic!("invalid state"), @@ -160,6 +161,7 @@ impl PlayerState { let _ = end_of_track.send(()); } + EndOfTrack { .. } => warn!("signal_end_of_track from end of track state"), Stopped => warn!("signal_end_of_track from stopped state"), Invalid => panic!("invalid state"), } @@ -168,10 +170,11 @@ impl PlayerState { fn paused_to_playing(&mut self) { use self::PlayerState::*; match ::std::mem::replace(self, Invalid) { - Paused { decoder, end_of_track } => { + Paused { decoder, end_of_track, track_id } => { *self = Playing { decoder: decoder, end_of_track: end_of_track, + track_id: track_id, }; } _ => panic!("invalid state"), @@ -181,10 +184,11 @@ impl PlayerState { fn playing_to_paused(&mut self) { use self::PlayerState::*; match ::std::mem::replace(self, Invalid) { - Playing { decoder, end_of_track } => { + Playing { decoder, end_of_track, track_id } => { *self = Paused { decoder: decoder, end_of_track: end_of_track, + track_id: track_id, }; } _ => panic!("invalid state"), @@ -274,9 +278,15 @@ impl PlayerInternal { None => { self.stop_sink(); - self.run_onstop(); - let old_state = mem::replace(&mut self.state, PlayerState::Stopped); + let new_state = match self.state { + PlayerState::Playing { track_id, .. } + | PlayerState::Paused { track_id, .. } => + PlayerState::EndOfTrack { track_id }, + _ => PlayerState::Stopped, + }; + + let old_state = mem::replace(&mut self.state, new_state); old_state.signal_end_of_track(); } } @@ -288,24 +298,35 @@ impl PlayerInternal { PlayerCommand::Load(track_id, play, position, end_of_track) => { if self.state.is_playing() { self.stop_sink_if_running(); - self.run_onstop(); } match self.load_track(track_id, position as i64) { Some(decoder) => { if play { - self.run_onstart(); + match self.state { + PlayerState::Playing { track_id: old_track_id, ..} + | PlayerState::EndOfTrack { track_id: old_track_id, .. } => + self.send_event(PlayerEvent::Changed { + old_track_id: old_track_id, + new_track_id: track_id + }), + _ => self.send_event(PlayerEvent::Started { track_id }), + } + self.start_sink(); self.state = PlayerState::Playing { + track_id: track_id, decoder: decoder, end_of_track: end_of_track, }; } else { self.state = PlayerState::Paused { + track_id: track_id, decoder: decoder, end_of_track: end_of_track, }; + self.send_event(PlayerEvent::Stopped { track_id }); } } @@ -327,10 +348,10 @@ impl PlayerInternal { } PlayerCommand::Play => { - if let PlayerState::Paused { .. } = self.state { + if let PlayerState::Paused { track_id, .. } = self.state { self.state.paused_to_playing(); - self.run_onstart(); + self.send_event(PlayerEvent::Started { track_id }); self.start_sink(); } else { warn!("Player::play called from invalid state"); @@ -338,11 +359,11 @@ impl PlayerInternal { } PlayerCommand::Pause => { - if let PlayerState::Playing { .. } = self.state { + if let PlayerState::Playing { track_id, .. } = self.state { self.state.playing_to_paused(); self.stop_sink_if_running(); - self.run_onstop(); + self.send_event(PlayerEvent::Stopped { track_id }); } else { warn!("Player::pause called from invalid state"); } @@ -350,12 +371,11 @@ impl PlayerInternal { PlayerCommand::Stop => { match self.state { - PlayerState::Playing { .. } => { + PlayerState::Playing { track_id, .. } + | PlayerState::Paused { track_id, .. } + | PlayerState::EndOfTrack { track_id } => { self.stop_sink_if_running(); - self.run_onstop(); - self.state = PlayerState::Stopped; - } - PlayerState::Paused { .. } => { + self.send_event(PlayerEvent::Stopped { track_id }); self.state = PlayerState::Stopped; }, PlayerState::Stopped => { @@ -367,15 +387,14 @@ impl PlayerInternal { } } - fn run_onstart(&self) { - if let Some(ref program) = self.config.onstart { - run_program(program) - } - } - - fn run_onstop(&self) { - if let Some(ref program) = self.config.onstop { - run_program(program) + fn send_event(&mut self, event: PlayerEvent) { + match self.config.event_sender { + Some(ref s) => + match s.send(event.clone()) { + Ok(_) => info!("Sent event {:?} to event listener.", event), + Err(err) => error!("Failed to send event {:?} to listener: {:?}", event, err) + } + None => () } } @@ -509,13 +528,3 @@ impl Seek for Subfile { } } } - -fn run_program(program: &str) { - info!("Running {}", program); - let mut v: Vec<&str> = program.split_whitespace().collect(); - let status = Command::new(&v.remove(0)) - .args(&v) - .status() - .expect("program failed to start"); - info!("Exit status: {}", status); -} diff --git a/src/main.rs b/src/main.rs index 01c3442..8251c3f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -31,6 +31,9 @@ use librespot::playback::mixer::{self, Mixer}; use librespot::playback::player::Player; use librespot::connect::spirc::{Spirc, SpircTask}; +mod player_event_handler; +use player_event_handler::run_program_on_events; + fn usage(program: &str, opts: &getopts::Options) -> String { let brief = format!("Usage: {} [options]", program); opts.usage(&brief) @@ -94,6 +97,7 @@ fn setup(args: &[String]) -> Setup { .optopt("b", "bitrate", "Bitrate (96, 160 or 320). Defaults to 160", "BITRATE") .optopt("", "onstart", "Run PROGRAM when playback is about to begin.", "PROGRAM") .optopt("", "onstop", "Run PROGRAM when playback has ended.", "PROGRAM") + .optopt("", "onchange", "Run PROGRAM between two tracks.", "PROGRAM") .optflag("v", "verbose", "Enable verbose output") .optopt("u", "username", "Username to sign in with", "USERNAME") .optopt("p", "password", "Password", "PASSWORD") @@ -185,8 +189,9 @@ fn setup(args: &[String]) -> Setup { PlayerConfig { bitrate: bitrate, - onstart: matches.opt_str("onstart"), - onstop: matches.opt_str("onstop"), + event_sender: run_program_on_events(matches.opt_str("onstart"), + matches.opt_str("onstop"), + matches.opt_str("onchange")) } }; diff --git a/src/player_event_handler.rs b/src/player_event_handler.rs new file mode 100644 index 0000000..25724d1 --- /dev/null +++ b/src/player_event_handler.rs @@ -0,0 +1,50 @@ +use std::process::Command; +use std::sync::mpsc::{channel, Sender}; +use std::thread; +use librespot::playback::config::PlayerEvent; + +fn run_program(program: &str, args: Vec) { + info!("Running {}", program); + let mut v: Vec<&str> = program.split_whitespace().collect(); + let status = Command::new(&v.remove(0)) + .args(&v) + .args(args) + .status() + .expect("program failed to start"); + info!("Exit status: {}", status); +} + +pub fn run_program_on_events(onstart: Option, + onstop: Option, + onchange: Option) -> Option> { + if onstart.is_none() && onstop.is_none() && onchange.is_none() { + None + } else { + let (sender, receiver) = channel(); + thread::spawn(move || { + while let Ok(msg) = receiver.recv() { + match msg { + PlayerEvent::Changed { old_track_id, new_track_id } => { + let args = vec![old_track_id.to_base16(), new_track_id.to_base16()]; + if let Some(ref onchange) = onchange.as_ref() { + run_program(onchange, args); + } + }, + PlayerEvent::Started { track_id } => { + let args = vec![track_id.to_base16()]; + if let Some(ref onstart) = onstart.as_ref() { + run_program(onstart, args); + } + } + PlayerEvent::Stopped { track_id } => { + let args = vec![track_id.to_base16()]; + if let Some(ref onstop) = onstop.as_ref() { + run_program(onstop, args); + } + } + } + } + }); + Some(sender) + } +} From 1b943d069f34617c5d86db74730869536ddd21e7 Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Tue, 20 Feb 2018 21:57:42 +0100 Subject: [PATCH 03/13] Move PlayerEvent into player. --- playback/src/config.rs | 17 +---------------- playback/src/player.rs | 19 ++++++++++++++++++- src/player_event_handler.rs | 2 +- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/playback/src/config.rs b/playback/src/config.rs index f0fd13d..6861142 100644 --- a/playback/src/config.rs +++ b/playback/src/config.rs @@ -1,6 +1,7 @@ use std::str::FromStr; use core::spotify_id::SpotifyId; use std::sync::mpsc::Sender; +use player::PlayerEvent; #[derive(Clone, Copy, Debug, Hash, PartialOrd, Ord, PartialEq, Eq)] pub enum Bitrate { @@ -27,22 +28,6 @@ impl Default for Bitrate { } } -#[derive(Debug, Clone)] -pub enum PlayerEvent { - Started { - track_id: SpotifyId, - }, - - Changed { - old_track_id: SpotifyId, - new_track_id: SpotifyId, - }, - - Stopped { - track_id: SpotifyId, - } -} - #[derive(Clone, Debug)] pub struct PlayerConfig { pub bitrate: Bitrate, diff --git a/playback/src/player.rs b/playback/src/player.rs index 593e7f1..9546a49 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -8,7 +8,7 @@ use std::sync::mpsc::{RecvError, TryRecvError, RecvTimeoutError}; use std::thread; use std::time::Duration; -use config::{Bitrate, PlayerConfig, PlayerEvent}; +use config::{Bitrate, PlayerConfig}; use core::session::Session; use core::spotify_id::SpotifyId; @@ -42,6 +42,23 @@ enum PlayerCommand { Seek(u32), } +#[derive(Debug, Clone)] +pub enum PlayerEvent { + Started { + track_id: SpotifyId, + }, + + Changed { + old_track_id: SpotifyId, + new_track_id: SpotifyId, + }, + + Stopped { + track_id: SpotifyId, + } +} + + impl Player { pub fn new(config: PlayerConfig, session: Session, audio_filter: Option>, diff --git a/src/player_event_handler.rs b/src/player_event_handler.rs index 25724d1..8d87276 100644 --- a/src/player_event_handler.rs +++ b/src/player_event_handler.rs @@ -1,7 +1,7 @@ use std::process::Command; use std::sync::mpsc::{channel, Sender}; use std::thread; -use librespot::playback::config::PlayerEvent; +use librespot::playback::player::PlayerEvent; fn run_program(program: &str, args: Vec) { info!("Running {}", program); From 0a6825ba61e1c321b13f407f8b7da29ba420a32f Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Tue, 20 Feb 2018 21:58:02 +0100 Subject: [PATCH 04/13] Add playing_to_end_of_track method to PlayerState. --- playback/src/player.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/playback/src/player.rs b/playback/src/player.rs index 9546a49..7438b74 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -170,12 +170,24 @@ impl PlayerState { } } - fn signal_end_of_track(self) { + fn send_end_of_track(self) { use self::PlayerState::*; match self { Paused { end_of_track, .. } | Playing { end_of_track, .. } => { let _ = end_of_track.send(()); + }, + _ => () + } + } + + fn playing_to_end_of_track(&mut self) { + use self::PlayerState::*; + match *self { + Paused { track_id, .. } | + Playing { track_id, .. } => { + let old_state = mem::replace(self, EndOfTrack { track_id }); + old_state.send_end_of_track(); } EndOfTrack { .. } => warn!("signal_end_of_track from end of track state"), @@ -295,16 +307,7 @@ impl PlayerInternal { None => { self.stop_sink(); - - let new_state = match self.state { - PlayerState::Playing { track_id, .. } - | PlayerState::Paused { track_id, .. } => - PlayerState::EndOfTrack { track_id }, - _ => PlayerState::Stopped, - }; - - let old_state = mem::replace(&mut self.state, new_state); - old_state.signal_end_of_track(); + self.state.playing_to_end_of_track(); } } } From 3e2e6d63f7c3a6216d49ddb1d57c7f89c09a6981 Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Tue, 20 Feb 2018 22:03:21 +0100 Subject: [PATCH 05/13] Send Changed event after song change even if we stop playback. --- playback/src/player.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/playback/src/player.rs b/playback/src/player.rs index 7438b74..ebf2441 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -346,6 +346,15 @@ impl PlayerInternal { decoder: decoder, end_of_track: end_of_track, }; + match self.state { + PlayerState::Playing { track_id: old_track_id, ..} + | PlayerState::EndOfTrack { track_id: old_track_id, .. } => + self.send_event(PlayerEvent::Changed { + old_track_id: old_track_id, + new_track_id: track_id + }), + _ => (), + } self.send_event(PlayerEvent::Stopped { track_id }); } } From ef48afbf418759b04af5d6752501a590511e80a0 Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Tue, 20 Feb 2018 22:05:05 +0100 Subject: [PATCH 06/13] Simplify match with if let. --- playback/src/player.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/playback/src/player.rs b/playback/src/player.rs index ebf2441..c0a8a35 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -417,13 +417,11 @@ impl PlayerInternal { } fn send_event(&mut self, event: PlayerEvent) { - match self.config.event_sender { - Some(ref s) => - match s.send(event.clone()) { - Ok(_) => info!("Sent event {:?} to event listener.", event), - Err(err) => error!("Failed to send event {:?} to listener: {:?}", event, err) - } - None => () + if let Some(ref s) = self.config.event_sender { + match s.send(event.clone()) { + Ok(_) => info!("Sent event {:?} to event listener.", event), + Err(err) => error!("Failed to send event {:?} to listener: {:?}", event, err) + } } } From 081a282e12ce755a0a8e301a95568b6ac6400450 Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Tue, 20 Feb 2018 22:09:53 +0100 Subject: [PATCH 07/13] Removed unreachable cases. --- playback/src/player.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/playback/src/player.rs b/playback/src/player.rs index c0a8a35..778368b 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -188,11 +188,8 @@ impl PlayerState { Playing { track_id, .. } => { let old_state = mem::replace(self, EndOfTrack { track_id }); old_state.send_end_of_track(); - } - - EndOfTrack { .. } => warn!("signal_end_of_track from end of track state"), - Stopped => warn!("signal_end_of_track from stopped state"), - Invalid => panic!("invalid state"), + }, + _ => panic!("Called playing_to_end_of_track in non-playing state.") } } From 2eb4aa61d3a14a4f0558991bc96de7cac40302ad Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Tue, 20 Feb 2018 22:45:14 +0100 Subject: [PATCH 08/13] Use single program on events, and pass events in envars. --- src/main.rs | 8 ++--- src/player_event_handler.rs | 60 ++++++++++++++++--------------------- 2 files changed, 28 insertions(+), 40 deletions(-) diff --git a/src/main.rs b/src/main.rs index 8251c3f..61208b0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -95,9 +95,7 @@ fn setup(args: &[String]) -> Setup { .reqopt("n", "name", "Device name", "NAME") .optopt("", "device-type", "Displayed device type", "DEVICE_TYPE") .optopt("b", "bitrate", "Bitrate (96, 160 or 320). Defaults to 160", "BITRATE") - .optopt("", "onstart", "Run PROGRAM when playback is about to begin.", "PROGRAM") - .optopt("", "onstop", "Run PROGRAM when playback has ended.", "PROGRAM") - .optopt("", "onchange", "Run PROGRAM between two tracks.", "PROGRAM") + .optopt("", "onevent", "Run PROGRAM when playback is about to begin.", "PROGRAM") .optflag("v", "verbose", "Enable verbose output") .optopt("u", "username", "Username to sign in with", "USERNAME") .optopt("p", "password", "Password", "PASSWORD") @@ -189,9 +187,7 @@ fn setup(args: &[String]) -> Setup { PlayerConfig { bitrate: bitrate, - event_sender: run_program_on_events(matches.opt_str("onstart"), - matches.opt_str("onstop"), - matches.opt_str("onchange")) + event_sender: matches.opt_str("onevent").map(run_program_on_events) } }; diff --git a/src/player_event_handler.rs b/src/player_event_handler.rs index 8d87276..a712513 100644 --- a/src/player_event_handler.rs +++ b/src/player_event_handler.rs @@ -1,50 +1,42 @@ use std::process::Command; use std::sync::mpsc::{channel, Sender}; use std::thread; +use std::collections::HashMap; use librespot::playback::player::PlayerEvent; -fn run_program(program: &str, args: Vec) { - info!("Running {}", program); +fn run_program(program: &str, env_vars: HashMap<&str, String>) { let mut v: Vec<&str> = program.split_whitespace().collect(); + info!("Running {:?}", v); let status = Command::new(&v.remove(0)) .args(&v) - .args(args) + .envs(env_vars.iter()) .status() .expect("program failed to start"); info!("Exit status: {}", status); } -pub fn run_program_on_events(onstart: Option, - onstop: Option, - onchange: Option) -> Option> { - if onstart.is_none() && onstop.is_none() && onchange.is_none() { - None - } else { - let (sender, receiver) = channel(); - thread::spawn(move || { - while let Ok(msg) = receiver.recv() { - match msg { - PlayerEvent::Changed { old_track_id, new_track_id } => { - let args = vec![old_track_id.to_base16(), new_track_id.to_base16()]; - if let Some(ref onchange) = onchange.as_ref() { - run_program(onchange, args); - } - }, - PlayerEvent::Started { track_id } => { - let args = vec![track_id.to_base16()]; - if let Some(ref onstart) = onstart.as_ref() { - run_program(onstart, args); - } - } - PlayerEvent::Stopped { track_id } => { - let args = vec![track_id.to_base16()]; - if let Some(ref onstop) = onstop.as_ref() { - run_program(onstop, args); - } - } +pub fn run_program_on_events(onevent: String) -> Sender { + let (sender, receiver) = channel(); + thread::spawn(move || { + while let Ok(msg) = receiver.recv() { + let mut env_vars = HashMap::new(); + match msg { + PlayerEvent::Changed { old_track_id, new_track_id } => { + env_vars.insert("PLAYER_EVENT", "change".to_string()); + env_vars.insert("OLD_TRACK_ID", old_track_id.to_base16()); + env_vars.insert("TRACK_ID", new_track_id.to_base16()); + }, + PlayerEvent::Started { track_id } => { + env_vars.insert("PLAYER_EVENT", "start".to_string()); + env_vars.insert("TRACK_ID", track_id.to_base16()); + } + PlayerEvent::Stopped { track_id } => { + env_vars.insert("PLAYER_EVENT", "stop".to_string()); + env_vars.insert("TRACK_ID", track_id.to_base16()); } } - }); - Some(sender) - } + run_program(&onevent, env_vars); + } + }); + sender } From 93af49aadf4164a0e8637b54dc1052dc77b67035 Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Tue, 20 Feb 2018 23:09:48 +0100 Subject: [PATCH 09/13] Send player event messages over futures aware channel. --- playback/src/config.rs | 5 ----- playback/src/player.rs | 22 +++++++++---------- src/main.rs | 27 ++++++++++++++++++------ src/player_event_handler.rs | 42 +++++++++++++++---------------------- 4 files changed, 49 insertions(+), 47 deletions(-) diff --git a/playback/src/config.rs b/playback/src/config.rs index 6861142..e1619e6 100644 --- a/playback/src/config.rs +++ b/playback/src/config.rs @@ -1,7 +1,4 @@ use std::str::FromStr; -use core::spotify_id::SpotifyId; -use std::sync::mpsc::Sender; -use player::PlayerEvent; #[derive(Clone, Copy, Debug, Hash, PartialOrd, Ord, PartialEq, Eq)] pub enum Bitrate { @@ -31,14 +28,12 @@ impl Default for Bitrate { #[derive(Clone, Debug)] pub struct PlayerConfig { pub bitrate: Bitrate, - pub event_sender : Option>, } impl Default for PlayerConfig { fn default() -> PlayerConfig { PlayerConfig { bitrate: Bitrate::default(), - event_sender: None, } } } diff --git a/playback/src/player.rs b/playback/src/player.rs index 778368b..1b7e869 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -1,5 +1,6 @@ use futures::sync::oneshot; use futures::{future, Future}; +use futures::sync::mpsc::{UnboundedReceiver, UnboundedSender, unbounded}; use std; use std::borrow::Cow; use std::io::{Read, Seek, SeekFrom, Result}; @@ -32,6 +33,7 @@ struct PlayerInternal { sink: Box, sink_running: bool, audio_filter: Option>, + event_sender: UnboundedSender, } enum PlayerCommand { @@ -58,14 +60,15 @@ pub enum PlayerEvent { } } - +type PlayerEventChannel = UnboundedReceiver; impl Player { pub fn new(config: PlayerConfig, session: Session, audio_filter: Option>, - sink_builder: F) -> Player + sink_builder: F) -> (Player, PlayerEventChannel) where F: FnOnce() -> Box + Send + 'static { let (cmd_tx, cmd_rx) = std::sync::mpsc::channel(); + let (event_sender, event_receiver) = unbounded(); let handle = thread::spawn(move || { debug!("new Player[{}]", session.session_id()); @@ -79,15 +82,14 @@ impl Player { sink: sink_builder(), sink_running: false, audio_filter: audio_filter, + event_sender: event_sender, }; internal.run(); }); - Player { - commands: Some(cmd_tx), - thread_handle: Some(handle), - } + (Player { commands: Some(cmd_tx), thread_handle: Some(handle) }, + event_receiver) } fn command(&self, cmd: PlayerCommand) { @@ -414,11 +416,9 @@ impl PlayerInternal { } fn send_event(&mut self, event: PlayerEvent) { - if let Some(ref s) = self.config.event_sender { - match s.send(event.clone()) { - Ok(_) => info!("Sent event {:?} to event listener.", event), - Err(err) => error!("Failed to send event {:?} to listener: {:?}", event, err) - } + match self.event_sender.unbounded_send(event.clone()) { + Ok(_) => info!("Sent event {:?} to event listener.", event), + Err(err) => error!("Failed to send event {:?} to listener: {:?}", event, err) } } diff --git a/src/main.rs b/src/main.rs index 61208b0..cd78381 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9,6 +9,7 @@ extern crate tokio_signal; use env_logger::LogBuilder; use futures::{Future, Async, Poll, Stream}; +use futures::sync::mpsc::UnboundedReceiver; use std::env; use std::io::{self, stderr, Write}; use std::path::PathBuf; @@ -28,7 +29,7 @@ use librespot::playback::audio_backend::{self, Sink, BACKENDS}; use librespot::playback::config::{Bitrate, PlayerConfig}; use librespot::connect::discovery::{discovery, DiscoveryStream}; use librespot::playback::mixer::{self, Mixer}; -use librespot::playback::player::Player; +use librespot::playback::player::{Player, PlayerEvent}; use librespot::connect::spirc::{Spirc, SpircTask}; mod player_event_handler; @@ -86,6 +87,7 @@ struct Setup { credentials: Option, enable_discovery: bool, zeroconf_port: u16, + player_event_program: Option, } fn setup(args: &[String]) -> Setup { @@ -185,10 +187,7 @@ fn setup(args: &[String]) -> Setup { .map(|bitrate| Bitrate::from_str(bitrate).expect("Invalid bitrate")) .unwrap_or(Bitrate::default()); - PlayerConfig { - bitrate: bitrate, - event_sender: matches.opt_str("onevent").map(run_program_on_events) - } + PlayerConfig { bitrate: bitrate } }; let connect_config = { @@ -216,6 +215,7 @@ fn setup(args: &[String]) -> Setup { enable_discovery: enable_discovery, zeroconf_port: zeroconf_port, mixer: mixer, + player_event_program: matches.opt_str("onevent"), } } @@ -237,6 +237,9 @@ struct Main { connect: Box>, shutdown: bool, + + player_event_channel: Option>, + player_event_program: Option, } impl Main { @@ -257,6 +260,9 @@ impl Main { spirc_task: None, shutdown: false, signal: Box::new(tokio_signal::ctrl_c(&handle).flatten_stream()), + + player_event_channel: None, + player_event_program: setup.player_event_program, }; if setup.enable_discovery { @@ -314,13 +320,14 @@ impl Future for Main { let audio_filter = mixer.get_audio_filter(); let backend = self.backend; - let player = Player::new(player_config, session.clone(), audio_filter, move || { + let (player, event_channel) = Player::new(player_config, session.clone(), audio_filter, move || { (backend)(device) }); let (spirc, spirc_task) = Spirc::new(connect_config, session, player, mixer); self.spirc = Some(spirc); self.spirc_task = Some(spirc_task); + self.player_event_channel = Some(event_channel); progress = true; } @@ -348,6 +355,14 @@ impl Future for Main { } } + if let Some(ref mut player_event_channel) = self.player_event_channel { + if let Async::Ready(Some(event)) = player_event_channel.poll().unwrap() { + if let Some(ref program) = self.player_event_program { + run_program_on_events(event, program); + } + } + } + if !progress { return Ok(Async::NotReady); } diff --git a/src/player_event_handler.rs b/src/player_event_handler.rs index a712513..79b043e 100644 --- a/src/player_event_handler.rs +++ b/src/player_event_handler.rs @@ -1,6 +1,4 @@ use std::process::Command; -use std::sync::mpsc::{channel, Sender}; -use std::thread; use std::collections::HashMap; use librespot::playback::player::PlayerEvent; @@ -15,28 +13,22 @@ fn run_program(program: &str, env_vars: HashMap<&str, String>) { info!("Exit status: {}", status); } -pub fn run_program_on_events(onevent: String) -> Sender { - let (sender, receiver) = channel(); - thread::spawn(move || { - while let Ok(msg) = receiver.recv() { - let mut env_vars = HashMap::new(); - match msg { - PlayerEvent::Changed { old_track_id, new_track_id } => { - env_vars.insert("PLAYER_EVENT", "change".to_string()); - env_vars.insert("OLD_TRACK_ID", old_track_id.to_base16()); - env_vars.insert("TRACK_ID", new_track_id.to_base16()); - }, - PlayerEvent::Started { track_id } => { - env_vars.insert("PLAYER_EVENT", "start".to_string()); - env_vars.insert("TRACK_ID", track_id.to_base16()); - } - PlayerEvent::Stopped { track_id } => { - env_vars.insert("PLAYER_EVENT", "stop".to_string()); - env_vars.insert("TRACK_ID", track_id.to_base16()); - } - } - run_program(&onevent, env_vars); +pub fn run_program_on_events(event: PlayerEvent, onevent: &str) { + let mut env_vars = HashMap::new(); + match event { + PlayerEvent::Changed { old_track_id, new_track_id } => { + env_vars.insert("PLAYER_EVENT", "change".to_string()); + env_vars.insert("OLD_TRACK_ID", old_track_id.to_base16()); + env_vars.insert("TRACK_ID", new_track_id.to_base16()); + }, + PlayerEvent::Started { track_id } => { + env_vars.insert("PLAYER_EVENT", "start".to_string()); + env_vars.insert("TRACK_ID", track_id.to_base16()); } - }); - sender + PlayerEvent::Stopped { track_id } => { + env_vars.insert("PLAYER_EVENT", "stop".to_string()); + env_vars.insert("TRACK_ID", track_id.to_base16()); + } + } + run_program(onevent, env_vars); } From 23d3c1593f078372c7e37fb7d2f70a2607973a34 Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Tue, 20 Feb 2018 23:12:52 +0100 Subject: [PATCH 10/13] Just spawn event handlers, don't wait for exit code. --- src/player_event_handler.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/player_event_handler.rs b/src/player_event_handler.rs index 79b043e..7882718 100644 --- a/src/player_event_handler.rs +++ b/src/player_event_handler.rs @@ -4,13 +4,12 @@ use librespot::playback::player::PlayerEvent; fn run_program(program: &str, env_vars: HashMap<&str, String>) { let mut v: Vec<&str> = program.split_whitespace().collect(); - info!("Running {:?}", v); - let status = Command::new(&v.remove(0)) + info!("Running {:?} with environment variables {:?}", v, env_vars); + Command::new(&v.remove(0)) .args(&v) .envs(env_vars.iter()) - .status() + .spawn() .expect("program failed to start"); - info!("Exit status: {}", status); } pub fn run_program_on_events(event: PlayerEvent, onevent: &str) { From 9ff6fe900c3ac5783efad267ba78925ae0d9cc99 Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Tue, 20 Feb 2018 23:31:33 +0100 Subject: [PATCH 11/13] Don't log messages when sending player events over channel. --- playback/src/player.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/playback/src/player.rs b/playback/src/player.rs index 1b7e869..b875bdc 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -416,10 +416,7 @@ impl PlayerInternal { } fn send_event(&mut self, event: PlayerEvent) { - match self.event_sender.unbounded_send(event.clone()) { - Ok(_) => info!("Sent event {:?} to event listener.", event), - Err(err) => error!("Failed to send event {:?} to listener: {:?}", event, err) - } + let _ = self.event_sender.unbounded_send(event.clone()); } fn find_available_alternative<'a>(&self, track: &'a Track) -> Option> { From 151845904830087259363e9a39bd476c9e34ef64 Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Fri, 23 Feb 2018 20:16:03 +0100 Subject: [PATCH 12/13] Minor fixes after review. --- playback/src/player.rs | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/playback/src/player.rs b/playback/src/player.rs index b875bdc..afd79a5 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -1,6 +1,6 @@ use futures::sync::oneshot; use futures::{future, Future}; -use futures::sync::mpsc::{UnboundedReceiver, UnboundedSender, unbounded}; +use futures; use std; use std::borrow::Cow; use std::io::{Read, Seek, SeekFrom, Result}; @@ -33,7 +33,7 @@ struct PlayerInternal { sink: Box, sink_running: bool, audio_filter: Option>, - event_sender: UnboundedSender, + event_sender: futures::sync::mpsc::UnboundedSender, } enum PlayerCommand { @@ -60,7 +60,7 @@ pub enum PlayerEvent { } } -type PlayerEventChannel = UnboundedReceiver; +type PlayerEventChannel = futures::sync::mpsc::UnboundedReceiver; impl Player { pub fn new(config: PlayerConfig, session: Session, audio_filter: Option>, @@ -68,7 +68,7 @@ impl Player { where F: FnOnce() -> Box + Send + 'static { let (cmd_tx, cmd_rx) = std::sync::mpsc::channel(); - let (event_sender, event_receiver) = unbounded(); + let (event_sender, event_receiver) = futures::sync::mpsc::unbounded(); let handle = thread::spawn(move || { debug!("new Player[{}]", session.session_id()); @@ -172,24 +172,12 @@ impl PlayerState { } } - fn send_end_of_track(self) { - use self::PlayerState::*; - match self { - Paused { end_of_track, .. } | - Playing { end_of_track, .. } => { - let _ = end_of_track.send(()); - }, - _ => () - } - } - fn playing_to_end_of_track(&mut self) { use self::PlayerState::*; - match *self { - Paused { track_id, .. } | - Playing { track_id, .. } => { + match mem::replace(self, Invalid) { + Playing { track_id, end_of_track, ..} => { + end_of_track.send(()); let old_state = mem::replace(self, EndOfTrack { track_id }); - old_state.send_end_of_track(); }, _ => panic!("Called playing_to_end_of_track in non-playing state.") } From 8c3f587f307faa34cac34f22842b78378aab0ba1 Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Sat, 24 Feb 2018 10:50:48 +0100 Subject: [PATCH 13/13] Assignment instead of mem::repalce() --- playback/src/player.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/playback/src/player.rs b/playback/src/player.rs index afd79a5..7fdf416 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -177,7 +177,7 @@ impl PlayerState { match mem::replace(self, Invalid) { Playing { track_id, end_of_track, ..} => { end_of_track.send(()); - let old_state = mem::replace(self, EndOfTrack { track_id }); + *self = EndOfTrack { track_id }; }, _ => panic!("Called playing_to_end_of_track in non-playing state.") }