From 6fd11ab45011cdee99ff0f4b864f75a8b8f76a5a Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Thu, 9 Apr 2026 19:52:39 +0200 Subject: [PATCH] :recycle:, feat: add retry logic for transient renderer errors - Replace play_current_from_queue with new `playCurrentFromQueueWithRetry` method in control_point.rs - Replace play_next_from_queue with `playNextFromQueueWithRetry` and add panic safety in musicrenderer.rs - Implement retry logic (3 attempts, 200ms delay) for transient renderer errors like JBL Authentics 300 - Deprecate old methods without retry support --- pmocontrol/src/control_point.rs | 6 +- .../src/music_renderer/musicrenderer.rs | 92 ++++++++++++++++++- 2 files changed, 93 insertions(+), 5 deletions(-) diff --git a/pmocontrol/src/control_point.rs b/pmocontrol/src/control_point.rs index e440c496..c6fe554d 100644 --- a/pmocontrol/src/control_point.rs +++ b/pmocontrol/src/control_point.rs @@ -1186,7 +1186,8 @@ impl ControlPoint { let renderer = reg.read().unwrap().get_renderer(rid).ok_or_else(|| { ControlPointError::ControlPoint(format!("Renderer {} not found", rid.0)) })?; - renderer.play_current_from_queue() + // Use retry version to handle transient renderer errors (like JBL Authentics 300) + renderer.play_current_from_queue_with_retry() })) } else { None @@ -1258,7 +1259,8 @@ impl ControlPoint { let renderer = reg.read().unwrap().get_renderer(rid).ok_or_else(|| { ControlPointError::ControlPoint(format!("Renderer {} not found", rid.0)) })?; - renderer.play_current_from_queue() + // Use retry version to handle transient renderer errors (like JBL Authentics 300) + renderer.play_current_from_queue_with_retry() })) } else { None diff --git a/pmocontrol/src/music_renderer/musicrenderer.rs b/pmocontrol/src/music_renderer/musicrenderer.rs index 8adc1090..cd05675e 100644 --- a/pmocontrol/src/music_renderer/musicrenderer.rs +++ b/pmocontrol/src/music_renderer/musicrenderer.rs @@ -703,8 +703,9 @@ impl MusicRenderer { ); // Use catch_unwind to prevent panics in play_next_from_queue // from poisoning the backend mutex + // Also add retry logic for transient errors from renderer let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - self.play_next_from_queue() + self.play_next_from_queue_with_retry() })); match result { Ok(Ok(())) => {} @@ -712,7 +713,7 @@ impl MusicRenderer { error!( renderer = self.info.friendly_name(), error = %err, - "Auto-advance failed; clearing queue playback state" + "Auto-advance failed after retries; clearing queue playback state" ); self.set_playback_source(PlaybackSource::None); } @@ -993,9 +994,60 @@ impl MusicRenderer { self.lock_backend_for("upcoming_len").upcoming_len() } + /// Play the current item from the queue with retry logic for transient renderer errors. + /// + /// Some renderers (like JBL Authentics 300) may fail the first Play command + /// due to timing issues. This method retries with a small delay. + pub fn play_current_from_queue_with_retry(&self) -> Result<(), ControlPointError> { + const MAX_RETRIES: usize = 3; + const RETRY_DELAY_MS: u64 = 200; + + let mut last_error = None; + + for attempt in 0..MAX_RETRIES { + // 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(); + + // 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); + + match self + .lock_backend_for("play_current_from_queue_with_retry") + .play_from_queue() + { + Ok(()) => return Ok(()), + Err(e) => { + last_error = Some(e); + if attempt < MAX_RETRIES - 1 { + tracing::warn!( + renderer = self.info.friendly_name(), + attempt = attempt + 1, + error = %last_error.as_ref().unwrap(), + "Initial play attempt failed, retrying..." + ); + std::thread::sleep(std::time::Duration::from_millis(RETRY_DELAY_MS)); + } + } + } + } + + // All retries failed + Err(last_error.unwrap_or_else(|| { + ControlPointError::ControlPoint("Unknown error in retry logic".into()) + })) + } + /// Play the current item from the queue. + #[deprecated( + since = "0.1.0", + note = "Use play_current_from_queue_with_retry instead" + )] pub fn play_current_from_queue(&self) -> Result<(), ControlPointError> { - // Reset the has_played flag before starting playback to prevent + // 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(); @@ -1026,6 +1078,40 @@ impl MusicRenderer { Ok(()) } + /// Play next from queue with retry logic for transient renderer errors. + /// + /// Some renderers (like JBL Authentics 300) may fail the first Play command + /// due to timing issues. This method retries with a small delay. + pub fn play_next_from_queue_with_retry(&self) -> Result<(), ControlPointError> { + const MAX_RETRIES: usize = 3; + const RETRY_DELAY_MS: u64 = 200; + + let mut last_error = None; + + for attempt in 0..MAX_RETRIES { + match self.play_next_from_queue() { + Ok(()) => return Ok(()), + Err(e) => { + last_error = Some(e); + if attempt < MAX_RETRIES - 1 { + tracing::warn!( + renderer = self.info.friendly_name(), + attempt = attempt + 1, + error = %last_error.as_ref().unwrap(), + "Auto-advance attempt failed, retrying..." + ); + std::thread::sleep(std::time::Duration::from_millis(RETRY_DELAY_MS)); + } + } + } + } + + // All retries failed + Err(last_error.unwrap_or_else(|| { + ControlPointError::ControlPoint("Unknown error in retry logic".into()) + })) + } + /// Advance the queue index by one without starting playback. /// /// Used by the WebRenderer gapless path: the browser autonomously transitions