Further initial loading improvements

This should fix remaining cases of a client connecting, and failing
to start playback from *beyond* the beginning when `librespot` is
still loading that track.

This undoes the `suppress_loading_status` workaround from #430,
under the assumption that the race condition reported there has
since been fixed on Spotify's end.
This commit is contained in:
Roderick van Domburg 2021-12-29 22:18:38 +01:00
parent e5938c7e24
commit e51f475a00
No known key found for this signature in database
GPG key ID: A9EF5222A26F0451
4 changed files with 42 additions and 50 deletions

View file

@ -58,7 +58,7 @@ hyper = "0.14"
log = "0.4" log = "0.4"
rpassword = "5.0" rpassword = "5.0"
thiserror = "1.0" thiserror = "1.0"
tokio = { version = "1", features = ["rt", "rt-multi-thread", "macros", "signal", "sync", "parking_lot", "process"] } tokio = { version = "1", features = ["rt", "macros", "signal", "sync", "parking_lot", "process"] }
url = "2.2" url = "2.2"
sha-1 = "0.9" sha-1 = "0.9"

View file

@ -427,7 +427,8 @@ pub(super) async fn audio_file_fetch(
} }
None => break, None => break,
} }
} },
else => (),
} }
if fetch.get_download_strategy() == DownloadStrategy::Streaming() { if fetch.get_download_strategy() == DownloadStrategy::Streaming() {

View file

@ -550,7 +550,7 @@ impl SpircTask {
SpircCommand::Play => { SpircCommand::Play => {
if active { if active {
self.handle_play(); self.handle_play();
self.notify(None, true) self.notify(None)
} else { } else {
CommandSender::new(self, MessageType::kMessageTypePlay).send() CommandSender::new(self, MessageType::kMessageTypePlay).send()
} }
@ -558,7 +558,7 @@ impl SpircTask {
SpircCommand::PlayPause => { SpircCommand::PlayPause => {
if active { if active {
self.handle_play_pause(); self.handle_play_pause();
self.notify(None, true) self.notify(None)
} else { } else {
CommandSender::new(self, MessageType::kMessageTypePlayPause).send() CommandSender::new(self, MessageType::kMessageTypePlayPause).send()
} }
@ -566,7 +566,7 @@ impl SpircTask {
SpircCommand::Pause => { SpircCommand::Pause => {
if active { if active {
self.handle_pause(); self.handle_pause();
self.notify(None, true) self.notify(None)
} else { } else {
CommandSender::new(self, MessageType::kMessageTypePause).send() CommandSender::new(self, MessageType::kMessageTypePause).send()
} }
@ -574,7 +574,7 @@ impl SpircTask {
SpircCommand::Prev => { SpircCommand::Prev => {
if active { if active {
self.handle_prev(); self.handle_prev();
self.notify(None, true) self.notify(None)
} else { } else {
CommandSender::new(self, MessageType::kMessageTypePrev).send() CommandSender::new(self, MessageType::kMessageTypePrev).send()
} }
@ -582,7 +582,7 @@ impl SpircTask {
SpircCommand::Next => { SpircCommand::Next => {
if active { if active {
self.handle_next(); self.handle_next();
self.notify(None, true) self.notify(None)
} else { } else {
CommandSender::new(self, MessageType::kMessageTypeNext).send() CommandSender::new(self, MessageType::kMessageTypeNext).send()
} }
@ -590,7 +590,7 @@ impl SpircTask {
SpircCommand::VolumeUp => { SpircCommand::VolumeUp => {
if active { if active {
self.handle_volume_up(); self.handle_volume_up();
self.notify(None, true) self.notify(None)
} else { } else {
CommandSender::new(self, MessageType::kMessageTypeVolumeUp).send() CommandSender::new(self, MessageType::kMessageTypeVolumeUp).send()
} }
@ -598,7 +598,7 @@ impl SpircTask {
SpircCommand::VolumeDown => { SpircCommand::VolumeDown => {
if active { if active {
self.handle_volume_down(); self.handle_volume_down();
self.notify(None, true) self.notify(None)
} else { } else {
CommandSender::new(self, MessageType::kMessageTypeVolumeDown).send() CommandSender::new(self, MessageType::kMessageTypeVolumeDown).send()
} }
@ -629,7 +629,7 @@ impl SpircTask {
PlayerEvent::Loading { .. } => { PlayerEvent::Loading { .. } => {
trace!("==> kPlayStatusLoading"); trace!("==> kPlayStatusLoading");
self.state.set_status(PlayStatus::kPlayStatusLoading); self.state.set_status(PlayStatus::kPlayStatusLoading);
self.notify(None, false) self.notify(None)
} }
PlayerEvent::Playing { position_ms, .. } => { PlayerEvent::Playing { position_ms, .. } => {
trace!("==> kPlayStatusPlay"); trace!("==> kPlayStatusPlay");
@ -642,7 +642,7 @@ impl SpircTask {
if (*nominal_start_time - new_nominal_start_time).abs() > 100 { if (*nominal_start_time - new_nominal_start_time).abs() > 100 {
*nominal_start_time = new_nominal_start_time; *nominal_start_time = new_nominal_start_time;
self.update_state_position(position_ms); self.update_state_position(position_ms);
self.notify(None, true) self.notify(None)
} else { } else {
Ok(()) Ok(())
} }
@ -655,7 +655,7 @@ impl SpircTask {
nominal_start_time: new_nominal_start_time, nominal_start_time: new_nominal_start_time,
preloading_of_next_track_triggered: false, preloading_of_next_track_triggered: false,
}; };
self.notify(None, true) self.notify(None)
} }
_ => Ok(()), _ => Ok(()),
} }
@ -673,7 +673,7 @@ impl SpircTask {
if *position_ms != new_position_ms { if *position_ms != new_position_ms {
*position_ms = new_position_ms; *position_ms = new_position_ms;
self.update_state_position(new_position_ms); self.update_state_position(new_position_ms);
self.notify(None, true) self.notify(None)
} else { } else {
Ok(()) Ok(())
} }
@ -686,7 +686,7 @@ impl SpircTask {
position_ms: new_position_ms, position_ms: new_position_ms,
preloading_of_next_track_triggered: false, preloading_of_next_track_triggered: false,
}; };
self.notify(None, true) self.notify(None)
} }
_ => Ok(()), _ => Ok(()),
} }
@ -699,7 +699,7 @@ impl SpircTask {
warn!("The player has stopped unexpectedly."); warn!("The player has stopped unexpectedly.");
self.state.set_status(PlayStatus::kPlayStatusStop); self.state.set_status(PlayStatus::kPlayStatusStop);
self.play_status = SpircPlayStatus::Stopped; self.play_status = SpircPlayStatus::Stopped;
self.notify(None, true) self.notify(None)
} }
} }
} }
@ -788,7 +788,7 @@ impl SpircTask {
} }
match update.get_typ() { match update.get_typ() {
MessageType::kMessageTypeHello => self.notify(Some(ident), true), MessageType::kMessageTypeHello => self.notify(Some(ident)),
MessageType::kMessageTypeLoad => { MessageType::kMessageTypeLoad => {
if !self.device.get_is_active() { if !self.device.get_is_active() {
@ -810,47 +810,47 @@ impl SpircTask {
self.play_status = SpircPlayStatus::Stopped; self.play_status = SpircPlayStatus::Stopped;
} }
self.notify(None, true) self.notify(None)
} }
MessageType::kMessageTypePlay => { MessageType::kMessageTypePlay => {
self.handle_play(); self.handle_play();
self.notify(None, true) self.notify(None)
} }
MessageType::kMessageTypePlayPause => { MessageType::kMessageTypePlayPause => {
self.handle_play_pause(); self.handle_play_pause();
self.notify(None, true) self.notify(None)
} }
MessageType::kMessageTypePause => { MessageType::kMessageTypePause => {
self.handle_pause(); self.handle_pause();
self.notify(None, true) self.notify(None)
} }
MessageType::kMessageTypeNext => { MessageType::kMessageTypeNext => {
self.handle_next(); self.handle_next();
self.notify(None, true) self.notify(None)
} }
MessageType::kMessageTypePrev => { MessageType::kMessageTypePrev => {
self.handle_prev(); self.handle_prev();
self.notify(None, true) self.notify(None)
} }
MessageType::kMessageTypeVolumeUp => { MessageType::kMessageTypeVolumeUp => {
self.handle_volume_up(); self.handle_volume_up();
self.notify(None, true) self.notify(None)
} }
MessageType::kMessageTypeVolumeDown => { MessageType::kMessageTypeVolumeDown => {
self.handle_volume_down(); self.handle_volume_down();
self.notify(None, true) self.notify(None)
} }
MessageType::kMessageTypeRepeat => { MessageType::kMessageTypeRepeat => {
self.state.set_repeat(update.get_state().get_repeat()); self.state.set_repeat(update.get_state().get_repeat());
self.notify(None, true) self.notify(None)
} }
MessageType::kMessageTypeShuffle => { MessageType::kMessageTypeShuffle => {
@ -870,17 +870,16 @@ impl SpircTask {
let context = self.state.get_context_uri(); let context = self.state.get_context_uri();
debug!("{:?}", context); debug!("{:?}", context);
} }
self.notify(None, true) self.notify(None)
} }
MessageType::kMessageTypeSeek => { MessageType::kMessageTypeSeek => {
self.handle_seek(update.get_position()); self.handle_seek(update.get_position());
self.notify(None, true) self.notify(None)
} }
MessageType::kMessageTypeReplace => { MessageType::kMessageTypeReplace => {
self.update_tracks(&update); self.update_tracks(&update);
self.notify(None, true)?;
if let SpircPlayStatus::Playing { if let SpircPlayStatus::Playing {
preloading_of_next_track_triggered, preloading_of_next_track_triggered,
@ -898,12 +897,13 @@ impl SpircTask {
} }
} }
} }
Ok(())
self.notify(None)
} }
MessageType::kMessageTypeVolume => { MessageType::kMessageTypeVolume => {
self.set_volume(update.get_volume() as u16); self.set_volume(update.get_volume() as u16);
self.notify(None, true) self.notify(None)
} }
MessageType::kMessageTypeNotify => { MessageType::kMessageTypeNotify => {
@ -1172,7 +1172,7 @@ impl SpircTask {
fn handle_end_of_track(&mut self) -> Result<(), Error> { fn handle_end_of_track(&mut self) -> Result<(), Error> {
self.handle_next(); self.handle_next();
self.notify(None, true) self.notify(None)
} }
fn position(&mut self) -> u32 { fn position(&mut self) -> u32 {
@ -1391,14 +1391,7 @@ impl SpircTask {
CommandSender::new(self, MessageType::kMessageTypeHello).send() CommandSender::new(self, MessageType::kMessageTypeHello).send()
} }
fn notify( fn notify(&mut self, recipient: Option<&str>) -> Result<(), Error> {
&mut self,
recipient: Option<&str>,
suppress_loading_status: bool,
) -> Result<(), Error> {
if suppress_loading_status && (self.state.get_status() == PlayStatus::kPlayStatusLoading) {
return Ok(());
};
let status_string = match self.state.get_status() { let status_string = match self.state.get_status() {
PlayStatus::kPlayStatusLoading => "kPlayStatusLoading", PlayStatus::kPlayStatusLoading => "kPlayStatusLoading",
PlayStatus::kPlayStatusPause => "kPlayStatusPause", PlayStatus::kPlayStatusPause => "kPlayStatusPause",

View file

@ -857,14 +857,6 @@ impl PlayerTrackLoader {
let stream_loader_controller = encrypted_file.get_stream_loader_controller().ok()?; let stream_loader_controller = encrypted_file.get_stream_loader_controller().ok()?;
if play_from_beginning {
// No need to seek -> we stream from the beginning
stream_loader_controller.set_stream_mode();
} else {
// we need to seek -> we set stream mode after the initial seek.
stream_loader_controller.set_random_access_mode();
}
let key = match self.session.audio_key().request(spotify_id, file_id).await { let key = match self.session.audio_key().request(spotify_id, file_id).await {
Ok(key) => key, Ok(key) => key,
Err(e) => { Err(e) => {
@ -875,6 +867,10 @@ impl PlayerTrackLoader {
let mut decrypted_file = AudioDecrypt::new(key, encrypted_file); let mut decrypted_file = AudioDecrypt::new(key, encrypted_file);
// Parsing normalisation data and starting playback from *beyond* the beginning
// will trigger a seek() so always start in random access mode.
stream_loader_controller.set_random_access_mode();
let normalisation_data = match NormalisationData::parse_from_file(&mut decrypted_file) { let normalisation_data = match NormalisationData::parse_from_file(&mut decrypted_file) {
Ok(data) => data, Ok(data) => data,
Err(_) => { Err(_) => {
@ -930,15 +926,17 @@ impl PlayerTrackLoader {
let mut stream_position_pcm = 0; let mut stream_position_pcm = 0;
let position_pcm = PlayerInternal::position_ms_to_pcm(position_ms); let position_pcm = PlayerInternal::position_ms_to_pcm(position_ms);
if position_pcm > 0 { if !play_from_beginning {
stream_loader_controller.set_random_access_mode();
match decoder.seek(position_pcm) { match decoder.seek(position_pcm) {
Ok(_) => stream_position_pcm = position_pcm, Ok(_) => stream_position_pcm = position_pcm,
Err(e) => error!("PlayerTrackLoader::load_track error seeking: {}", e), Err(e) => error!("PlayerTrackLoader::load_track error seeking: {}", e),
} }
stream_loader_controller.set_stream_mode();
}; };
// Transition from random access mode to streaming mode now that
// we are ready to play from the requested position.
stream_loader_controller.set_stream_mode();
info!("<{}> ({} ms) loaded", audio.name, audio.duration); info!("<{}> ({} ms) loaded", audio.name, audio.duration);
return Some(PlayerLoadedTrackData { return Some(PlayerLoadedTrackData {