🚀 refactor(pmocontrol): eliminate QueueBackend boilerplate with HasQueue blanket impl

- Add `Hasqueue` trait and implement it for all renderers (Upnp, OpenHome, LinkPlay, ArylicTcp, Chromecast)
- Replace manual `QueueBackend` implementations with blanket impl for types implementing Hasqueue
  (removes ~30+ duplicated methods across renderers)
- Fix BUG: `sync_queue` in UpnpRenderer now correctly propagates cancel_token instead of ignoring it
- Update version to 0.3.48 in Cargo.toml, lockfile and root file
- Add refactoring plan document (`refactoring_pmocontrol.md`) detailing remaining P1-P3 tasks
This commit is contained in:
2026-04-10 00:06:15 +02:00
parent e8e33414f0
commit 9e447023a8
12 changed files with 474 additions and 403 deletions

View File

@@ -18,8 +18,7 @@ use crate::music_renderer::capabilities::{
use crate::music_renderer::musicrenderer::MusicRendererBackend;
use crate::music_renderer::time_utils::{format_hhmmss, ms_to_seconds, parse_hhmmss_strict};
use crate::music_renderer::RendererFromMediaRendererInfo;
use crate::queue::MusicQueue;
use crate::queue::{EnqueueMode, PlaybackItem, QueueBackend, QueueSnapshot};
use crate::queue::{EnqueueMode, HasQueue, MusicQueue, PlaybackItem, QueueBackend, QueueSnapshot};
use crate::DeviceIdentity;
/// Raw response from Arylic MCU+PINFGET command
@@ -365,76 +364,9 @@ impl QueueTransportControl for ArylicTcpRenderer {
}
}
impl QueueBackend for ArylicTcpRenderer {
fn len(&self) -> Result<usize, ControlPointError> {
self.queue.lock().unwrap().len()
}
fn track_ids(&self) -> Result<Vec<u32>, ControlPointError> {
self.queue.lock().unwrap().track_ids()
}
fn id_to_position(&self, id: u32) -> Result<usize, ControlPointError> {
self.queue.lock().unwrap().id_to_position(id)
}
fn position_to_id(&self, id: usize) -> Result<u32, ControlPointError> {
self.queue.lock().unwrap().position_to_id(id)
}
fn current_track(&self) -> Result<Option<u32>, ControlPointError> {
self.queue.lock().unwrap().current_track()
}
fn current_index(&self) -> Result<Option<usize>, ControlPointError> {
self.queue.lock().unwrap().current_index()
}
fn queue_snapshot(&self) -> Result<QueueSnapshot, ControlPointError> {
self.queue.lock().unwrap().queue_snapshot()
}
fn set_index(&mut self, index: Option<usize>) -> Result<(), ControlPointError> {
self.queue.lock().unwrap().set_index(index)
}
fn replace_queue(
&mut self,
items: Vec<PlaybackItem>,
current_index: Option<usize>,
) -> Result<(), ControlPointError> {
self.queue
.lock()
.unwrap()
.replace_queue(items, current_index)
}
fn sync_queue(
&mut self,
items: Vec<PlaybackItem>,
_cancel_token: &Arc<AtomicBool>,
on_ready: Option<Box<dyn FnOnce() + Send>>,
) -> Result<(), ControlPointError> {
self.queue
.lock()
.unwrap()
.sync_queue(items, &Arc::new(AtomicBool::new(false)), on_ready)
}
fn get_item(&self, index: usize) -> Result<Option<PlaybackItem>, ControlPointError> {
self.queue.lock().unwrap().get_item(index)
}
fn replace_item(&mut self, index: usize, item: PlaybackItem) -> Result<(), ControlPointError> {
self.queue.lock().unwrap().replace_item(index, item)
}
fn enqueue_items(
&mut self,
items: Vec<PlaybackItem>,
mode: EnqueueMode,
) -> Result<(), ControlPointError> {
self.queue.lock().unwrap().enqueue_items(items, mode)
impl HasQueue for ArylicTcpRenderer {
fn queue(&self) -> &Arc<Mutex<MusicQueue>> {
&self.queue
}
}

View File

@@ -1,9 +1,13 @@
// pmocontrol/src/capabilities.rs
use anyhow::Result;
use std::sync::{Arc, Mutex};
use crate::queue::MusicQueue;
use crate::{errors::ControlPointError, model::PlaybackState};
use crate::queue::{HasQueue, MusicQueue};
use crate::{errors::ControlPointError, model::PlaybackState, PlaybackItem};
/// Trait for types that track whether they're playing a continuous stream.
pub trait HasContinuousStream {
fn continuous_stream(&self) -> &Arc<Mutex<bool>>;
}
/// Backend-specific operations for renderers.
///
@@ -18,7 +22,39 @@ pub trait RendererBackend {
/// These operations combine queue management with transport control,
/// allowing navigation (next/previous) and track selection from the queue.
#[allow(dead_code)]
pub trait QueueTransportControl {
pub trait QueueTransportControl: HasQueue + HasContinuousStream {
/// Play a specific item from the queue (backend-specific implementation).
fn play_item(&self, item: &PlaybackItem) -> Result<(), ControlPointError>;
/// Play from the queue at the current index (or initialize to 0 if not set).
/// This is the default implementation that handles queue navigation.
fn play_from_queue(&self) -> Result<(), ControlPointError> {
let mut queue = self.queue().lock().unwrap();
let current_index = match queue.current_index()? {
Some(idx) => idx,
None => {
if queue.len()? > 0 {
queue.set_index(Some(0))?;
0
} else {
return Err(ControlPointError::QueueError("Queue is empty".into()));
}
}
};
let item = queue
.get_item(current_index)?
.ok_or_else(|| ControlPointError::QueueError("Current item not found".into()))?;
drop(queue);
let is_stream = crate::music_renderer::is_continuous_stream_url(&item.uri);
*self.continuous_stream().lock().unwrap() = is_stream;
self.play_item(&item)
}
/// Play the next track from the queue.
fn play_next(&self) -> Result<(), ControlPointError>;
@@ -26,9 +62,6 @@ pub trait QueueTransportControl {
#[allow(dead_code)]
fn play_previous(&self) -> Result<(), ControlPointError>;
/// Play from the queue at the current index (or initialize to 0 if not set).
fn play_from_queue(&self) -> Result<(), ControlPointError>;
/// Play from a specific index in the queue.
fn play_from_index(&self, index: usize) -> Result<(), ControlPointError>;
}

View File

@@ -29,7 +29,7 @@ use crate::music_renderer::capabilities::{
use crate::music_renderer::musicrenderer::MusicRendererBackend;
use crate::music_renderer::time_utils::{format_hhmmss_f64, parse_hhmmss_strict};
use crate::music_renderer::RendererFromMediaRendererInfo;
use crate::queue::{EnqueueMode, MusicQueue, PlaybackItem, QueueBackend, QueueSnapshot};
use crate::queue::{EnqueueMode, HasQueue, MusicQueue, PlaybackItem, QueueBackend, QueueSnapshot};
use crate::DeviceIdentity;
use rust_cast::{
@@ -863,75 +863,8 @@ impl QueueTransportControl for ChromecastRenderer {
}
}
impl QueueBackend for ChromecastRenderer {
fn len(&self) -> Result<usize, ControlPointError> {
self.queue.lock().unwrap().len()
}
fn track_ids(&self) -> Result<Vec<u32>, ControlPointError> {
self.queue.lock().unwrap().track_ids()
}
fn id_to_position(&self, id: u32) -> Result<usize, ControlPointError> {
self.queue.lock().unwrap().id_to_position(id)
}
fn position_to_id(&self, id: usize) -> Result<u32, ControlPointError> {
self.queue.lock().unwrap().position_to_id(id)
}
fn current_track(&self) -> Result<Option<u32>, ControlPointError> {
self.queue.lock().unwrap().current_track()
}
fn current_index(&self) -> Result<Option<usize>, ControlPointError> {
self.queue.lock().unwrap().current_index()
}
fn queue_snapshot(&self) -> Result<QueueSnapshot, ControlPointError> {
self.queue.lock().unwrap().queue_snapshot()
}
fn set_index(&mut self, index: Option<usize>) -> Result<(), ControlPointError> {
self.queue.lock().unwrap().set_index(index)
}
fn replace_queue(
&mut self,
items: Vec<PlaybackItem>,
current_index: Option<usize>,
) -> Result<(), ControlPointError> {
self.queue
.lock()
.unwrap()
.replace_queue(items, current_index)
}
fn sync_queue(
&mut self,
items: Vec<PlaybackItem>,
_cancel_token: &Arc<AtomicBool>,
on_ready: Option<Box<dyn FnOnce() + Send>>,
) -> Result<(), ControlPointError> {
self.queue
.lock()
.unwrap()
.sync_queue(items, &Arc::new(AtomicBool::new(false)), on_ready)
}
fn get_item(&self, index: usize) -> Result<Option<PlaybackItem>, ControlPointError> {
self.queue.lock().unwrap().get_item(index)
}
fn replace_item(&mut self, index: usize, item: PlaybackItem) -> Result<(), ControlPointError> {
self.queue.lock().unwrap().replace_item(index, item)
}
fn enqueue_items(
&mut self,
items: Vec<PlaybackItem>,
mode: EnqueueMode,
) -> Result<(), ControlPointError> {
self.queue.lock().unwrap().enqueue_items(items, mode)
impl HasQueue for ChromecastRenderer {
fn queue(&self) -> &Arc<Mutex<MusicQueue>> {
&self.queue
}
}

View File

@@ -16,8 +16,7 @@ use crate::music_renderer::capabilities::{
use crate::music_renderer::musicrenderer::MusicRendererBackend;
use crate::music_renderer::time_utils::parse_hhmmss_strict;
use crate::music_renderer::RendererFromMediaRendererInfo;
use crate::queue::MusicQueue;
use crate::queue::{EnqueueMode, PlaybackItem, QueueBackend, QueueSnapshot};
use crate::queue::{EnqueueMode, HasQueue, MusicQueue, PlaybackItem, QueueBackend, QueueSnapshot};
use crate::DeviceIdentity;
const DEFAULT_HTTP_TIMEOUT_SECS: u64 = 3;
@@ -243,75 +242,8 @@ impl QueueTransportControl for LinkPlayRenderer {
}
}
impl QueueBackend for LinkPlayRenderer {
fn len(&self) -> Result<usize, ControlPointError> {
self.queue.lock().unwrap().len()
}
fn track_ids(&self) -> Result<Vec<u32>, ControlPointError> {
self.queue.lock().unwrap().track_ids()
}
fn id_to_position(&self, id: u32) -> Result<usize, ControlPointError> {
self.queue.lock().unwrap().id_to_position(id)
}
fn position_to_id(&self, id: usize) -> Result<u32, ControlPointError> {
self.queue.lock().unwrap().position_to_id(id)
}
fn current_track(&self) -> Result<Option<u32>, ControlPointError> {
self.queue.lock().unwrap().current_track()
}
fn current_index(&self) -> Result<Option<usize>, ControlPointError> {
self.queue.lock().unwrap().current_index()
}
fn queue_snapshot(&self) -> Result<QueueSnapshot, ControlPointError> {
self.queue.lock().unwrap().queue_snapshot()
}
fn set_index(&mut self, index: Option<usize>) -> Result<(), ControlPointError> {
self.queue.lock().unwrap().set_index(index)
}
fn replace_queue(
&mut self,
items: Vec<PlaybackItem>,
current_index: Option<usize>,
) -> Result<(), ControlPointError> {
self.queue
.lock()
.unwrap()
.replace_queue(items, current_index)
}
fn sync_queue(
&mut self,
items: Vec<PlaybackItem>,
_cancel_token: &Arc<AtomicBool>,
on_ready: Option<Box<dyn FnOnce() + Send>>,
) -> Result<(), ControlPointError> {
self.queue
.lock()
.unwrap()
.sync_queue(items, &Arc::new(AtomicBool::new(false)), on_ready)
}
fn get_item(&self, index: usize) -> Result<Option<PlaybackItem>, ControlPointError> {
self.queue.lock().unwrap().get_item(index)
}
fn replace_item(&mut self, index: usize, item: PlaybackItem) -> Result<(), ControlPointError> {
self.queue.lock().unwrap().replace_item(index, item)
}
fn enqueue_items(
&mut self,
items: Vec<PlaybackItem>,
mode: EnqueueMode,
) -> Result<(), ControlPointError> {
self.queue.lock().unwrap().enqueue_items(items, mode)
impl HasQueue for LinkPlayRenderer {
fn queue(&self) -> &Arc<Mutex<MusicQueue>> {
&self.queue
}
}

View File

@@ -16,7 +16,7 @@ use crate::music_renderer::openhome::{
build_time_client, build_volume_client,
};
use crate::music_renderer::RendererFromMediaRendererInfo;
use crate::queue::{EnqueueMode, MusicQueue, PlaybackItem, QueueBackend, QueueSnapshot};
use crate::queue::{EnqueueMode, HasQueue, MusicQueue, PlaybackItem, QueueBackend, QueueSnapshot};
use crate::upnp_clients::{
OhInfoClient, OhPlaylistClient, OhProductClient, OhRadioClient, OhTimeClient, OhVolumeClient,
OPENHOME_PLAYLIST_HEAD_ID,
@@ -626,90 +626,31 @@ impl QueueTransportControl for OpenHomeRenderer {
}
}
impl QueueBackend for OpenHomeRenderer {
fn len(&self) -> Result<usize, ControlPointError> {
self.queue
.lock()
.map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))?
.len()
impl HasQueue for OpenHomeRenderer {
fn queue(&self) -> &Arc<Mutex<MusicQueue>> {
&self.queue
}
}
fn track_ids(&self) -> Result<Vec<u32>, ControlPointError> {
self.queue
.lock()
.map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))?
.track_ids()
}
fn id_to_position(&self, id: u32) -> Result<usize, ControlPointError> {
self.queue
.lock()
.map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))?
.id_to_position(id)
}
fn position_to_id(&self, id: usize) -> Result<u32, ControlPointError> {
self.queue
.lock()
.map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))?
.position_to_id(id)
}
fn current_track(&self) -> Result<Option<u32>, ControlPointError> {
self.queue
.lock()
.map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))?
.current_track()
}
fn current_index(&self) -> Result<Option<usize>, ControlPointError> {
self.queue
.lock()
.map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))?
.current_index()
}
fn queue_snapshot(&self) -> Result<QueueSnapshot, ControlPointError> {
self.queue
.lock()
.map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))?
.queue_snapshot()
}
fn set_index(&mut self, index: Option<usize>) -> Result<(), ControlPointError> {
self.queue
.lock()
.map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))?
.set_index(index)
}
fn replace_queue(
impl OpenHomeRenderer {
pub fn replace_queue_with_background(
&mut self,
items: Vec<PlaybackItem>,
current_index: Option<usize>,
) -> Result<(), ControlPointError> {
// ✅ CORRECTION BUG PRODUCTION: On ne charge PAS toutes les métadonnées
// dans le thread principal. OpenHome sur 1000 titres inondait la base SQLite
// et bloquait TOUS les autres threads (mutex >500ms).
//
// On fait juste l'insertion minimaliste maintenant. Le préchargement
// des métadonnées est délégué à un thread background.
self.queue
.lock()
.map_err(|_| ControlPointError::QueueError("Mutex poisoned".into()))?
.replace_queue(items, current_index)?;
// Background worker: charge les métadonnées petit à petit sans bloquer personne
let queue = self.queue.clone();
std::thread::spawn(move || {
debug!("🔄 OpenHome: préchargement métadonnées queue en background");
if let Ok(mut queue) = queue.lock() {
// On ne fait que les 10 prochains titres maintenant, le reste on s'en fout
if let Ok(Some(idx)) = queue.current_index() {
let end = std::cmp::min(idx + 10, queue.len().unwrap_or(0));
for i in idx..end {
let _ = queue.get_item(i);
// Petit délai pour ne pas noyer la base de données
std::thread::sleep(std::time::Duration::from_millis(5));
}
}
@@ -719,41 +660,4 @@ impl QueueBackend for OpenHomeRenderer {
Ok(())
}
fn sync_queue(
&mut self,
items: Vec<PlaybackItem>,
cancel_token: &Arc<AtomicBool>,
on_ready: Option<Box<dyn FnOnce() + Send>>,
) -> Result<(), ControlPointError> {
self.queue
.lock()
.map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))?
.sync_queue(items, cancel_token, on_ready)
}
fn get_item(&self, index: usize) -> Result<Option<PlaybackItem>, ControlPointError> {
self.queue
.lock()
.map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))?
.get_item(index)
}
fn replace_item(&mut self, index: usize, item: PlaybackItem) -> Result<(), ControlPointError> {
self.queue
.lock()
.map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))?
.replace_item(index, item)
}
fn enqueue_items(
&mut self,
items: Vec<PlaybackItem>,
mode: EnqueueMode,
) -> Result<(), ControlPointError> {
self.queue
.lock()
.map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))?
.enqueue_items(items, mode)
}
}

View File

@@ -3,12 +3,12 @@ use std::sync::{atomic::AtomicBool, Arc, Mutex};
use crate::errors::ControlPointError;
use crate::model::PlaybackState;
use crate::music_renderer::capabilities::{
PlaybackPosition, PlaybackPositionInfo, PlaybackStatus, QueueTransportControl, RendererBackend,
TransportControl, VolumeControl,
HasContinuousStream, PlaybackPosition, PlaybackPositionInfo, PlaybackStatus,
QueueTransportControl, RendererBackend, TransportControl, VolumeControl,
};
use crate::music_renderer::musicrenderer::{build_didl_lite_metadata, MusicRendererBackend};
use crate::music_renderer::RendererFromMediaRendererInfo;
use crate::queue::{EnqueueMode, MusicQueue, PlaybackItem, QueueBackend, QueueSnapshot};
use crate::queue::{EnqueueMode, HasQueue, MusicQueue, PlaybackItem, QueueBackend, QueueSnapshot};
use crate::upnp_clients::{
AvTransportClient, ConnectionInfo, ConnectionManagerClient, PositionInfo, ProtocolInfo,
RenderingControlClient,
@@ -307,76 +307,9 @@ impl QueueTransportControl for UpnpRenderer {
}
}
impl QueueBackend for UpnpRenderer {
fn len(&self) -> Result<usize, ControlPointError> {
self.queue.lock().unwrap().len()
}
fn track_ids(&self) -> Result<Vec<u32>, ControlPointError> {
self.queue.lock().unwrap().track_ids()
}
fn id_to_position(&self, id: u32) -> Result<usize, ControlPointError> {
self.queue.lock().unwrap().id_to_position(id)
}
fn position_to_id(&self, id: usize) -> Result<u32, ControlPointError> {
self.queue.lock().unwrap().position_to_id(id)
}
fn current_track(&self) -> Result<Option<u32>, ControlPointError> {
self.queue.lock().unwrap().current_track()
}
fn current_index(&self) -> Result<Option<usize>, ControlPointError> {
self.queue.lock().unwrap().current_index()
}
fn queue_snapshot(&self) -> Result<QueueSnapshot, ControlPointError> {
self.queue.lock().unwrap().queue_snapshot()
}
fn set_index(&mut self, index: Option<usize>) -> Result<(), ControlPointError> {
self.queue.lock().unwrap().set_index(index)
}
fn replace_queue(
&mut self,
items: Vec<PlaybackItem>,
current_index: Option<usize>,
) -> Result<(), ControlPointError> {
self.queue
.lock()
.unwrap()
.replace_queue(items, current_index)
}
fn sync_queue(
&mut self,
items: Vec<PlaybackItem>,
_cancel_token: &Arc<AtomicBool>,
on_ready: Option<Box<dyn FnOnce() + Send>>,
) -> Result<(), ControlPointError> {
self.queue
.lock()
.unwrap()
.sync_queue(items, &Arc::new(AtomicBool::new(false)), on_ready)
}
fn get_item(&self, index: usize) -> Result<Option<PlaybackItem>, ControlPointError> {
self.queue.lock().unwrap().get_item(index)
}
fn replace_item(&mut self, index: usize, item: PlaybackItem) -> Result<(), ControlPointError> {
self.queue.lock().unwrap().replace_item(index, item)
}
fn enqueue_items(
&mut self,
items: Vec<PlaybackItem>,
mode: EnqueueMode,
) -> Result<(), ControlPointError> {
self.queue.lock().unwrap().enqueue_items(items, mode)
impl HasQueue for UpnpRenderer {
fn queue(&self) -> &Arc<Mutex<MusicQueue>> {
&self.queue
}
}

View File

@@ -28,8 +28,89 @@
//! - This identity is used by the sync helpers to preserve the current
//! track across queue rebuilds when the MediaServer content changes.
use crate::queue::MusicQueue;
use crate::{errors::ControlPointError, PlaybackItem, QueueSnapshot};
use std::sync::{atomic::AtomicBool, Arc};
use std::sync::{atomic::AtomicBool, Arc, Mutex};
/// Trait for types that have aMusicQueue.
pub trait HasQueue {
fn queue(&self) -> &Arc<Mutex<MusicQueue>>;
}
/// Blanket implementation of QueueBackend for types that have a queue.
/// All methods simply delegate to the underlying MusicQueue.
impl<T: HasQueue> QueueBackend for T {
fn len(&self) -> Result<usize, ControlPointError> {
self.queue().lock().unwrap().len()
}
fn track_ids(&self) -> Result<Vec<u32>, ControlPointError> {
self.queue().lock().unwrap().track_ids()
}
fn id_to_position(&self, id: u32) -> Result<usize, ControlPointError> {
self.queue().lock().unwrap().id_to_position(id)
}
fn position_to_id(&self, id: usize) -> Result<u32, ControlPointError> {
self.queue().lock().unwrap().position_to_id(id)
}
fn current_track(&self) -> Result<Option<u32>, ControlPointError> {
self.queue().lock().unwrap().current_track()
}
fn current_index(&self) -> Result<Option<usize>, ControlPointError> {
self.queue().lock().unwrap().current_index()
}
fn queue_snapshot(&self) -> Result<QueueSnapshot, ControlPointError> {
self.queue().lock().unwrap().queue_snapshot()
}
fn set_index(&mut self, index: Option<usize>) -> Result<(), ControlPointError> {
self.queue().lock().unwrap().set_index(index)
}
fn replace_queue(
&mut self,
items: Vec<PlaybackItem>,
current_index: Option<usize>,
) -> Result<(), ControlPointError> {
self.queue()
.lock()
.unwrap()
.replace_queue(items, current_index)
}
fn sync_queue(
&mut self,
items: Vec<PlaybackItem>,
cancel_token: &Arc<AtomicBool>,
on_ready: Option<Box<dyn FnOnce() + Send>>,
) -> Result<(), ControlPointError> {
self.queue()
.lock()
.unwrap()
.sync_queue(items, cancel_token, on_ready)
}
fn get_item(&self, index: usize) -> Result<Option<PlaybackItem>, ControlPointError> {
self.queue().lock().unwrap().get_item(index)
}
fn replace_item(&mut self, index: usize, item: PlaybackItem) -> Result<(), ControlPointError> {
self.queue().lock().unwrap().replace_item(index, item)
}
fn enqueue_items(
&mut self,
items: Vec<PlaybackItem>,
mode: EnqueueMode,
) -> Result<(), ControlPointError> {
self.queue().lock().unwrap().enqueue_items(items, mode)
}
}
/// High-level enqueue mode.
///

View File

@@ -6,7 +6,7 @@ mod snapshot;
use std::sync::{Arc, Mutex};
pub use backend::{EnqueueMode, QueueBackend};
pub use backend::{EnqueueMode, HasQueue, QueueBackend};
pub use music_queue::{MusicQueue, SyncScheduleOutcome};
pub use snapshot::{PlaybackItem, QueueSnapshot};