From 9e447023a8f0eb512910eb324ff6788da3e3cbeb Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Fri, 10 Apr 2026 00:06:15 +0200 Subject: [PATCH] :rocket: 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 --- Blackboard/Todo/refactoring_pmocontrol.md | 323 ++++++++++++++++++ Cargo.lock | 2 +- PMOMusic/Cargo.toml | 2 +- pmocontrol/src/music_renderer/arylic_tcp.rs | 76 +---- pmocontrol/src/music_renderer/capabilities.rs | 47 ++- .../src/music_renderer/chromecast_renderer.rs | 75 +--- .../src/music_renderer/linkplay_renderer.rs | 76 +---- .../src/music_renderer/openhome_renderer.rs | 110 +----- .../src/music_renderer/upnp_renderer.rs | 79 +---- pmocontrol/src/queue/backend.rs | 83 ++++- pmocontrol/src/queue/mod.rs | 2 +- version.txt | 2 +- 12 files changed, 474 insertions(+), 403 deletions(-) create mode 100644 Blackboard/Todo/refactoring_pmocontrol.md diff --git a/Blackboard/Todo/refactoring_pmocontrol.md b/Blackboard/Todo/refactoring_pmocontrol.md new file mode 100644 index 00000000..70bd7edf --- /dev/null +++ b/Blackboard/Todo/refactoring_pmocontrol.md @@ -0,0 +1,323 @@ +# Refactoring Plan - Crate `pmocontrol` + +## Contexte + +La crate `pmocontrol` implémente un control point UPnP multiprotocole pour contrôler des renderers audio (UPnP/DLNA, OpenHome, LinkPlay, Arylic TCP, Chromecast). Une refactorisation récente avait pour objectif de monter la logique vers les couches abstraites, mais des duplications et des problèmes de conception subsistent. + +--- + +## Structure analysée + +- `music_renderer/` : Implémentations concrètes + façade `MusicRenderer` +- `queue/` : Gestion abstraite et concrète des files de lecture +- `discovery/` : Découverte SSDP et gestion des appareils +- `upnp_clients/` : Clients SOAP pour services UPnP +- `control_point.rs` : Point de contrôle principal + +Backends : `UpnpRenderer`, `OpenHomeRenderer`, `LinkPlayRenderer`, `ArylicTcpRenderer`, `ChromecastRenderer`, `HybridUpnpArylicRenderer` + +--- + +## CATÉGORIE P0 : BUGS LOGIQUES (à corriger immédiatement) + +### BUG-1 : `sync_queue` dans UpnpRenderer ignore le cancel_token + +**Fichier :** `src/music_renderer/upnp_renderer.rs` (lignes ~354-364) + +**Description :** Le paramètre `cancel_token` est reçu comme `_cancel_token` (ignoré) et remplacé par un `Arc::new(AtomicBool::new(false))` fraîchement créé. Les demandes d'annulation de synchronisation de queue sont silencieusement ignorées pour le backend UPnP. + +**Correction :** +```rust +fn sync_queue( + &mut self, + items: Vec, + cancel_token: &Arc, // utiliser le param, pas _cancel_token + on_ready: Option>, +) -> Result<(), ControlPointError> { + self.queue + .lock() + .unwrap() + .sync_queue(items, cancel_token, on_ready) // passer le vrai token +} +``` + +**Tâche :** Vérifier également les autres backends (OpenHome, LinkPlay, Arylic, Chromecast) s'ils propagent correctement le cancel_token. + +--- + +## CATÉGORIE P1 : DUPLICATIONS MAJEURES (à traiter en priorité) + +### DUP-1 : Implémentation de `QueueBackend` répétée dans les 5+ renderers + +**Fichiers :** +- `src/music_renderer/upnp_renderer.rs` (~310-381) +- `src/music_renderer/arylic_tcp.rs` (~368-450) +- `src/music_renderer/linkplay_renderer.rs` (~246-330) +- `src/music_renderer/chromecast_renderer.rs` (~866+) +- `src/music_renderer/openhome_renderer.rs` (~629+) + +**Description :** Chaque renderer implémente `QueueBackend` de manière identique : chaque méthode verrouille `self.queue` et délègue à la file sous-jacente. ~150+ lignes de boilerplate. + +**Approche recommandée — Trait délégateur :** +```rust +// Dans queue/mod.rs ou music_renderer/mod.rs +pub trait HasQueue { + fn queue(&self) -> &Arc>; +} + +// Impl automatique pour QueueBackend si le type implémente HasQueue +impl QueueBackend for T { + fn len(&self) -> Result { + self.queue().lock().unwrap().len() + } + fn track_ids(&self) -> Result, ControlPointError> { + self.queue().lock().unwrap().track_ids() + } + // ... toutes les méthodes déléguantes +} + +// Dans chaque renderer : une seule ligne +impl HasQueue for UpnpRenderer { + fn queue(&self) -> &Arc> { &self.queue } +} +``` + +**Tâche :** Définir le trait `HasQueue`, implémenter `QueueBackend for T where T: HasQueue`, supprimer les implémentations manuelles dans chaque renderer. + +--- + +### DUP-2 : Logique commune de `play_from_queue` dupliquée dans 4+ renderers + +**Fichiers :** +- `src/music_renderer/upnp_renderer.rs` (~184-256) +- `src/music_renderer/linkplay_renderer.rs` (~189-212) +- `src/music_renderer/arylic_tcp.rs` (~311-334) +- `src/music_renderer/openhome_renderer.rs` (~partie similaire) + +**Description :** Les 10-12 premières lignes de `play_from_queue` sont identiques dans tous les renderers : verrouillage de queue, gestion de l'index courant, fallback sur index 0 si non défini, récupération de l'item. Seule la partie terminale (play effectif sur le backend) diffère. + +**Approche recommandée — Méthode par défaut dans un trait :** +```rust +pub trait QueueTransportControl: HasQueue + HasContinuousStream { + // Primitive spécifique au backend + fn play_item(&self, item: &PlaybackItem) -> Result<(), ControlPointError>; + + // Implémentation commune par défaut + 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 = is_continuous_stream_url(&item.uri); + *self.continuous_stream().lock().unwrap() = is_stream; + self.play_item(&item) + } +} +``` + +**Tâche :** Créer `QueueTransportControl` avec une méthode par défaut, implémenter `play_item` dans chaque renderer, supprimer la logique commune dupliquée. + +--- + +### DUP-3 : Initialisation redondante des champs partagés dans tous les renderers + +**Fichiers :** Constructeurs dans tous les fichiers renderer + +**Description :** Chaque renderer répète la même construction : +```rust +let queue = Arc::new(Mutex::new(MusicQueue::from_renderer_info(info)?)); +// ... +continuous_stream: Arc::new(Mutex::new(false)), +``` + +**Approche recommandée :** +```rust +pub struct SharedRendererState { + pub queue: Arc>, + pub continuous_stream: Arc>, +} + +impl SharedRendererState { + pub fn from_renderer_info(info: &RendererInfo) -> Result { + Ok(Self { + queue: Arc::new(Mutex::new(MusicQueue::from_renderer_info(info)?)), + continuous_stream: Arc::new(Mutex::new(false)), + }) + } +} +``` + +**Tâche :** Créer `SharedRendererState`, l'utiliser dans tous les constructeurs de renderers. + +--- + +### DUP-4 : `parse_didl_duration` implémentée deux fois différemment + +**Fichiers :** +- `src/music_renderer/upnp_renderer.rs` (~383-420) : parsing manuel par string search (fragile) +- `src/music_renderer/musicrenderer.rs` (~2089-2117) : via parser DIDL-Lite structuré (robuste) + +**Description :** Deux implémentations divergentes. L'une risque de mal parser du DIDL là où l'autre réussit. + +**Tâche :** Conserver uniquement la version via `DIDLLite::parse`, l'exporter depuis `music_renderer/mod.rs`, supprimer la version par string search dans `upnp_renderer.rs`. + +--- + +## CATÉGORIE P2 : ALGORITHMES COMPLEXES ET ABSTRACTIONS MAL PLACÉES + +### ALGO-1 : `schedule_sync` dans `music_queue.rs` — logique intriquée + +**Fichier :** `src/queue/music_queue.rs` (~97-200) + +**Description :** La méthode crée un thread worker avec : +- Des `AtomicBool` pour synchronisation (sync_in_progress, sync_pending, sync_cancel_token) +- Une boucle infinie interne qui re-tente si un nouveau job arrive +- Un Guard RAII basé sur `Drop` pour le cleanup +- Des closures capturées mêlant synchronisation et logique métier + +Difficile à tester, à observer de l'extérieur, pas de timeout. + +**Tâche :** +1. Extraire la logique du worker dans une fonction `sync_worker_loop` avec signature claire +2. Documenter le protocole de synchronisation avec les AtomicBool +3. Ajouter une stratégie de timeout ou de sortie en cas de blocage + +--- + +### ALGO-2 : Enum dispatch sprawl dans `MusicRendererBackend` + +**Fichier :** `src/music_renderer/musicrenderer.rs` (~2134-2495) + +**Description :** L'enum a 6 variantes. Chaque trait implémenté pour l'enum (`TransportControl`, `PlaybackStatus`, `PlaybackPosition`, `RendererBackend`, `QueueBackend`, etc.) contient un `match` sur les 6 variantes. Estimation : 200+ lignes de boilerplate purement mécanique. Ajouter une 7e variante requiert des mises à jour dans 25+ endroits. + +**Approche recommandée — Macro de dispatch :** +```rust +macro_rules! dispatch { + ($self:expr, $method:ident($($arg:expr),*)) => { + match $self { + MusicRendererBackend::Upnp(b) => b.$method($($arg),*), + MusicRendererBackend::OpenHome(b) => b.$method($($arg),*), + MusicRendererBackend::LinkPlay(b) => b.$method($($arg),*), + MusicRendererBackend::ArylicTcp(b) => b.$method($($arg),*), + MusicRendererBackend::Chromecast(b) => b.$method($($arg),*), + MusicRendererBackend::HybridUpnpArylic { upnp, .. } => upnp.$method($($arg),*), + } + } +} + +impl TransportControl for MusicRendererBackend { + fn play_uri(&self, uri: &str, meta: &str) -> Result<(), ControlPointError> { + dispatch!(self, play_uri(uri, meta)) + } + // ... +} +``` + +**Tâche :** Définir la macro `dispatch!`, remplacer les match statements redondants, valider les cas où HybridUpnpArylic a une logique spéciale. + +--- + +### ALGO-3 : Logique de protection des durées de streams dupliquée dans 3 endroits + +**Fichiers :** +- `src/queue/interne.rs` (~74-126) : `protect_stream_durations` +- `src/queue/openhome.rs` (~400+) : logique similaire pour playlists OpenHome +- `src/music_renderer/musicrenderer.rs` (~488-537) : dans `poll_and_emit_changes` + +**Description :** La logique "refuser la diminution de durée pour un stream continu" est réimplémentée trois fois. Si la définition de "diminution acceptable" change, il faut modifier 3 fichiers. + +**Tâche :** Créer `music_renderer/stream_utils.rs` (ou équivalent) avec une fonction `protect_stream_duration(old, new, is_stream) -> Option` et l'utiliser dans les 3 endroits. + +--- + +### ALGO-4 : Détection de flux continu fragmentée + +**Fichiers :** `stream_detection.rs`, `musicrenderer.rs`, `queue/interne.rs`, `queue/openhome.rs` + +**Description :** La détection "est-ce un stream continu?" passe par plusieurs chemins non unifiés : +1. `TrackMetadata::is_continuous_stream` +2. Appel `is_continuous_stream_url(uri)` (réseau) +3. Absence de durée dans les métadonnées + +Un stream peut être marqué continu dans une couche mais pas l'autre. + +**Tâche :** Créer une fonction canonique unique : +```rust +pub fn is_continuous_stream(metadata: Option<&TrackMetadata>, uri: &str) -> bool { + metadata.map(|m| m.is_continuous_stream).unwrap_or(false) + || is_continuous_stream_url(uri) +} +``` +Faire passer tous les codepaths par cette fonction. + +--- + +## CATÉGORIE P3 : BONNES PRATIQUES (amélioration continue) + +### BP-1 : `.unwrap()` sur mutex locks (>50 occurrences) + +**Problème :** Si un mutex est empoisonné (panique dans une autre tâche), `.unwrap()` propage la panique. Aucun code ne gère ce cas. + +**Tâche :** Remplacer `.unwrap()` par `.expect("message contextuel")` à court terme. À long terme, envisager `parking_lot::Mutex` (pas de concept de poison). + +--- + +### BP-2 : Absence de gestion d'erreur dans les threads watcher et sync + +**Fichiers :** `musicrenderer.rs` (watcher_loop), `music_queue.rs` (schedule_sync) + +**Tâche :** Ajouter `error!` logs dans les threads et décider explicitement de la politique de redémarrage (continuer vs arrêter). + +--- + +### BP-3 : Champs `pub` au lieu de `pub(crate)` dans `PlaylistBinding` + +**Fichier :** `src/music_renderer/musicrenderer.rs` (struct `PlaylistBinding`) + +**Tâche :** Rendre les champs `pub` → `pub(crate)` ou privés avec accesseurs. + +--- + +### BP-4 : Documentation manquante sur les contrats des traits + +**Fichiers :** `src/music_renderer/capabilities.rs`, `src/queue/backend.rs` + +**Tâche :** Ajouter des doc-comments sur les traits clés (`TransportControl`, `PlaybackStatus`, `QueueBackend`) décrivant les invariants, les pré/post-conditions, et le comportement attendu. + +--- + +## PLAN D'EXÉCUTION + +### Phase 1 — Bugs (immédiat) +- [ ] **BUG-1** : Corriger le cancel_token ignoré dans `upnp_renderer.rs::sync_queue` +- [ ] Vérifier les autres renderers pour le même bug + +### Phase 2 — Éliminer les duplications majeures (1-2 semaines) +- [ ] **DUP-1** : Trait `HasQueue` + impl automatique de `QueueBackend` +- [ ] **DUP-4** : Unifier `parse_didl_duration` sur la version DIDL-Lite +- [ ] **DUP-3** : Créer `SharedRendererState` pour l'init commune +- [ ] **DUP-2** : Trait `QueueTransportControl` avec `play_from_queue` par défaut + +### Phase 3 — Simplifier les algorithmes (2-4 semaines) +- [ ] **ALGO-2** : Macro `dispatch!` pour `MusicRendererBackend` +- [ ] **ALGO-3** : Centraliser la protection des durées de stream +- [ ] **ALGO-4** : Unifier la détection de flux continu +- [ ] **ALGO-1** : Refactoriser `schedule_sync` (extraire `sync_worker_loop`) + +### Phase 4 — Qualité continue +- [ ] **BP-1** : Remplacer les `.unwrap()` critiques +- [ ] **BP-2** : Gestion d'erreur dans les threads +- [ ] **BP-3** : Visibilité des champs `PlaylistBinding` +- [ ] **BP-4** : Documentation des traits diff --git a/Cargo.lock b/Cargo.lock index f1f8e3f9..fd6c093e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,7 +4,7 @@ version = 4 [[package]] name = "PMOMusic" -version = "0.3.47" +version = "0.3.48" dependencies = [ "axum 0.8.7", "console-subscriber", diff --git a/PMOMusic/Cargo.toml b/PMOMusic/Cargo.toml index 007bf163..32f6d26a 100644 --- a/PMOMusic/Cargo.toml +++ b/PMOMusic/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "PMOMusic" -version = "0.3.47" +version = "0.3.48" edition = "2024" [dependencies] diff --git a/pmocontrol/src/music_renderer/arylic_tcp.rs b/pmocontrol/src/music_renderer/arylic_tcp.rs index 82752bfa..29fdf180 100644 --- a/pmocontrol/src/music_renderer/arylic_tcp.rs +++ b/pmocontrol/src/music_renderer/arylic_tcp.rs @@ -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 { - self.queue.lock().unwrap().len() - } - - fn track_ids(&self) -> Result, ControlPointError> { - self.queue.lock().unwrap().track_ids() - } - - fn id_to_position(&self, id: u32) -> Result { - self.queue.lock().unwrap().id_to_position(id) - } - - fn position_to_id(&self, id: usize) -> Result { - self.queue.lock().unwrap().position_to_id(id) - } - - fn current_track(&self) -> Result, ControlPointError> { - self.queue.lock().unwrap().current_track() - } - - fn current_index(&self) -> Result, ControlPointError> { - self.queue.lock().unwrap().current_index() - } - - fn queue_snapshot(&self) -> Result { - self.queue.lock().unwrap().queue_snapshot() - } - - fn set_index(&mut self, index: Option) -> Result<(), ControlPointError> { - self.queue.lock().unwrap().set_index(index) - } - - fn replace_queue( - &mut self, - items: Vec, - current_index: Option, - ) -> Result<(), ControlPointError> { - self.queue - .lock() - .unwrap() - .replace_queue(items, current_index) - } - - fn sync_queue( - &mut self, - items: Vec, - _cancel_token: &Arc, - on_ready: Option>, - ) -> Result<(), ControlPointError> { - self.queue - .lock() - .unwrap() - .sync_queue(items, &Arc::new(AtomicBool::new(false)), on_ready) - } - - fn get_item(&self, index: usize) -> Result, 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, - mode: EnqueueMode, - ) -> Result<(), ControlPointError> { - self.queue.lock().unwrap().enqueue_items(items, mode) +impl HasQueue for ArylicTcpRenderer { + fn queue(&self) -> &Arc> { + &self.queue } } diff --git a/pmocontrol/src/music_renderer/capabilities.rs b/pmocontrol/src/music_renderer/capabilities.rs index 1f4b049e..1e760681 100644 --- a/pmocontrol/src/music_renderer/capabilities.rs +++ b/pmocontrol/src/music_renderer/capabilities.rs @@ -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>; +} /// 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>; } diff --git a/pmocontrol/src/music_renderer/chromecast_renderer.rs b/pmocontrol/src/music_renderer/chromecast_renderer.rs index 9d6fa61f..39c1f5bd 100644 --- a/pmocontrol/src/music_renderer/chromecast_renderer.rs +++ b/pmocontrol/src/music_renderer/chromecast_renderer.rs @@ -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 { - self.queue.lock().unwrap().len() - } - - fn track_ids(&self) -> Result, ControlPointError> { - self.queue.lock().unwrap().track_ids() - } - - fn id_to_position(&self, id: u32) -> Result { - self.queue.lock().unwrap().id_to_position(id) - } - - fn position_to_id(&self, id: usize) -> Result { - self.queue.lock().unwrap().position_to_id(id) - } - - fn current_track(&self) -> Result, ControlPointError> { - self.queue.lock().unwrap().current_track() - } - - fn current_index(&self) -> Result, ControlPointError> { - self.queue.lock().unwrap().current_index() - } - - fn queue_snapshot(&self) -> Result { - self.queue.lock().unwrap().queue_snapshot() - } - - fn set_index(&mut self, index: Option) -> Result<(), ControlPointError> { - self.queue.lock().unwrap().set_index(index) - } - - fn replace_queue( - &mut self, - items: Vec, - current_index: Option, - ) -> Result<(), ControlPointError> { - self.queue - .lock() - .unwrap() - .replace_queue(items, current_index) - } - - fn sync_queue( - &mut self, - items: Vec, - _cancel_token: &Arc, - on_ready: Option>, - ) -> Result<(), ControlPointError> { - self.queue - .lock() - .unwrap() - .sync_queue(items, &Arc::new(AtomicBool::new(false)), on_ready) - } - - fn get_item(&self, index: usize) -> Result, 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, - mode: EnqueueMode, - ) -> Result<(), ControlPointError> { - self.queue.lock().unwrap().enqueue_items(items, mode) +impl HasQueue for ChromecastRenderer { + fn queue(&self) -> &Arc> { + &self.queue } } diff --git a/pmocontrol/src/music_renderer/linkplay_renderer.rs b/pmocontrol/src/music_renderer/linkplay_renderer.rs index 689cac7e..4ce747b2 100644 --- a/pmocontrol/src/music_renderer/linkplay_renderer.rs +++ b/pmocontrol/src/music_renderer/linkplay_renderer.rs @@ -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 { - self.queue.lock().unwrap().len() - } - - fn track_ids(&self) -> Result, ControlPointError> { - self.queue.lock().unwrap().track_ids() - } - - fn id_to_position(&self, id: u32) -> Result { - self.queue.lock().unwrap().id_to_position(id) - } - - fn position_to_id(&self, id: usize) -> Result { - self.queue.lock().unwrap().position_to_id(id) - } - - fn current_track(&self) -> Result, ControlPointError> { - self.queue.lock().unwrap().current_track() - } - - fn current_index(&self) -> Result, ControlPointError> { - self.queue.lock().unwrap().current_index() - } - - fn queue_snapshot(&self) -> Result { - self.queue.lock().unwrap().queue_snapshot() - } - - fn set_index(&mut self, index: Option) -> Result<(), ControlPointError> { - self.queue.lock().unwrap().set_index(index) - } - - fn replace_queue( - &mut self, - items: Vec, - current_index: Option, - ) -> Result<(), ControlPointError> { - self.queue - .lock() - .unwrap() - .replace_queue(items, current_index) - } - - fn sync_queue( - &mut self, - items: Vec, - _cancel_token: &Arc, - on_ready: Option>, - ) -> Result<(), ControlPointError> { - self.queue - .lock() - .unwrap() - .sync_queue(items, &Arc::new(AtomicBool::new(false)), on_ready) - } - - fn get_item(&self, index: usize) -> Result, 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, - mode: EnqueueMode, - ) -> Result<(), ControlPointError> { - self.queue.lock().unwrap().enqueue_items(items, mode) +impl HasQueue for LinkPlayRenderer { + fn queue(&self) -> &Arc> { + &self.queue } } diff --git a/pmocontrol/src/music_renderer/openhome_renderer.rs b/pmocontrol/src/music_renderer/openhome_renderer.rs index 07e06dc9..d0dde366 100644 --- a/pmocontrol/src/music_renderer/openhome_renderer.rs +++ b/pmocontrol/src/music_renderer/openhome_renderer.rs @@ -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 { - self.queue - .lock() - .map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))? - .len() +impl HasQueue for OpenHomeRenderer { + fn queue(&self) -> &Arc> { + &self.queue } +} - fn track_ids(&self) -> Result, ControlPointError> { - self.queue - .lock() - .map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))? - .track_ids() - } - - fn id_to_position(&self, id: u32) -> Result { - self.queue - .lock() - .map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))? - .id_to_position(id) - } - - fn position_to_id(&self, id: usize) -> Result { - self.queue - .lock() - .map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))? - .position_to_id(id) - } - - fn current_track(&self) -> Result, ControlPointError> { - self.queue - .lock() - .map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))? - .current_track() - } - - fn current_index(&self) -> Result, ControlPointError> { - self.queue - .lock() - .map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))? - .current_index() - } - - fn queue_snapshot(&self) -> Result { - self.queue - .lock() - .map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))? - .queue_snapshot() - } - - fn set_index(&mut self, index: Option) -> 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, current_index: Option, ) -> 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, - cancel_token: &Arc, - on_ready: Option>, - ) -> 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, 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, - mode: EnqueueMode, - ) -> Result<(), ControlPointError> { - self.queue - .lock() - .map_err(|_| ControlPointError::QueueError("Queue mutex poisoned".into()))? - .enqueue_items(items, mode) - } } diff --git a/pmocontrol/src/music_renderer/upnp_renderer.rs b/pmocontrol/src/music_renderer/upnp_renderer.rs index ad57b3cc..bf4098e8 100644 --- a/pmocontrol/src/music_renderer/upnp_renderer.rs +++ b/pmocontrol/src/music_renderer/upnp_renderer.rs @@ -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 { - self.queue.lock().unwrap().len() - } - - fn track_ids(&self) -> Result, ControlPointError> { - self.queue.lock().unwrap().track_ids() - } - - fn id_to_position(&self, id: u32) -> Result { - self.queue.lock().unwrap().id_to_position(id) - } - - fn position_to_id(&self, id: usize) -> Result { - self.queue.lock().unwrap().position_to_id(id) - } - - fn current_track(&self) -> Result, ControlPointError> { - self.queue.lock().unwrap().current_track() - } - - fn current_index(&self) -> Result, ControlPointError> { - self.queue.lock().unwrap().current_index() - } - - fn queue_snapshot(&self) -> Result { - self.queue.lock().unwrap().queue_snapshot() - } - - fn set_index(&mut self, index: Option) -> Result<(), ControlPointError> { - self.queue.lock().unwrap().set_index(index) - } - - fn replace_queue( - &mut self, - items: Vec, - current_index: Option, - ) -> Result<(), ControlPointError> { - self.queue - .lock() - .unwrap() - .replace_queue(items, current_index) - } - - fn sync_queue( - &mut self, - items: Vec, - _cancel_token: &Arc, - on_ready: Option>, - ) -> Result<(), ControlPointError> { - self.queue - .lock() - .unwrap() - .sync_queue(items, &Arc::new(AtomicBool::new(false)), on_ready) - } - - fn get_item(&self, index: usize) -> Result, 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, - mode: EnqueueMode, - ) -> Result<(), ControlPointError> { - self.queue.lock().unwrap().enqueue_items(items, mode) +impl HasQueue for UpnpRenderer { + fn queue(&self) -> &Arc> { + &self.queue } } diff --git a/pmocontrol/src/queue/backend.rs b/pmocontrol/src/queue/backend.rs index c70d343f..2565f0aa 100644 --- a/pmocontrol/src/queue/backend.rs +++ b/pmocontrol/src/queue/backend.rs @@ -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>; +} + +/// Blanket implementation of QueueBackend for types that have a queue. +/// All methods simply delegate to the underlying MusicQueue. +impl QueueBackend for T { + fn len(&self) -> Result { + self.queue().lock().unwrap().len() + } + + fn track_ids(&self) -> Result, ControlPointError> { + self.queue().lock().unwrap().track_ids() + } + + fn id_to_position(&self, id: u32) -> Result { + self.queue().lock().unwrap().id_to_position(id) + } + + fn position_to_id(&self, id: usize) -> Result { + self.queue().lock().unwrap().position_to_id(id) + } + + fn current_track(&self) -> Result, ControlPointError> { + self.queue().lock().unwrap().current_track() + } + + fn current_index(&self) -> Result, ControlPointError> { + self.queue().lock().unwrap().current_index() + } + + fn queue_snapshot(&self) -> Result { + self.queue().lock().unwrap().queue_snapshot() + } + + fn set_index(&mut self, index: Option) -> Result<(), ControlPointError> { + self.queue().lock().unwrap().set_index(index) + } + + fn replace_queue( + &mut self, + items: Vec, + current_index: Option, + ) -> Result<(), ControlPointError> { + self.queue() + .lock() + .unwrap() + .replace_queue(items, current_index) + } + + fn sync_queue( + &mut self, + items: Vec, + cancel_token: &Arc, + on_ready: Option>, + ) -> Result<(), ControlPointError> { + self.queue() + .lock() + .unwrap() + .sync_queue(items, cancel_token, on_ready) + } + + fn get_item(&self, index: usize) -> Result, 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, + mode: EnqueueMode, + ) -> Result<(), ControlPointError> { + self.queue().lock().unwrap().enqueue_items(items, mode) + } +} /// High-level enqueue mode. /// diff --git a/pmocontrol/src/queue/mod.rs b/pmocontrol/src/queue/mod.rs index 2dfd125c..99d77bf0 100644 --- a/pmocontrol/src/queue/mod.rs +++ b/pmocontrol/src/queue/mod.rs @@ -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}; diff --git a/version.txt b/version.txt index c54101be..2fb885a2 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.3.47 +0.3.48