Skip to content

Commit

Permalink
connect: adjust context resolving
Browse files Browse the repository at this point in the history
- improved certain logging parts
- preload autoplay when autoplay attribute mutates
- fix transfer context uri
- fix typo
- handle empty strings for resolve uri
- fix unexpected stop of playback
  • Loading branch information
photovoltex committed Nov 30, 2024
1 parent 37444bc commit b8d3e9d
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 45 deletions.
26 changes: 16 additions & 10 deletions connect/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub(super) enum SpircPlayStatus {
#[derive(Debug, Clone)]
pub(super) struct ResolveContext {
context: Context,
fallback: Option<String>,
resolve_uri: Option<String>,
autoplay: bool,
/// if `true` updates the entire context, otherwise only fills the context from the next
/// retrieve page, it is usually used when loading the next page of an already established context
Expand All @@ -74,24 +74,29 @@ pub(super) struct ResolveContext {
impl ResolveContext {
pub fn from_uri(
uri: impl Into<String>,
fallback_uri: impl Into<String>,
resolve_uri: impl Into<String>,
autoplay: bool,
) -> Self {
let fallback_uri = resolve_uri.into();
Self {
context: Context {
uri: uri.into(),
..Default::default()
},
fallback: Some(fallback_uri.into()),
resolve_uri: (!fallback_uri.is_empty()).then_some(fallback_uri),
autoplay,
update: true,
}
}

pub fn from_context(context: Context, autoplay: bool) -> Self {
let resolve_uri = ConnectState::get_context_uri_from_context(&context)
.and_then(|s| (!s.is_empty()).then_some(s))
.cloned();

Self {
context,
fallback: None,
resolve_uri,
autoplay,
update: true,
}
Expand Down Expand Up @@ -119,15 +124,15 @@ impl ResolveContext {
uri,
..Default::default()
},
fallback: None,
resolve_uri: None,
update: false,
autoplay: false,
}
}

/// the uri which should be used to resolve the context, might not be the context uri
pub fn resolve_uri(&self) -> Option<&String> {
ConnectState::get_context_uri_from_context(&self.context).or(self.fallback.as_ref())
self.resolve_uri.as_ref()
}

/// the actual context uri
Expand All @@ -148,9 +153,9 @@ impl Display for ResolveContext {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(
f,
"context_uri: {}, resolve_uri: {:?}, autoplay: {}, update: {}",
self.context.uri,
"resolve_uri: <{:?}>, context_uri: <{}>, autoplay: <{}>, update: <{}>",
self.resolve_uri(),
self.context.uri,
self.autoplay,
self.update
)
Expand All @@ -159,11 +164,12 @@ impl Display for ResolveContext {

impl PartialEq for ResolveContext {
fn eq(&self, other: &Self) -> bool {
let eq_context = self.context.uri == other.context.uri;
let eq_context = self.context_uri() == other.context_uri();
let eq_resolve = self.resolve_uri() == other.resolve_uri();
let eq_autoplay = self.autoplay == other.autoplay;
let eq_update = self.update == other.update;

eq_autoplay && eq_context && eq_update
eq_context && eq_resolve && eq_autoplay && eq_update
}
}

Expand Down
76 changes: 43 additions & 33 deletions connect/src/spirc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,10 +510,15 @@ impl SpircTask {
} else {
last_resolve = Some(resolve.clone());

let resolve_uri = resolve
.resolve_uri()
.ok_or(SpircError::InvalidUri(resolve.context_uri().to_string()))?;
let resolve_uri = match resolve.resolve_uri() {
Some(resolve) => resolve,
None => {
warn!("tried to resolve context without resolve_uri: {resolve}");
return Ok(());
}
};

debug!("resolving: {resolve}");
// the autoplay endpoint can return a 404, when it tries to retrieve an
// autoplay context for an empty playlist as it seems
if let Err(why) = self
Expand Down Expand Up @@ -564,21 +569,28 @@ impl SpircTask {
update: bool,
) -> Result<(), Error> {
if !autoplay {
match self.session.spclient().get_context(resolve_uri).await {
Err(why) => error!("failed to resolve context '{context_uri}': {why}"),
Ok(mut ctx) if update => {
ctx.uri = context_uri.to_string();
self.connect_state.update_context(ctx, UpdateContext::Default)?
}
Ok(mut ctx) if matches!(ctx.pages.first(), Some(p) if !p.tracks.is_empty()) => {
debug!("update context from single page, context {} had {} pages", ctx.uri, ctx.pages.len());
self.connect_state.fill_context_from_page(ctx.pages.remove(0))?;
}
Ok(ctx) => error!("resolving context should only update the tracks, but had no page, or track. {ctx:#?}"),
let mut ctx = self.session.spclient().get_context(resolve_uri).await?;

if update {
ctx.uri = context_uri.to_string();
self.connect_state
.update_context(ctx, UpdateContext::Default)?
} else if matches!(ctx.pages.first(), Some(p) if !p.tracks.is_empty()) {
debug!(
"update context from single page, context {} had {} pages",
ctx.uri,
ctx.pages.len()
);
self.connect_state
.fill_context_from_page(ctx.pages.remove(0))?;
} else {
error!("resolving context should only update the tracks, but had no page, or track. {ctx:#?}");
};

if let Err(why) = self.notify().await {
error!("failed to update connect state, after updating the context: {why}")
}

return Ok(());
}

Expand All @@ -590,7 +602,7 @@ impl SpircTask {
let previous_tracks = self.connect_state.prev_autoplay_track_uris();

debug!(
"loading autoplay context {resolve_uri} with {} previous tracks",
"loading autoplay context <{resolve_uri}> with {} previous tracks",
previous_tracks.len()
);

Expand Down Expand Up @@ -925,6 +937,8 @@ impl SpircTask {
if key == "autoplay" && old_value != new_value {
self.player
.emit_auto_play_changed_event(matches!(new_value, "1"));

self.preload_autoplay_when_required()
}
} else {
trace!(
Expand Down Expand Up @@ -1090,12 +1104,7 @@ impl SpircTask {
self.connect_state
.reset_context(Some(&transfer.current_session.context.uri));

let mut ctx_uri =
ConnectState::get_context_uri_from_context(&transfer.current_session.context)
.ok_or(SpircError::InvalidUri(
transfer.current_session.context.uri.clone(),
))?
.clone();
let mut ctx_uri = transfer.current_session.context.uri.clone();

debug!("trying to find initial track");
match self.connect_state.current_track_from_transfer(&transfer) {
Expand All @@ -1113,12 +1122,12 @@ impl SpircTask {
self.connect_state.fill_up_context = ContextType::Autoplay;
}

let fallback_uri = self.connect_state.current_track(|t| &t.uri).clone();
let resolve_uri = self.connect_state.current_track(|t| &t.uri).clone();

debug!("async resolve context for {}", ctx_uri);
debug!("async resolve context for <{}>", ctx_uri);
self.resolve_context.push(ResolveContext::from_uri(
ctx_uri.clone(),
&fallback_uri,
&resolve_uri,
false,
));

Expand Down Expand Up @@ -1149,7 +1158,7 @@ impl SpircTask {
debug!("currently in autoplay context, async resolving autoplay for {ctx_uri}");

self.resolve_context
.push(ResolveContext::from_uri(ctx_uri, fallback_uri, true))
.push(ResolveContext::from_uri(ctx_uri, resolve_uri, true))
}

self.transfer_state = Some(transfer);
Expand Down Expand Up @@ -1431,15 +1440,13 @@ impl SpircTask {
}
LoadNext::Empty if self.session.autoplay() => {
let current_context = self.connect_state.context_uri();
let fallback = self.connect_state.current_track(|t| &t.uri);
let resolve = self.connect_state.current_track(|t| &t.uri);
let resolve = ResolveContext::from_uri(current_context, resolve, true);

// When in autoplay, keep topping up the playlist when it nears the end
debug!("Preloading autoplay context for <{current_context}>");
debug!("Preloading autoplay: {resolve}");
// resolve the next autoplay context
self.resolve_context.push(ResolveContext::from_uri(
current_context,
fallback,
true,
));
self.resolve_context.push(resolve);
}
LoadNext::Empty => {
debug!("next context is empty and autoplay isn't enabled, no preloading required")
Expand All @@ -1450,7 +1457,10 @@ impl SpircTask {
}

fn is_playing(&self) -> bool {
matches!(self.play_status, SpircPlayStatus::Playing { .. })
matches!(
self.play_status,
SpircPlayStatus::Playing { .. } | SpircPlayStatus::LoadingPlay { .. }
)
}

fn handle_next(&mut self, track_uri: Option<String>) -> Result<(), Error> {
Expand Down
2 changes: 1 addition & 1 deletion connect/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub enum StateError {
CouldNotResolveTrackFromTransfer,
#[error("message field {0} was not available")]
MessageFieldNone(String),
#[error("context is not available. shuffle: {0:?}")]
#[error("context is not available. type: {0:?}")]
NoContext(ContextType),
#[error("could not find track {0:?} in context of {1}")]
CanNotFindTrackInContext(Option<usize>, usize),
Expand Down
2 changes: 1 addition & 1 deletion connect/src/state/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl ConnectState {
};

debug!(
"updated context {ty:?} from {} ({} tracks) to {} ({} tracks)",
"updated context {ty:?} from <{}> ({} tracks) to <{}> ({} tracks)",
self.player.context_uri,
prev_context
.map(|c| c.tracks.len().to_string())
Expand Down

0 comments on commit b8d3e9d

Please sign in to comment.