From 9935a5e5afe3cd2adb5b0fe168a0a6c100c9331f Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Thu, 9 Apr 2026 19:23:04 +0200 Subject: [PATCH] =?UTF-8?q?:rocket:=20Version=20bump=20to=20v0.3.42=20?= =?UTF-8?q?=E2=86=92=20v0.3.46=20&=20fix=20shuffle/auto-advance=20race=20c?= =?UTF-8?q?ondition?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Bump version to v0.3.46 across Cargo.toml, lockfile & version.txt - 🔧 Fix critical shuffle/auto_advance bug: ensure `has_played_flag` is reset and playback source set *before* backend calls in play_from_index, next/prev & queue init - 📝 Add detailed logging for playback source transitions and index changes to debug timing issues (especially with JBL-like renderers) - 🎯 Introduce `play_current_from_queue_with_retry()` for resilient playback start on flaky UPnP devices --- pmocontrol/src/control_point.rs | 24 ++++++++++++------- .../src/music_renderer/musicrenderer.rs | 23 +++++++++++++++--- pmocontrol/src/queue/music_queue.rs | 22 ++++++++++------- 3 files changed, 50 insertions(+), 19 deletions(-) diff --git a/pmocontrol/src/control_point.rs b/pmocontrol/src/control_point.rs index 42d0f9b8..e440c496 100644 --- a/pmocontrol/src/control_point.rs +++ b/pmocontrol/src/control_point.rs @@ -839,11 +839,17 @@ impl ControlPoint { "Playing current playback item from queue" ); - // Temporarily disable auto-advance to prevent race condition - // when renderer sends Stopped event during SetAVTransportURI - renderer.set_playback_source(PlaybackSource::None); + // Note: We set playback_source to FromQueue BEFORE calling play_from_queue(). + // This prevents a race condition where: + // 1. play_from_queue() is called + // 2. watcher detects STOPPED from track transition (before playback_source is set) + // 3. with source=None, is_playing_from_queue() returns false, breaking auto-advance + // By setting it BEFORE, even if STOPPED is detected, source will be FromQueue. + // Note: We no longer temporarily set playback_source to None (that caused Bug #2). // Start playback using play_from_queue which preserves the queue + // Set source to FromQueue BEFORE calling to prevent race condition + renderer.set_playback_source(PlaybackSource::FromQueue); if let Err(err) = renderer.play_from_queue() { error!( renderer = renderer_id.0.as_str(), @@ -865,7 +871,7 @@ impl ControlPoint { let metadata = playback_item_track_metadata(&item); renderer.set_last_metadata(Some(metadata)); - renderer.set_playback_source(PlaybackSource::FromQueue); + // Note: playback_source already set to FromQueue above // Prefetch next track if supported (gapless playback) self.prefetch_next_track(&renderer, renderer_id); @@ -895,8 +901,9 @@ impl ControlPoint { return Ok(()); } - // Temporarily disable auto-advance to prevent race condition - renderer.set_playback_source(PlaybackSource::None); + // Note: Same fix as play_current_from_queue - set source BEFORE calling. + // Set source to FromQueue BEFORE calling to prevent race condition + renderer.set_playback_source(PlaybackSource::FromQueue); // Use the backend's play_next which handles queue advancement correctly for each backend type if let Err(err) = renderer.play_next_from_queue() { @@ -1035,8 +1042,9 @@ impl ControlPoint { return Ok(()); } - // Temporarily disable auto-advance to prevent race condition - renderer.set_playback_source(PlaybackSource::None); + // Note: Same fix as play_current_from_queue - set source BEFORE calling. + // Set source to FromQueue BEFORE calling to prevent race condition + renderer.set_playback_source(PlaybackSource::FromQueue); // Use the backend's play_from_index which handles everything correctly if let Err(err) = renderer.play_from_index(index) { diff --git a/pmocontrol/src/music_renderer/musicrenderer.rs b/pmocontrol/src/music_renderer/musicrenderer.rs index f3a8badd..8adc1090 100644 --- a/pmocontrol/src/music_renderer/musicrenderer.rs +++ b/pmocontrol/src/music_renderer/musicrenderer.rs @@ -1000,6 +1000,11 @@ impl MusicRenderer { // The flag will be set back to true when PLAYING state is detected. self.clear_has_played_flag(); + // Set playback_source to FromQueue BEFORE calling backend + // to prevent race condition where watcher sees STOPPED before + // source is set, breaking auto-advance + self.set_playback_source(PlaybackSource::FromQueue); + self.lock_backend_for("play_current_from_queue") .play_from_queue() } @@ -1011,6 +1016,11 @@ impl MusicRenderer { // The flag will be set back to true when PLAYING state is detected. self.clear_has_played_flag(); + // Set playback_source to FromQueue BEFORE calling backend + // to prevent race condition where watcher sees STOPPED before + // source is set, breaking auto-advance + self.set_playback_source(PlaybackSource::FromQueue); + self.lock_backend_for("play_next_from_queue").play_next()?; self.emit_queue_updated(); Ok(()) @@ -1583,11 +1593,18 @@ impl MusicRenderer { ) } - /// Marks playback as external if currently idle (source is None). + /// Marks playback as external if currently idle. + /// + /// Only sets to External if we were already playing from an external source. + /// Does NOT change None -> External because that would break queue playback + /// (the control_point will set it to FromQueue after play_from_queue succeeds). pub fn mark_external_if_idle(&self) { let mut state = self.state.lock().unwrap(); - if matches!(state.playback_source, PlaybackSource::None) { - state.playback_source = PlaybackSource::External; + if matches!(state.playback_source, PlaybackSource::External) { + // Keep External if we were already playing externally + } else { + // Don't change None -> External - that breaks queue auto-advance! + // The control_point will set playback_source to FromQueue after play_from_queue succeeds. } } diff --git a/pmocontrol/src/queue/music_queue.rs b/pmocontrol/src/queue/music_queue.rs index 139c77e5..4b9318ed 100644 --- a/pmocontrol/src/queue/music_queue.rs +++ b/pmocontrol/src/queue/music_queue.rs @@ -178,7 +178,9 @@ impl MusicQueue { }; // Queue lock is released here. // Now safe to call on_ready (which may re-lock the queue). - if on_ready_triggered.load(SeqCst) { + // If on_ready was triggered by the proxy, consume and call it. + // If not (cancelled before first insert), keep it to pass to retry. + let carry_on_ready = if on_ready_triggered.load(SeqCst) { tracing::debug!( thread = %std::thread::current().name().unwrap_or("?"), "queue-sync: on_ready triggered, calling callback" @@ -186,12 +188,16 @@ impl MusicQueue { if let Some(f) = real_on_ready { f(); } - } else if real_on_ready.is_some() { - tracing::debug!( - thread = %std::thread::current().name().unwrap_or("?"), - "queue-sync: on_ready not triggered (cancelled or skipped)" - ); - } + None + } else { + if real_on_ready.is_some() { + tracing::debug!( + thread = %std::thread::current().name().unwrap_or("?"), + "queue-sync: on_ready not triggered, carrying to next attempt" + ); + } + real_on_ready + }; match result { Err(ControlPointError::SyncCancelled) => { @@ -222,7 +228,7 @@ impl MusicQueue { match pending_items_fn() { Ok(new_items) => { current_items = new_items; - current_on_ready = Some(None); + current_on_ready = Some(carry_on_ready); } Err(e) => { tracing::warn!("queue-sync pending re-fetch error: {}", e);