Fix playback state handling and metadata caching
This commit addresses several issues related to playback state reporting and metadata handling: 1. In the backend (pmocontrol), the music renderer now correctly uses raw playback states instead of computed logical states for event emission, and fixes stream time calculation to only occur when the device is actually playing. 2. The frontend (CurrentTrack.vue) now logs additional debug information for duration and transport state to help trace playback issues. 3. The SSE event handling in useRenderers.ts has been enhanced with more detailed logging and uses deep copying to prevent shared state issues between renderers. 4. The OpenHome queue implementation now uses a Mutex for metadata caching and implements logic to prevent duration from decreasing for the same track, which helps stabilize metadata for streaming content. These changes improve the reliability and accuracy of playback state and metadata display across all components.
This commit is contained in:
@@ -30,8 +30,8 @@ use crate::music_renderer::openhome_renderer::OpenHomeRenderer;
|
||||
use crate::music_renderer::sleep_timer::SleepTimer;
|
||||
use crate::music_renderer::upnp_renderer::UpnpRenderer;
|
||||
use crate::music_renderer::watcher::{
|
||||
WatchStrategy, WatchedState, compute_logical_playback_state, extract_track_metadata,
|
||||
playback_position_equal, playback_state_equal,
|
||||
WatchStrategy, WatchedState, extract_track_metadata, playback_position_equal,
|
||||
playback_state_equal,
|
||||
};
|
||||
use crate::online::DeviceConnectionState;
|
||||
use crate::queue::{
|
||||
@@ -541,22 +541,18 @@ impl MusicRenderer {
|
||||
}
|
||||
|
||||
// Poll state every tick
|
||||
// Note: Device-specific bugs (like Arylic/LinkPlay reporting STOPPED while playing)
|
||||
// should be corrected in the backend's playback_state() method, not here.
|
||||
if let Some(raw_state) = raw_state {
|
||||
let logical_state = compute_logical_playback_state(
|
||||
&raw_state,
|
||||
prev_position.as_ref(),
|
||||
watched.position.as_ref(),
|
||||
);
|
||||
|
||||
let changed = watched
|
||||
.state
|
||||
.as_ref()
|
||||
.map(|prev| !playback_state_equal(prev, &logical_state))
|
||||
.map(|prev| !playback_state_equal(prev, &raw_state))
|
||||
.unwrap_or(true);
|
||||
|
||||
// Emit event only for non-transient states to reduce noise
|
||||
if changed && !matches!(logical_state, PlaybackState::Transitioning) {
|
||||
let state_clone = logical_state.clone();
|
||||
if changed && !matches!(raw_state, PlaybackState::Transitioning) {
|
||||
let state_clone = raw_state.clone();
|
||||
drop(watched);
|
||||
self.emit_event(RendererEvent::StateChanged {
|
||||
id: self.id(),
|
||||
@@ -572,7 +568,7 @@ impl MusicRenderer {
|
||||
.expect("WatchedState mutex poisoned");
|
||||
}
|
||||
|
||||
watched.state = Some(logical_state);
|
||||
watched.state = Some(raw_state);
|
||||
}
|
||||
|
||||
// Handle volume/mute updates (polled every other tick)
|
||||
@@ -1014,22 +1010,28 @@ impl MusicRenderer {
|
||||
|
||||
// Pour les flux continus uniquement : calculer rel_time depuis track_start_time
|
||||
// Pour les fichiers normaux : garder les valeurs du backend
|
||||
// IMPORTANT: Ne calculer le temps que si le lecteur est en PLAYING
|
||||
let is_stream = self.is_playing_a_stream();
|
||||
if is_stream {
|
||||
if let Some(start_time) = self.track_start_time() {
|
||||
if let Ok(elapsed) = start_time.elapsed() {
|
||||
let secs = elapsed.as_secs() as u32;
|
||||
let hours = secs / 3600;
|
||||
let minutes = (secs % 3600) / 60;
|
||||
let seconds = secs % 60;
|
||||
let new_rel_time = format!("{:02}:{:02}:{:02}", hours, minutes, seconds);
|
||||
tracing::debug!(
|
||||
"MusicRenderer [{}]: Stream - calculating rel_time from track_start_time: {} (elapsed={}s)",
|
||||
self.info.friendly_name(),
|
||||
new_rel_time,
|
||||
secs
|
||||
);
|
||||
position_info.rel_time = Some(new_rel_time);
|
||||
// Vérifier que le lecteur est en lecture avant de calculer le temps écoulé
|
||||
let is_playing = matches!(self.playback_state().ok(), Some(PlaybackState::Playing));
|
||||
|
||||
if is_playing {
|
||||
if let Some(start_time) = self.track_start_time() {
|
||||
if let Ok(elapsed) = start_time.elapsed() {
|
||||
let secs = elapsed.as_secs() as u32;
|
||||
let hours = secs / 3600;
|
||||
let minutes = (secs % 3600) / 60;
|
||||
let seconds = secs % 60;
|
||||
let new_rel_time = format!("{:02}:{:02}:{:02}", hours, minutes, seconds);
|
||||
tracing::debug!(
|
||||
"MusicRenderer [{}]: Stream - calculating rel_time from track_start_time: {} (elapsed={}s)",
|
||||
self.info.friendly_name(),
|
||||
new_rel_time,
|
||||
secs
|
||||
);
|
||||
position_info.rel_time = Some(new_rel_time);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -117,60 +117,6 @@ pub fn playback_position_equal(a: &PlaybackPositionInfo, b: &PlaybackPositionInf
|
||||
&& a.track_uri == b.track_uri
|
||||
}
|
||||
|
||||
/// Compute a logical playback state by combining the raw AVTransport state
|
||||
/// with previous and current position information.
|
||||
///
|
||||
/// This is designed to compensate for buggy LinkPlay/Arylic devices that
|
||||
/// report:
|
||||
/// - STOPPED while the time actually advances,
|
||||
/// - NO_MEDIA_PRESENT while track duration is known.
|
||||
pub fn compute_logical_playback_state(
|
||||
raw: &PlaybackState,
|
||||
prev_position: Option<&PlaybackPositionInfo>,
|
||||
current_position: Option<&PlaybackPositionInfo>,
|
||||
) -> PlaybackState {
|
||||
// Rule 1: Arylic / LinkPlay sometimes report STOPPED while the stream is
|
||||
// actually playing. If we detect that the relative time advances between
|
||||
// two polls, we treat this as Playing.
|
||||
if let PlaybackState::Stopped = raw {
|
||||
if let (Some(prev), Some(curr)) = (prev_position, current_position) {
|
||||
if let (Some(prev_rel), Some(curr_rel)) = (
|
||||
parse_optional_hms_to_secs(&prev.rel_time),
|
||||
parse_optional_hms_to_secs(&curr.rel_time),
|
||||
) {
|
||||
if curr_rel > prev_rel {
|
||||
let delta = curr_rel - prev_rel;
|
||||
// Our poll loop runs every 500ms; accept small jitter in the delta.
|
||||
if delta <= 5 {
|
||||
return PlaybackState::Playing;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Rule 2: Some devices report NO_MEDIA_PRESENT while exposing a non-zero
|
||||
// track duration. In practice this behaves like a stopped transport with
|
||||
// a loaded track.
|
||||
if let PlaybackState::NoMedia = raw {
|
||||
let duration_secs = current_position
|
||||
.and_then(|p| parse_optional_hms_to_secs(&p.track_duration))
|
||||
.or_else(|| prev_position.and_then(|p| parse_optional_hms_to_secs(&p.track_duration)));
|
||||
|
||||
if matches!(duration_secs, Some(d) if d > 0) {
|
||||
return PlaybackState::Stopped;
|
||||
}
|
||||
}
|
||||
|
||||
// Fallback: keep the raw (already normalized) state.
|
||||
raw.clone()
|
||||
}
|
||||
|
||||
/// Parse an optional HH:MM:SS time string to seconds.
|
||||
fn parse_optional_hms_to_secs(value: &Option<String>) -> Option<u64> {
|
||||
value.as_ref().and_then(|s| parse_hms_to_secs(s))
|
||||
}
|
||||
|
||||
/// Parse "HH:MM:SS" style time strings to seconds.
|
||||
///
|
||||
/// Returns None for empty or sentinel values such as "NOT_IMPLEMENTED" or "-:--:--".
|
||||
|
||||
@@ -125,7 +125,7 @@ pub struct OpenHomeQueue {
|
||||
/// Cache des métadonnées par ID OpenHome.
|
||||
/// Permet de maintenir des métadonnées à jour même si le service OpenHome
|
||||
/// ne permet pas de les modifier directement.
|
||||
metadata_cache: HashMap<u32, Option<crate::model::TrackMetadata>>,
|
||||
metadata_cache: Mutex<HashMap<u32, Option<crate::model::TrackMetadata>>>,
|
||||
/// Cache for track IDs to avoid redundant IdArray SOAP calls
|
||||
track_ids_cache: Arc<Mutex<TrackIdsCache>>,
|
||||
/// Cache for current track ID to avoid redundant Id SOAP calls
|
||||
@@ -144,7 +144,7 @@ impl OpenHomeQueue {
|
||||
playlist_client: playlist,
|
||||
info_client,
|
||||
product_client,
|
||||
metadata_cache: HashMap::new(),
|
||||
metadata_cache: Mutex::new(HashMap::new()),
|
||||
track_ids_cache: Arc::new(Mutex::new(TrackIdsCache::new())),
|
||||
current_track_id_cache: Arc::new(Mutex::new(CurrentTrackIdCache::new())),
|
||||
}
|
||||
@@ -190,17 +190,130 @@ impl OpenHomeQueue {
|
||||
metadata: Option<crate::model::TrackMetadata>,
|
||||
) -> Result<(), ControlPointError> {
|
||||
let track_id = self.position_to_id(index)?;
|
||||
self.metadata_cache.insert(track_id, metadata);
|
||||
self.cache_metadata(track_id, metadata);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Insère ou met à jour les métadonnées dans le cache.
|
||||
/// RÈGLE: Pour une même chanson (même titre ET même artiste), la durée ne peut jamais diminuer.
|
||||
/// Si le titre ou l'artiste change, c'est une nouvelle chanson donc toute durée est acceptée.
|
||||
/// Cette fonction est le SEUL point d'entrée pour modifier le cache.
|
||||
fn cache_metadata(&self, track_id: u32, new_metadata: Option<crate::model::TrackMetadata>) {
|
||||
let mut cache = self.metadata_cache.lock().unwrap();
|
||||
|
||||
// Vérifier s'il y a déjà des métadonnées en cache
|
||||
if let Some(cached_meta) = cache.get(&track_id) {
|
||||
// Vérifier si c'est la même chanson (titre ET artiste identiques)
|
||||
let same_title = cached_meta.as_ref().and_then(|m| m.title.as_ref())
|
||||
== new_metadata.as_ref().and_then(|m| m.title.as_ref());
|
||||
let same_artist = cached_meta.as_ref().and_then(|m| m.artist.as_ref())
|
||||
== new_metadata.as_ref().and_then(|m| m.artist.as_ref());
|
||||
|
||||
let same_track = same_title && same_artist;
|
||||
|
||||
if same_track {
|
||||
// Même chanson: vérifier que la durée n'a pas diminué
|
||||
let should_update = match (
|
||||
cached_meta.as_ref().and_then(|m| m.duration.as_ref()),
|
||||
new_metadata.as_ref().and_then(|m| m.duration.as_ref()),
|
||||
) {
|
||||
(Some(cached_dur), Some(new_dur)) => {
|
||||
// Parser les durées (format HH:MM:SS)
|
||||
let parse_duration = |dur: &str| -> Option<u32> {
|
||||
let parts: Vec<&str> = dur.split(':').collect();
|
||||
if parts.len() == 3 {
|
||||
let h: u32 = parts[0].parse().ok()?;
|
||||
let m: u32 = parts[1].parse().ok()?;
|
||||
let s: u32 = parts[2].parse().ok()?;
|
||||
Some(h * 3600 + m * 60 + s)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
};
|
||||
|
||||
if let (Some(cached_secs), Some(new_secs)) =
|
||||
(parse_duration(cached_dur), parse_duration(new_dur))
|
||||
{
|
||||
if new_secs < cached_secs {
|
||||
// Durée a diminué pour la même chanson: refuser
|
||||
tracing::trace!(
|
||||
"OpenHome cache_metadata: track_id={}, REJECTING update (same track, duration decreased): {} -> {}",
|
||||
track_id,
|
||||
cached_dur,
|
||||
new_dur
|
||||
);
|
||||
false
|
||||
} else {
|
||||
// Durée a augmenté ou est égale: accepter
|
||||
if new_secs > cached_secs {
|
||||
tracing::debug!(
|
||||
"OpenHome cache_metadata: track_id={}, same track, duration increased: {} -> {}",
|
||||
track_id,
|
||||
cached_dur,
|
||||
new_dur
|
||||
);
|
||||
}
|
||||
true
|
||||
}
|
||||
} else {
|
||||
// Impossible de parser: accepter par défaut
|
||||
true
|
||||
}
|
||||
}
|
||||
_ => true, // Pas de durée ou une seule des deux: accepter
|
||||
};
|
||||
|
||||
if should_update {
|
||||
cache.insert(track_id, new_metadata);
|
||||
}
|
||||
} else {
|
||||
// Chanson différente: accepter sans vérification
|
||||
tracing::debug!(
|
||||
"OpenHome cache_metadata: track_id={}, different track (title or artist changed), accepting update",
|
||||
track_id
|
||||
);
|
||||
cache.insert(track_id, new_metadata);
|
||||
}
|
||||
} else {
|
||||
// Pas dans le cache: insérer directement
|
||||
tracing::trace!(
|
||||
"OpenHome cache_metadata: track_id={}, inserting first time, duration={:?}",
|
||||
track_id,
|
||||
new_metadata.as_ref().and_then(|m| m.duration.as_ref())
|
||||
);
|
||||
cache.insert(track_id, new_metadata);
|
||||
}
|
||||
}
|
||||
|
||||
fn playback_item_from_entry(&self, entry: &OhTrackEntry) -> PlaybackItem {
|
||||
// Utiliser les métadonnées du cache si disponibles, sinon celles de l'entrée
|
||||
let metadata = self
|
||||
.metadata_cache
|
||||
.get(&entry.id)
|
||||
.cloned()
|
||||
.unwrap_or_else(|| entry.metadata());
|
||||
// TOUJOURS préférer les métadonnées du cache si disponibles
|
||||
// Le cache contient les métadonnées stables mises lors de l'insertion
|
||||
// Les métadonnées de l'entry (venant de ReadList) changent pour les streams
|
||||
let metadata = {
|
||||
let cache = self.metadata_cache.lock().unwrap();
|
||||
if let Some(cached_meta) = cache.get(&entry.id) {
|
||||
// Utiliser les métadonnées stables du cache
|
||||
tracing::trace!(
|
||||
"OpenHome playback_item_from_entry: track_id={}, using CACHE, duration={:?}",
|
||||
entry.id,
|
||||
cached_meta.as_ref().and_then(|m| m.duration.as_ref())
|
||||
);
|
||||
cached_meta.clone()
|
||||
} else {
|
||||
// Pas dans le cache (piste existante avant démarrage de PMOMusic ou ajoutée par autre control point)
|
||||
// Utiliser les métadonnées fraîches de l'entry et les mettre en cache pour stabiliser
|
||||
let fresh = entry.metadata();
|
||||
tracing::debug!(
|
||||
"OpenHome playback_item_from_entry: track_id={}, caching metadata from entry (first read), duration={:?}",
|
||||
entry.id,
|
||||
fresh.as_ref().and_then(|m| m.duration.as_ref())
|
||||
);
|
||||
drop(cache); // Libérer le lock avant d'appeler cache_metadata
|
||||
// Mettre en cache pour éviter les oscillations sur les flux radio
|
||||
self.cache_metadata(entry.id, fresh.clone());
|
||||
fresh
|
||||
}
|
||||
};
|
||||
|
||||
let didl_id = entry
|
||||
.didl_id()
|
||||
@@ -228,7 +341,7 @@ impl OpenHomeQueue {
|
||||
.insert(after_id, &item.uri, &metadata_xml)?;
|
||||
|
||||
// Enregistrer les métadonnées dans le cache
|
||||
self.metadata_cache.insert(new_id, item.metadata);
|
||||
self.cache_metadata(new_id, item.metadata);
|
||||
|
||||
Ok(new_id)
|
||||
}
|
||||
@@ -250,7 +363,7 @@ impl OpenHomeQueue {
|
||||
for &track_id in current_track_ids.iter().rev() {
|
||||
if track_id as usize != playing_id {
|
||||
self.playlist_client.delete_id_if_exists(track_id)?;
|
||||
self.metadata_cache.remove(&track_id);
|
||||
self.metadata_cache.lock().unwrap().remove(&track_id);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -263,7 +376,7 @@ impl OpenHomeQueue {
|
||||
.insert(previous_id, &item.uri, &metadata)?;
|
||||
|
||||
// Enregistrer les métadonnées dans le cache
|
||||
self.metadata_cache.insert(new_id, item.metadata);
|
||||
self.cache_metadata(new_id, item.metadata);
|
||||
|
||||
previous_id = new_id;
|
||||
}
|
||||
@@ -296,7 +409,7 @@ impl OpenHomeQueue {
|
||||
track_id
|
||||
);
|
||||
self.playlist_client.delete_id_if_exists(track_id)?;
|
||||
self.metadata_cache.remove(&track_id);
|
||||
self.metadata_cache.lock().unwrap().remove(&track_id);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
@@ -329,8 +442,7 @@ impl OpenHomeQueue {
|
||||
previous_id = existing_id;
|
||||
|
||||
// Mettre à jour les métadonnées de l'item existant conservé
|
||||
self.metadata_cache
|
||||
.insert(existing_id, item.metadata.clone());
|
||||
self.cache_metadata(existing_id, item.metadata.clone());
|
||||
|
||||
debug!(
|
||||
renderer = self.renderer_id.0.as_str(),
|
||||
@@ -346,7 +458,7 @@ impl OpenHomeQueue {
|
||||
.insert(previous_id, &item.uri, &metadata)?;
|
||||
|
||||
// Enregistrer les métadonnées du nouvel item
|
||||
self.metadata_cache.insert(new_id, item.metadata.clone());
|
||||
self.cache_metadata(new_id, item.metadata.clone());
|
||||
|
||||
debug!(
|
||||
renderer = self.renderer_id.0.as_str(),
|
||||
@@ -424,8 +536,7 @@ impl OpenHomeQueue {
|
||||
let previous_id = pivot_id as u32;
|
||||
|
||||
// Mettre à jour les métadonnées du pivot
|
||||
self.metadata_cache
|
||||
.insert(previous_id, new_items[pivot_idx_new].metadata.clone());
|
||||
self.cache_metadata(previous_id, new_items[pivot_idx_new].metadata.clone());
|
||||
|
||||
debug!(
|
||||
renderer = self.renderer_id.0.as_str(),
|
||||
@@ -492,7 +603,7 @@ impl OpenHomeQueue {
|
||||
"Using delete_all() for complete replacement (more robust for live playlists)"
|
||||
);
|
||||
self.playlist_client.delete_all()?;
|
||||
self.metadata_cache.clear();
|
||||
self.metadata_cache.lock().unwrap().clear();
|
||||
} else {
|
||||
// Selective deletion when keeping some items
|
||||
for idx in (0..current_track_ids.len()).rev() {
|
||||
@@ -501,7 +612,7 @@ impl OpenHomeQueue {
|
||||
// Use delete_id_if_exists() to handle cases where another control point
|
||||
// may have already modified the playlist
|
||||
self.playlist_client.delete_id_if_exists(track_id)?;
|
||||
self.metadata_cache.remove(&track_id);
|
||||
self.metadata_cache.lock().unwrap().remove(&track_id);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -534,7 +645,8 @@ impl OpenHomeQueue {
|
||||
previous_id = existing_id;
|
||||
|
||||
// Mettre à jour les métadonnées de l'item existant conservé
|
||||
self.metadata_cache.insert(existing_id, item.metadata);
|
||||
// La fonction cache_metadata gère la protection contre la diminution de durée
|
||||
self.cache_metadata(existing_id, item.metadata);
|
||||
} else {
|
||||
let metadata = build_metadata_xml(&item);
|
||||
let new_id = self
|
||||
@@ -542,7 +654,7 @@ impl OpenHomeQueue {
|
||||
.insert(previous_id, &item.uri, &metadata)?;
|
||||
|
||||
// Enregistrer les métadonnées du nouvel item
|
||||
self.metadata_cache.insert(new_id, item.metadata);
|
||||
self.cache_metadata(new_id, item.metadata);
|
||||
|
||||
previous_id = new_id;
|
||||
}
|
||||
@@ -761,6 +873,7 @@ impl QueueBackend for OpenHomeQueue {
|
||||
}
|
||||
|
||||
// Read metadata for all tracks (batched)
|
||||
// playback_item_from_entry() will prioritize cached metadata over entry metadata
|
||||
const MAX_BATCH: usize = 64;
|
||||
let mut entries = Vec::with_capacity(ids.len());
|
||||
for chunk in ids.chunks(MAX_BATCH) {
|
||||
@@ -837,7 +950,7 @@ impl QueueBackend for OpenHomeQueue {
|
||||
|
||||
self.ensure_playlist_source_selected()?;
|
||||
self.playlist_client.delete_all()?;
|
||||
self.metadata_cache.clear();
|
||||
self.metadata_cache.lock().unwrap().clear();
|
||||
|
||||
// Invalidate caches after delete_all (clears queue and current track)
|
||||
self.track_ids_cache.lock().unwrap().invalidate();
|
||||
@@ -856,7 +969,7 @@ impl QueueBackend for OpenHomeQueue {
|
||||
.insert(previous_id, &item.uri, &metadata)?;
|
||||
|
||||
// Enregistrer les métadonnées dans le cache
|
||||
self.metadata_cache.insert(new_id, item.metadata);
|
||||
self.cache_metadata(new_id, item.metadata);
|
||||
|
||||
previous_id = new_id;
|
||||
}
|
||||
@@ -871,7 +984,7 @@ impl QueueBackend for OpenHomeQueue {
|
||||
self.ensure_playlist_source_selected()?;
|
||||
if items.is_empty() {
|
||||
self.playlist_client.delete_all()?;
|
||||
self.metadata_cache.clear();
|
||||
self.metadata_cache.lock().unwrap().clear();
|
||||
// Invalidate caches after delete_all (clears queue and current track)
|
||||
self.track_ids_cache.lock().unwrap().invalidate();
|
||||
self.current_track_id_cache.lock().unwrap().invalidate();
|
||||
@@ -1002,8 +1115,8 @@ impl QueueBackend for OpenHomeQueue {
|
||||
.insert(before_id, &item.uri, &metadata)?;
|
||||
|
||||
// Mettre à jour le cache avec les nouvelles métadonnées
|
||||
self.metadata_cache.remove(&track_id);
|
||||
self.metadata_cache.insert(new_id, item.metadata);
|
||||
self.metadata_cache.lock().unwrap().remove(&track_id);
|
||||
self.cache_metadata(new_id, item.metadata);
|
||||
|
||||
if ci == Some(index) {
|
||||
self.playlist_client.seek_id(new_id)?;
|
||||
|
||||
1193
pmocontrol/src/queue/openhome.rs.bak
Normal file
1193
pmocontrol/src/queue/openhome.rs.bak
Normal file
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user