eric/pmomusic
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases 3 Wiki Activity

Fix queue duplication and internal queue playback bugs

Browse Source 
Fix bug where tracks were duplicated in queue position 0 during playlist refreshes by comparing items with URI or didl_id

Fix race condition in internal queue playback where transient STOPPED states caused unwanted auto-advance

- Updated sync_queue in interne.rs to use didl_id as fallback for URI comparison
- Added items_match function in openhome.rs for robust item comparison
- Modified lcs_flags in openhome.rs to use items_match for LCS algorithm
- Added has_played_since_track_start flag in musicrenderer.rs to prevent auto-advance on transient STOPPED states
- Updated play_* methods in musicrenderer.rs to reset the has_played flag before starting playback
- Added diagnostic logs in sync_queue for tracking item matching issues
This commit is contained in:
Eric Coissac
2026-01-16 22:45:38 +01:00
parent cdd0e79be8
commit 5701fbf465
8 changed files with 429 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

87
pmocontrol/src/music_renderer/musicrenderer.rs
View File

@@ -91,6 +91,10 @@ struct MusicRendererState {
user_stop_requested: bool,
/// Sleep timer for auto-stop functionality.
sleep_timer: SleepTimer,
/// Flag indicating that a PLAYING state has been observed since the last track start.
/// This prevents auto-advance on transient STOPPED states during track initialization.
/// Auto-advance is only allowed when this flag is true.
has_played_since_track_start: bool,
}
#[derive(Clone)]
@@ -422,25 +426,39 @@ impl MusicRenderer {
"Renderer stopped by user request; not auto-advancing"
);
self.set_playback_source(PlaybackSource::None);
self.clear_has_played_flag();
} else if self.is_playing_from_queue() {
debug!(
renderer = self.info.friendly_name(),
"Renderer stopped after queue-driven playback; advancing"
);
if let Err(err) = self.play_next_from_queue() {
error!(
// Only auto-advance if we have actually seen a PLAYING state
// since the track was started. This prevents auto-advance on
// transient STOPPED states during track initialization.
if self.check_and_clear_has_played_flag() {
debug!(
renderer = self.info.friendly_name(),
error = %err,
"Auto-advance failed; clearing queue playback state"
"Renderer stopped after queue-driven playback; advancing"
);
if let Err(err) = self.play_next_from_queue() {
error!(
renderer = self.info.friendly_name(),
error = %err,
"Auto-advance failed; clearing queue playback state"
);
self.set_playback_source(PlaybackSource::None);
}
} else {
debug!(
renderer = self.info.friendly_name(),
"Renderer stopped but no PLAYING state seen yet; ignoring (likely track initialization)"
);
self.set_playback_source(PlaybackSource::None);
}
} else {
self.set_playback_source(PlaybackSource::None);
self.clear_has_played_flag();
}
}
PlaybackState::Playing => {
self.mark_external_if_idle();
// Mark that we have seen a PLAYING state - auto-advance is now allowed
self.set_has_played_flag();
}
_ => {}
}
@@ -566,6 +584,11 @@ impl MusicRenderer {
/// Play the current item from the queue.
pub fn play_current_from_queue(&self) -> Result<(), ControlPointError> {
// Reset the has_played flag before starting playback to prevent
// auto-advance on transient STOPPED states during track initialization.
// The flag will be set back to true when PLAYING state is detected.
self.clear_has_played_flag();
self.backend
.lock()
.expect("Backend mutex poisoned")
@@ -574,6 +597,11 @@ impl MusicRenderer {
/// Advance to and play the next item from the queue.
pub fn play_next_from_queue(&self) -> Result<(), ControlPointError> {
// Reset the has_played flag before starting playback to prevent
// auto-advance on transient STOPPED states during track initialization.
// The flag will be set back to true when PLAYING state is detected.
self.clear_has_played_flag();
self.backend
.lock()
.expect("Backend mutex poisoned")
@@ -584,6 +612,11 @@ impl MusicRenderer {
/// Play from a specific index in the queue.
pub fn play_from_index(&self, index: usize) -> Result<(), ControlPointError> {
// Reset the has_played flag before starting playback to prevent
// auto-advance on transient STOPPED states during track initialization.
// The flag will be set back to true when PLAYING state is detected.
self.clear_has_played_flag();
self.backend
.lock()
.expect("Backend mutex poisoned")
@@ -618,6 +651,12 @@ impl MusicRenderer {
/// Transport control: stop
pub fn stop(&self) -> Result<(), ControlPointError> {
// Reset the has_played flag when stopping playback.
// This ensures that if we start a new track, the flag will be false
// until PLAYING state is observed, preventing auto-advance on
// transient STOPPED states during track initialization.
self.clear_has_played_flag();
self.backend.lock().expect("Backend mutex poisoned").stop()
}
@@ -972,6 +1011,11 @@ impl MusicRenderer {
///
/// Uses the backend's play_from_queue which preserves the queue for all backends.
pub fn play_from_queue(&self) -> Result<(), ControlPointError> {
// Reset the has_played flag before starting playback to prevent
// auto-advance on transient STOPPED states during track initialization.
// The flag will be set back to true when PLAYING state is detected.
self.clear_has_played_flag();
let backend = self.backend.lock().expect("Backend mutex poisoned");
backend.play_from_queue()
}
@@ -1027,6 +1071,31 @@ impl MusicRenderer {
was_requested
}
// --- Has-Played Flag Management (for auto-advance protection) ---
/// Sets the has_played_since_track_start flag to true.
/// Called when PLAYING state is detected.
fn set_has_played_flag(&self) {
self.state.lock().unwrap().has_played_since_track_start = true;
}
/// Clears the has_played_since_track_start flag.
/// Called when stopping playback or starting a new track.
/// This is public so that ControlPoint can reset it when jumping to a new track.
pub fn clear_has_played_flag(&self) {
self.state.lock().unwrap().has_played_since_track_start = false;
}
/// Checks and clears the has_played_since_track_start flag.
/// Returns true if PLAYING was seen since last track start, false otherwise.
/// Used to determine if auto-advance should be allowed.
fn check_and_clear_has_played_flag(&self) -> bool {
let mut state = self.state.lock().unwrap();
let has_played = state.has_played_since_track_start;
state.has_played_since_track_start = false;
has_played
}
// --- Sleep Timer Management ---
/// Starts the sleep timer with the given duration in seconds.

39
pmocontrol/src/queue/interne.rs
View File

@@ -132,24 +132,49 @@ impl QueueBackend for InternalQueue {
}
fn sync_queue(&mut self, items: Vec<PlaybackItem>) -> Result<(), ControlPointError> {
use tracing::debug;
if items.is_empty() {
return self.replace_queue(Vec::new(), None);
}
// Récupérer l'item actuel
let current = self
.current_index
.and_then(|idx| self.items.get(idx).map(|item| (idx, item.uri.clone())));
let current = self.current_index.and_then(|idx| {
self.items
.get(idx)
.map(|item| (idx, item.uri.clone(), item.didl_id.clone()))
});
if let Some((_current_idx, current_uri)) = current {
// Chercher l'item actuel dans la nouvelle liste (par URI)
let new_idx = items.iter().position(|item| item.uri == current_uri);
if let Some((_current_idx, current_uri, current_didl_id)) = current {
// Chercher l'item actuel dans la nouvelle liste (par URI d'abord, puis par didl_id)
let new_idx = items
.iter()
.position(|item| item.uri == current_uri)
.or_else(|| {
items
.iter()
.position(|item| item.didl_id == current_didl_id)
});
if let Some(new_idx) = new_idx {
// Item trouvé dans la nouvelle liste
debug!(
renderer = self.renderer_id.0.as_str(),
current_uri = current_uri.as_str(),
new_idx,
"sync_queue: current item found in new playlist"
);
self.replace_queue(items, Some(new_idx))
} else {
// Item pas trouvé, le garder comme premier
// Item pas trouvé - cela ne devrait pas arriver si la playlist n'a pas changé
// Loguer pour diagnostic
debug!(
renderer = self.renderer_id.0.as_str(),
current_uri = current_uri.as_str(),
current_didl_id = current_didl_id.as_str(),
new_items_count = items.len(),
"sync_queue: current item NOT found in new playlist, preserving as first item"
);
let current_item = self.items[self.current_index.unwrap()].clone();
let mut new_items = Vec::with_capacity(items.len() + 1);
new_items.push(current_item);

26
pmocontrol/src/queue/openhome.rs
View File

@@ -444,6 +444,14 @@ fn build_metadata_xml(item: &PlaybackItem) -> String {
xml
}
/// Compare two PlaybackItems for equality.
/// Items are considered equal if they have the same URI OR the same didl_id.
/// This allows matching items even when the MediaServer returns different URIs
/// for the same logical track (e.g., with session tokens or different encodings).
fn items_match(a: &PlaybackItem, b: &PlaybackItem) -> bool {
a.uri == b.uri || a.didl_id == b.didl_id
}
fn lcs_flags(current: &[PlaybackItem], desired: &[PlaybackItem]) -> (Vec<bool>, Vec<bool>) {
let m = current.len();
let n = desired.len();
@@ -451,7 +459,7 @@ fn lcs_flags(current: &[PlaybackItem], desired: &[PlaybackItem]) -> (Vec<bool>,
for i in 0..m {
for j in 0..n {
if current[i].uri == desired[j].uri {
if items_match(&current[i], &desired[j]) {
dp[i + 1][j + 1] = dp[i][j] + 1;
} else {
dp[i + 1][j + 1] = dp[i + 1][j].max(dp[i][j + 1]);
@@ -464,7 +472,7 @@ fn lcs_flags(current: &[PlaybackItem], desired: &[PlaybackItem]) -> (Vec<bool>,
let (mut i, mut j) = (m, n);
while i > 0 && j > 0 {
if current[i - 1].uri == desired[j - 1].uri {
if items_match(&current[i - 1], &desired[j - 1]) {
keep_current[i - 1] = true;
keep_desired[j - 1] = true;
i -= 1;
@@ -616,6 +624,7 @@ impl QueueBackend for OpenHomeQueue {
idx,
snapshot.items[idx].backend_id,
snapshot.items[idx].uri.clone(),
snapshot.items[idx].didl_id.clone(),
))
});
@@ -626,9 +635,16 @@ impl QueueBackend for OpenHomeQueue {
"OpenHome playlist state"
);
if let Some((playing_idx, playing_id, playing_uri)) = playing_info {
// Find if the currently playing item is in the new playlist (by URI)
let new_playing_idx = items.iter().position(|item| item.uri == playing_uri);
if let Some((playing_idx, playing_id, playing_uri, playing_didl_id)) = playing_info {
// Find if the currently playing item is in the new playlist (by URI first, then by didl_id)
let new_playing_idx = items
.iter()
.position(|item| item.uri == playing_uri)
.or_else(|| {
items
.iter()
.position(|item| item.didl_id == playing_didl_id)
});
if let Some(pivot_idx) = new_playing_idx {
// CASE 2: Currently playing item IS in the new playlist