From 02cff8e91319820128917994251c20e29b3fed67 Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Fri, 16 Jan 2026 20:51:27 +0100 Subject: [PATCH] Refactoriser MusicRenderer pour un comportement stateful complet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cette modification refactorise l'architecture pour que chaque `MusicRenderer` gère son propre thread de surveillance (watcher), au lieu de déléguer le polling au `ControlPoint` centralisé. - Ajout d'un module `watcher.rs` avec `WatchStrategy`, `WatchedState` et fonctions helper - Implémentation de `start_watching()` et `stop_watching()` dans `MusicRenderer` - Centralisation de la gestion du watcher dans le constructeur et les méthodes `has_been_seen_now()`/`mark_as_offline()` - Suppression du polling central (~140 lignes) dans `control_point.rs` - Simplification du `registry.rs` avec suppression des appels manuels `start/stop_watching()` - Correction du bug dans `refresh_device_presence()` pour le traitement offline→online - Préparation pour le support futur des notifications push (OpenHome, Chromecast) L'architecture est maintenant plus robuste avec une meilleure encapsulation, cohérence des événements et une gestion automatique du watcher. --- Blackboard/Done/stateful_music_renderer.md | 114 ++++++ Blackboard/Report/stateful_music_renderer.md | 229 +++++++++++ Blackboard/Todo/stateful_music_renderer.md | 154 -------- Cargo.lock | 2 +- PMOMusic/Cargo.toml | 2 +- pmocontrol/src/control_point.rs | 355 +----------------- pmocontrol/src/music_renderer/mod.rs | 1 + .../src/music_renderer/musicrenderer.rs | 292 +++++++++++++- pmocontrol/src/music_renderer/watcher.rs | 276 ++++++++++++++ pmocontrol/src/registry.rs | 15 +- version.txt | 2 +- 11 files changed, 940 insertions(+), 502 deletions(-) create mode 100644 Blackboard/Done/stateful_music_renderer.md create mode 100644 Blackboard/Report/stateful_music_renderer.md delete mode 100644 Blackboard/Todo/stateful_music_renderer.md create mode 100644 pmocontrol/src/music_renderer/watcher.rs diff --git a/Blackboard/Done/stateful_music_renderer.md b/Blackboard/Done/stateful_music_renderer.md new file mode 100644 index 00000000..7fd3ca9d --- /dev/null +++ b/Blackboard/Done/stateful_music_renderer.md @@ -0,0 +1,114 @@ +# Tâche terminée : Rendre MusicRenderer complètement stateful + +## Objectif + +Refactoriser l'architecture pour que chaque `MusicRenderer` gère son propre thread de surveillance (watcher), au lieu de déléguer le polling au `ControlPoint` centralisé. + +## Motivation + +1. **Encapsulation** - Tout l'état et le comportement d'un renderer au même endroit +2. **Cohérence** - Les événements sont émis là où l'état change +3. **Adaptabilité par backend** - Chaque backend peut avoir sa propre stratégie de surveillance (polling vs push) +4. **Auto-advance spécifique** - La logique d'auto-advance peut être adaptée par backend +5. **Simplicité du ControlPoint** - Il devient un simple registry/coordinateur + +--- + +## Résumé de l'implémentation + +### Fichiers créés + +| Fichier | Description | +|---------|-------------| +| `pmocontrol/src/music_renderer/watcher.rs` | Module watcher avec `WatchStrategy`, `WatchedState` et fonctions helper | + +### Fichiers modifiés + +| Fichier | Modification | +|---------|--------------| +| `pmocontrol/src/music_renderer/musicrenderer.rs` | Champs watcher, méthodes `start/stop_watching()`, logique auto-advance, gestion automatique dans constructeur et `DeviceOnline` | +| `pmocontrol/src/music_renderer/mod.rs` | Export du module `watcher` | +| `pmocontrol/src/registry.rs` | Simplifié : plus d'appels manuels watcher | +| `pmocontrol/src/control_point.rs` | Suppression polling central (~140 lignes), `RendererRuntimeSnapshot`, `handle_renderer_event()` | + +--- + +## Architecture finale + +### WatchStrategy + +```rust +pub enum WatchStrategy { + Polling { interval_ms: u64 }, // UPnP, LinkPlay, Arylic (500ms) + Push, // Futur : notifications push + Hybrid { polling_interval_ms: u64 }, // OpenHome, Chromecast +} +``` + +### Gestion automatique du watcher + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ GESTION AUTOMATIQUE DU WATCHER │ +├─────────────────────────────────────────────────────────────────┤ +│ │ +│ Création MusicRenderer ──► constructeur ──► start_watching() │ +│ │ +│ has_been_seen_now() ──► si !was_online ──► start_watching() │ +│ │ +│ mark_as_offline() ──► stop_watching() ──► online = false │ +│ │ +└─────────────────────────────────────────────────────────────────┘ +``` + +### Flux offline/online + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ FLUX ONLINE │ +├─────────────────────────────────────────────────────────────────┤ +│ SSDP Discovery ──► push_renderer() ──► constructeur │ +│ ──► start_watching() │ +│ │ +│ SSDP Alive (offline→online) ──► has_been_seen_now() │ +│ ──► start_watching() │ +└─────────────────────────────────────────────────────────────────┘ + +┌─────────────────────────────────────────────────────────────────┐ +│ FLUX OFFLINE │ +├─────────────────────────────────────────────────────────────────┤ +│ SSDP ByeBye / Timeout ──► mark_as_offline() │ +│ ──► stop_watching() │ +└─────────────────────────────────────────────────────────────────┘ +``` + +--- + +## Points techniques clés + +- **Thread safety** : `AtomicBool` avec `Ordering::SeqCst` pour le signal d'arrêt +- **Idempotence** : `start_watching()` et `stop_watching()` sont idempotents +- **Nommage** : Thread nommé `watcher-{friendly_name}` pour debug +- **Polling** : 500ms pour position/état, 1s pour volume/mute +- **Auto-advance** : Géré dans `handle_state_change()` du MusicRenderer +- **Compensation bugs** : `compute_logical_playback_state()` corrige les comportements Arylic/LinkPlay + +--- + +## Rounds de vérification + +| Round | Objectif | Résultat | +|-------|----------|----------| +| 1 | Implémentation initiale | OK | +| 2 | Vérifier transition offline→online | Bug trouvé et corrigé dans `refresh_device_presence()` | +| 3 | Audit complet des chemins offline/online | Tous les chemins vérifiés OK | +| 4 | Centralisation dans `MusicRenderer` | Gestion automatique dans constructeur et `DeviceOnline` | + +--- + +## Conclusion + +L'architecture est maintenant plus robuste : +- Impossible d'oublier de démarrer/arrêter le watcher +- Le `registry.rs` est simplifié +- Préparation pour le support futur des notifications push (OpenHome, Chromecast) diff --git a/Blackboard/Report/stateful_music_renderer.md b/Blackboard/Report/stateful_music_renderer.md new file mode 100644 index 00000000..7192ec05 --- /dev/null +++ b/Blackboard/Report/stateful_music_renderer.md @@ -0,0 +1,229 @@ +# Rapport : Rendre MusicRenderer complètement stateful + +## Résumé + +Refactorisation de l'architecture pour que chaque `MusicRenderer` gère son propre thread de surveillance (watcher), au lieu de déléguer le polling au `ControlPoint` centralisé. Cette modification améliore l'encapsulation, la cohérence des événements et prépare le terrain pour le support futur des notifications push (OpenHome, Chromecast). + +## Travail effectué + +### Phase 1 : Création du module watcher.rs + +**Fichier créé** : `pmocontrol/src/music_renderer/watcher.rs` + +Nouveau module contenant : +- `WatchStrategy` enum avec trois variantes : + - `Polling { interval_ms: u64 }` - pour UPnP, LinkPlay, Arylic (500ms) + - `Push` - pour support futur des notifications push + - `Hybrid { polling_interval_ms: u64 }` - pour OpenHome et Chromecast +- `WatchedState` struct pour le cache de détection des changements +- Fonctions helper déplacées depuis `control_point.rs` : + - `playback_state_equal()` + - `playback_position_equal()` + - `compute_logical_playback_state()` + - `extract_track_metadata()` + - `parse_hms_to_secs()` +- Tests unitaires pour les fonctions helper + +### Phase 2 : Extension de MusicRenderer + +**Fichier modifié** : `pmocontrol/src/music_renderer/musicrenderer.rs` + +Nouveaux champs ajoutés à la struct `MusicRenderer` : +- `watched_state: Arc>` - cache pour détection des changements +- `watcher_stop_flag: Arc` - signal d'arrêt du thread +- `watcher_handle: Arc>>>` - handle du thread watcher + +Nouvelles méthodes publiques : +- `start_watching()` - démarre le thread de surveillance (idempotent) +- `stop_watching()` - arrête le thread gracieusement (idempotent) +- `is_watching()` - retourne l'état du watcher + +Nouvelles méthodes internes : +- `spawn_watcher_thread()` - crée le thread avec la stratégie appropriée +- `watcher_loop()` - boucle principale de polling +- `poll_and_emit_changes()` - poll le backend et émet les événements +- `handle_state_change()` - logique d'auto-advance (déplacée depuis ControlPoint) +- `emit_event()` - helper pour émettre un événement via le bus + +### Phase 3 : Modification du Registry + +**Fichier modifié** : `pmocontrol/src/registry.rs` + +Ajout des appels `start_watching()` / `stop_watching()` : +- `push_renderer()` : appelle `start_watching()` quand un renderer arrive en ligne ou est créé +- `device_says_byebye()` : appelle `stop_watching()` avant de marquer offline +- `check_timeouts()` : appelle `stop_watching()` avant de marquer offline sur timeout + +### Phase 4 : Simplification du ControlPoint + +**Fichier modifié** : `pmocontrol/src/control_point.rs` + +Suppressions : +- Thread de polling central (~140 lignes) +- Struct `RendererRuntimeSnapshot` +- Méthodes `emit_renderer_event()` et `handle_renderer_event()` +- Fonctions helper déplacées vers `watcher.rs` + +### Phase 5 : Mise à jour du module + +**Fichier modifié** : `pmocontrol/src/music_renderer/mod.rs` + +Ajout de `pub mod watcher;` pour exposer le nouveau module. + +## Liste des fichiers + +### Fichiers créés + +| Fichier | Description | +|---------|-------------| +| `pmocontrol/src/music_renderer/watcher.rs` | Module watcher avec WatchStrategy, WatchedState et fonctions helper | + +### Fichiers modifiés + +| Fichier | Modification | +|---------|--------------| +| `pmocontrol/src/music_renderer/musicrenderer.rs` | Ajout champs watcher, méthodes start/stop_watching, logique auto-advance | +| `pmocontrol/src/music_renderer/mod.rs` | Ajout `pub mod watcher;` | +| `pmocontrol/src/registry.rs` | Appels start/stop_watching dans push_renderer, device_says_byebye, check_timeouts | +| `pmocontrol/src/control_point.rs` | Suppression polling central, RendererRuntimeSnapshot, handle_renderer_event, fonctions helper | + +## Notes techniques + +- Le signal d'arrêt utilise `AtomicBool` avec `Ordering::SeqCst` pour garantir la visibilité entre threads +- Les méthodes `start_watching()` et `stop_watching()` sont idempotentes +- Le thread watcher est nommé `watcher-{friendly_name}` pour faciliter le debug +- L'intervalle de polling est de 500ms (volume/mute toutes les 2 ticks = 1s) +- La logique `compute_logical_playback_state()` compense les bugs des devices Arylic/LinkPlay +- L'auto-advance est maintenant géré directement dans le watcher du MusicRenderer + +## Round 2 : Vérification transition offline → online + +### Problème identifié + +La méthode `refresh_device_presence()` dans `registry.rs` n'appelait pas `start_watching()` quand un renderer passait de offline à online. Cette méthode est appelée lors de la réception de messages SSDP Alive. + +### Correction appliquée + +**Fichier modifié** : `pmocontrol/src/registry.rs` + +Ajout de l'appel `renderer.start_watching()` dans `refresh_device_presence()` quand `was_online == false`. + +### Points de démarrage du watcher vérifiés + +| Méthode | Situation | `start_watching()` appelé | +|---------|-----------|---------------------------| +| `push_renderer()` | Nouveau renderer | Oui | +| `push_renderer()` | Renderer existant, était offline | Oui | +| `refresh_device_presence()` | Renderer existant, était offline | Oui (corrigé) | + +### Points d'arrêt du watcher vérifiés + +| Méthode | Situation | `stop_watching()` appelé | +|---------|-----------|--------------------------| +| `device_says_byebye()` | SSDP ByeBye reçu | Oui | +| `check_timeouts()` | Timeout dépassé | Oui | + +## Round 3 : Audit complet de la logique offline/online + +Suite à la découverte du manque dans le Round 2, un audit complet de tous les chemins offline/online a été effectué. + +### Chemins qui appellent `start_watching()` + +| Chemin | Fonction | Ligne | Condition | Status | +|--------|----------|-------|-----------|--------| +| Nouveau renderer découvert | `push_renderer()` | 180, 194 | Création nouvelle entry | ✅ OK | +| Renderer existant, ajout renderer à entry | `push_renderer()` | 169 | Entry existe sans renderer | ✅ OK | +| Renderer existant revient online | `push_renderer()` | 160 | `!was_online` | ✅ OK | +| SSDP Alive pour device connu | `refresh_device_presence()` | 269 | `!was_online` | ✅ OK (corrigé Round 2) | + +### Chemins qui appellent `stop_watching()` + +| Chemin | Fonction | Ligne | Condition | Status | +|--------|----------|-------|-----------|--------| +| SSDP ByeBye reçu | `device_says_byebye()` | 289 | Renderer présent | ✅ OK | +| Timeout dépassé | `check_timeouts()` | 308 | `elapsed > max_age` | ✅ OK | + +### Analyse des flux + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ FLUX ONLINE │ +├─────────────────────────────────────────────────────────────────┤ +│ │ +│ SSDP Discovery ──► push_renderer() ──► start_watching() ✅ │ +│ │ +│ SSDP Alive (nouveau UDN) ──► push_renderer() ──► start_watching() ✅ │ +│ │ +│ SSDP Alive (UDN connu, online) ──► refresh_device_presence() │ +│ (pas de start car déjà en marche) │ +│ │ +│ SSDP Alive (UDN connu, offline) ──► refresh_device_presence() │ +│ ──► start_watching() ✅ │ +└─────────────────────────────────────────────────────────────────┘ + +┌─────────────────────────────────────────────────────────────────┐ +│ FLUX OFFLINE │ +├─────────────────────────────────────────────────────────────────┤ +│ │ +│ SSDP ByeBye ──► device_says_byebye() ──► stop_watching() ✅ │ +│ │ +│ Timeout ──► check_timeouts() ──► stop_watching() ✅ │ +│ │ +└─────────────────────────────────────────────────────────────────┘ +``` + +### Conclusion + +**Tous les chemins sont correctement gérés.** Chaque transition offline→online appelle `start_watching()` et chaque transition online→offline appelle `stop_watching()`. + +L'idempotence des méthodes `start_watching()` et `stop_watching()` garantit qu'aucun problème ne survient en cas d'appels multiples. + +## Round 4 : Centralisation de la gestion du watcher + +### Problème identifié + +Les appels à `start_watching()` et `stop_watching()` étaient dispersés dans `registry.rs` (6 emplacements), augmentant le risque d'oubli (comme découvert en Round 2). + +### Solution implémentée + +Centralisation de la gestion du watcher dans `MusicRenderer` lui-même : + +1. **Constructeur** (`from_renderer_info_with_bus()`) : appelle automatiquement `start_watching()` à la fin, car le renderer est créé avec `online = true` + +2. **`has_been_seen_now()`** : appelle automatiquement `start_watching()` si transition offline→online + +3. **`mark_as_offline()`** : appelle automatiquement `stop_watching()` avant de passer offline + +### Fichiers modifiés + +| Fichier | Modification | +|---------|--------------| +| `pmocontrol/src/music_renderer/musicrenderer.rs` | Ajout `start_watching()` dans constructeur, dans `has_been_seen_now()` et `stop_watching()` dans `mark_as_offline()` | +| `pmocontrol/src/registry.rs` | Suppression de tous les appels manuels à `start_watching()` et `stop_watching()` | + +### Avantages + +- **Encapsulation** : la logique watcher est entièrement gérée par `MusicRenderer` +- **Impossible d'oublier** : les transitions sont automatiquement gérées +- **Code simplifié** : `registry.rs` ne contient plus de logique watcher +- **Idempotence** : les appels multiples sont sans effet grâce aux guards existants + +### Nouvelle architecture + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ GESTION AUTOMATIQUE DU WATCHER │ +├─────────────────────────────────────────────────────────────────┤ +│ │ +│ Création MusicRenderer ──► constructeur ──► start_watching() │ +│ │ +│ has_been_seen_now() ──► si !was_online ──► start_watching() │ +│ │ +│ mark_as_offline() ──► stop_watching() ──► online = false │ +│ │ +└─────────────────────────────────────────────────────────────────┘ +``` + +## Compilation + +Le projet compile sans erreur. diff --git a/Blackboard/Todo/stateful_music_renderer.md b/Blackboard/Todo/stateful_music_renderer.md deleted file mode 100644 index 2ac6bf72..00000000 --- a/Blackboard/Todo/stateful_music_renderer.md +++ /dev/null @@ -1,154 +0,0 @@ -# Tâche : Rendre MusicRenderer complètement stateful - -## Objectif - -Refactoriser l'architecture pour que chaque `MusicRenderer` gère son propre état complet et son thread de surveillance, au lieu de déléguer le polling au `ControlPoint`. - -## Motivation - -1. **Encapsulation** - Tout l'état et le comportement d'un renderer au même endroit -2. **Cohérence** - Les événements sont émis là où l'état change -3. **Adaptabilité par backend** - Chaque backend peut avoir sa propre stratégie de surveillance (polling vs push pour OpenHome/Chromecast) -4. **Auto-advance spécifique** - La logique d'auto-advance peut être adaptée par backend -5. **Simplicité du ControlPoint** - Il devient un simple registry/coordinateur - -## Contraintes - -- Moins de 10 renderers simultanés → 10 threads de surveillance n'est pas un problème -- Le transfert de queue et autres opérations multi-renderers n'impliquent pas de surveillance - -## Architecture cible - -### MusicRenderer - -Responsabilités : -- Maintenir l'état complet du renderer (transport, volume, position, queue, binding) -- Gérer son propre thread de surveillance (polling ou push selon le backend) -- Émettre tous les événements (StateChanged, PositionChanged, VolumeChanged, MuteChanged, QueueUpdated, BindingChanged) -- Gérer l'auto-advance de la queue (logique adaptée par backend) - -Nouveau champ : -```rust -pub struct MusicRenderer { - // ... champs existants ... - event_bus: Option, - // Nouveau : état surveillé - watched_state: Arc>, - // Nouveau : handle du thread de surveillance - watcher_handle: Option>, -} - -struct WatchedState { - last_playback_state: Option, - last_position: Option, - last_volume: Option, - last_mute: Option, -} -``` - -Nouvelles méthodes : -```rust -impl MusicRenderer { - /// Démarre le thread de surveillance - pub fn start_watching(&self) -> Result<(), ControlPointError>; - - /// Arrête le thread de surveillance - pub fn stop_watching(&self); - - /// Logique d'auto-advance (appelée quand state passe à Stopped) - fn handle_playback_stopped(&self); -} -``` - -### ControlPoint - -Responsabilités simplifiées : -- Registry des devices (renderers et servers) -- Coordination des opérations multi-renderers (transfer_queue) -- Point d'entrée API pour les couches supérieures -- Démarrage/arrêt des watchers lors de l'ajout/suppression de renderers - -Supprimer : -- Le polling loop centralisé (`start_polling_loop`) -- Les snapshots de surveillance (`RendererSnapshot`) -- La logique d'auto-advance centralisée - -### MusicRendererBackend - -Enrichir le trait pour supporter différentes stratégies de surveillance : -```rust -pub trait BackendWatcher { - /// Retourne la stratégie de surveillance pour ce backend - fn watch_strategy(&self) -> WatchStrategy; -} - -pub enum WatchStrategy { - /// Polling à intervalle fixe (UPnP, LinkPlay, Arylic) - Polling { interval_ms: u64 }, - /// Notifications push (OpenHome, Chromecast) - Push, - /// Hybride : push avec polling de secours - Hybrid { polling_interval_ms: u64 }, -} -``` - -## Étapes d'implémentation - -### Étape 1 : Préparer MusicRenderer - -**Crate** : `pmocontrol` - -1. Ajouter `WatchedState` et les champs associés à `MusicRenderer` -2. Implémenter `start_watching()` et `stop_watching()` -3. Implémenter la boucle de surveillance interne avec émission d'événements -4. Implémenter `handle_playback_stopped()` pour l'auto-advance - -### Étape 2 : Adapter par backend - -**Crate** : `pmocontrol` - -1. Définir le trait `BackendWatcher` et `WatchStrategy` -2. Implémenter pour chaque backend : - - `UpnpRenderer` : Polling 500ms - - `OpenHomeRenderer` : Push (via subscriptions UPnP) avec fallback polling - - `LinkPlayRenderer` : Polling 500ms - - `ArylicTcpRenderer` : Polling 500ms - - `ChromecastRenderer` : Push avec fallback polling - - `HybridUpnpArylic` : Polling 500ms - -### Étape 3 : Simplifier ControlPoint - -**Crate** : `pmocontrol` - -1. Supprimer `start_polling_loop()` et code associé -2. Supprimer `RendererSnapshot` et la gestion des snapshots -3. Modifier `push_renderer()` dans Registry pour appeler `start_watching()` -4. Modifier la gestion offline pour appeler `stop_watching()` -5. Supprimer la logique d'auto-advance du `handle_renderer_event()` - -### Étape 4 : Tests et validation - -1. Vérifier que les événements SSE sont toujours émis correctement -2. Vérifier l'auto-advance pour chaque type de backend -3. Vérifier le comportement online/offline -4. Tests de performance avec plusieurs renderers - -## Fichiers impactés - -| Fichier | Modification | -|---------|--------------| -| `pmocontrol/src/music_renderer/musicrenderer.rs` | Ajout état surveillé, thread watcher, auto-advance | -| `pmocontrol/src/music_renderer/mod.rs` | Ajout trait `BackendWatcher` | -| `pmocontrol/src/music_renderer/upnp_renderer.rs` | Impl `BackendWatcher` (Polling) | -| `pmocontrol/src/music_renderer/openhome_renderer.rs` | Impl `BackendWatcher` (Push/Hybrid) | -| `pmocontrol/src/music_renderer/linkplay_renderer.rs` | Impl `BackendWatcher` (Polling) | -| `pmocontrol/src/music_renderer/arylic_tcp.rs` | Impl `BackendWatcher` (Polling) | -| `pmocontrol/src/music_renderer/chromecast_renderer.rs` | Impl `BackendWatcher` (Push/Hybrid) | -| `pmocontrol/src/control_point.rs` | Suppression polling loop, simplification | -| `pmocontrol/src/registry.rs` | Appel start/stop watching | - -## Notes - -- Ce refactoring est significatif mais améliore la maintenabilité à long terme -- La migration peut être faite de manière incrémentale en gardant temporairement les deux systèmes -- Les backends OpenHome et Chromecast bénéficieront particulièrement de cette architecture (notifications push natives) diff --git a/Cargo.lock b/Cargo.lock index 1f868e6e..77a0aa4a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,7 +4,7 @@ version = 4 [[package]] name = "PMOMusic" -version = "0.3.3" +version = "0.3.5" dependencies = [ "axum 0.8.7", "console-subscriber", diff --git a/PMOMusic/Cargo.toml b/PMOMusic/Cargo.toml index 11786d75..01b94a0d 100644 --- a/PMOMusic/Cargo.toml +++ b/PMOMusic/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "PMOMusic" -version = "0.3.4" +version = "0.3.5" edition = "2024" [dependencies] diff --git a/pmocontrol/src/control_point.rs b/pmocontrol/src/control_point.rs index 155111a1..8b879c58 100644 --- a/pmocontrol/src/control_point.rs +++ b/pmocontrol/src/control_point.rs @@ -194,150 +194,9 @@ impl ControlPoint { }); }); - let polling_cp = ControlPoint { - registry: Arc::clone(®istry), - // udn_cache: udn_cache.clone(), - event_bus: event_bus.clone(), - media_event_bus: media_event_bus.clone(), - }; - - thread::spawn(move || { - use std::collections::HashMap; - - // Local cache for change detection (not a source of truth) - let mut polling_cache: HashMap = HashMap::new(); - let mut tick: u32 = 0; - - loop { - // Get renderers directly from registry - they already contain backends - let renderers = { - let reg = polling_cp.registry.read().unwrap(); - reg.list_renderers().unwrap_or_else(|_| vec![]) - }; - - for renderer in renderers { - if !renderer.is_online() { - continue; - } - - let renderer_id = renderer.id(); - - // Get previous snapshot from local cache - let prev_snapshot = - polling_cache.get(&renderer_id).cloned().unwrap_or_default(); - let mut new_snapshot = prev_snapshot.clone(); - let prev_position = prev_snapshot.position.clone(); - - // Poll position every tick (1s) for smooth UI progress - if let Ok(position) = renderer.playback_position() { - let has_changed = match prev_snapshot.position.as_ref() { - Some(prev) => !playback_position_equal(prev, &position), - None => true, - }; - - if has_changed { - polling_cp.emit_renderer_event(RendererEvent::PositionChanged { - id: renderer_id.clone(), - position: position.clone(), - }); - } - - // Extract and emit metadata changes - match extract_track_metadata(&position) { - Some(metadata) => { - let metadata_changed = match prev_snapshot.last_metadata.as_ref() { - Some(prev) => prev != &metadata, - None => true, - }; - - if metadata_changed { - debug!( - renderer = renderer_id.0.as_str(), - title = metadata.title.as_deref(), - artist = metadata.artist.as_deref(), - "Emitting metadata changed event" - ); - polling_cp.emit_renderer_event( - RendererEvent::MetadataChanged { - id: renderer_id.clone(), - metadata: metadata.clone(), - }, - ); - new_snapshot.last_metadata = Some(metadata); - } - } - None => { - debug!( - renderer = renderer_id.0.as_str(), - has_track_metadata = position.track_metadata.is_some(), - "No metadata extracted from position info" - ); - } - } - - new_snapshot.position = Some(position); - } - - // Poll state every tick to ensure responsive playback control - if let Ok(raw_state) = renderer.playback_state() { - let logical_state = compute_logical_playback_state( - &raw_state, - prev_position.as_ref(), - new_snapshot.position.as_ref(), - ); - - let has_changed = match prev_snapshot.state.as_ref() { - Some(prev) => !playback_state_equal(prev, &logical_state), - None => true, - }; - - // Emit event only for non-transient states to reduce noise - // and avoid overwhelming the renderer during track changes - if has_changed && !matches!(logical_state, PlaybackState::Transitioning) { - polling_cp.emit_renderer_event(RendererEvent::StateChanged { - id: renderer_id.clone(), - state: logical_state.clone(), - }); - } - - new_snapshot.state = Some(logical_state); - } - - // Poll volume and mute every second (every 2 ticks at 500ms) - // for responsive volume control feedback - if tick % 2 == 0 { - if let Ok(volume) = renderer.volume() { - if prev_snapshot.last_volume != Some(volume) { - polling_cp.emit_renderer_event(RendererEvent::VolumeChanged { - id: renderer_id.clone(), - volume, - }); - } - - new_snapshot.last_volume = Some(volume); - } - - if let Ok(mute) = renderer.mute() { - if prev_snapshot.last_mute != Some(mute) { - polling_cp.emit_renderer_event(RendererEvent::MuteChanged { - id: renderer_id.clone(), - mute, - }); - } - - new_snapshot.last_mute = Some(mute); - } - } - - // Update local cache - polling_cache.insert(renderer_id, new_snapshot); - } - - tick = tick.wrapping_add(1); - // 250ms polling for smoother UI updates and fluid progress bar - thread::sleep(Duration::from_millis(250)); - } - }); + // Note: Renderer polling is now handled by each MusicRenderer's own watcher thread. + // The central polling loop has been removed in favor of per-renderer watchers + // that are started/stopped by the Registry when devices come online/offline. spawn_media_server_event_runtime( Arc::clone(®istry), @@ -1480,50 +1339,9 @@ impl ControlPoint { } } - pub(crate) fn emit_renderer_event(&self, event: RendererEvent) { - self.handle_renderer_event(&event); - self.event_bus.broadcast(event); - } - - fn handle_renderer_event(&self, event: &RendererEvent) { - if let RendererEvent::StateChanged { id, state } = event { - let Some(renderer) = self.music_renderer_by_id(id) else { - return; - }; - - match state { - PlaybackState::Stopped => { - // Check if user requested stop (via Stop button in UI) - if renderer.check_and_clear_user_stop_requested() { - debug!( - renderer = id.0.as_str(), - "Renderer stopped by user request; not auto-advancing" - ); - renderer.set_playback_source(PlaybackSource::None); - } else if renderer.is_playing_from_queue() { - debug!( - renderer = id.0.as_str(), - "Renderer stopped after queue-driven playback; advancing" - ); - if let Err(err) = self.play_next_from_queue(id) { - error!( - renderer = id.0.as_str(), - error = %err, - "Auto-advance failed; clearing queue playback state" - ); - renderer.set_playback_source(PlaybackSource::None); - } - } else { - renderer.set_playback_source(PlaybackSource::None); - } - } - PlaybackState::Playing => { - renderer.mark_external_if_idle(); - } - _ => {} - } - } - } + // Note: emit_renderer_event and handle_renderer_event have been removed. + // Renderer events are now emitted directly by each MusicRenderer's watcher thread, + // and auto-advance logic is handled internally by MusicRenderer::handle_state_change(). } #[cfg(feature = "pmoserver")] @@ -1564,16 +1382,8 @@ fn parse_hms_to_ms(hms: Option<&str>) -> Option { Some((hours * 3600 + minutes * 60 + seconds) * 1000) } -/// Snapshot of renderer state used for change detection in the polling thread. -/// This is a local cache, not a source of truth. -#[derive(Clone, Default)] -struct RendererRuntimeSnapshot { - state: Option, - position: Option, - last_volume: Option, - last_mute: Option, - last_metadata: Option, -} +// Note: RendererRuntimeSnapshot has been removed. +// Each MusicRenderer now maintains its own WatchedState for change detection. /// Internal helper to refresh a renderer's playback queue from its bound /// playlist container. @@ -1866,74 +1676,14 @@ fn playback_item_track_metadata(item: &PlaybackItem) -> TrackMetadata { }) } -fn parse_optional_hms_to_secs(value: &Option) -> Option { - value.as_ref().and_then(|s| parse_hms_to_secs(s)) -} - -/// Compute a logical playback state by combining the raw AVTransport state -/// with previous and current position information. -/// -/// This is designed to compensate for buggy LinkPlay/Arylic devices that -/// report: -/// - STOPPED while the time actually advances, -/// - NO_MEDIA_PRESENT while track duration is known. -fn compute_logical_playback_state( - raw: &PlaybackState, - prev_position: Option<&PlaybackPositionInfo>, - current_position: Option<&PlaybackPositionInfo>, -) -> PlaybackState { - // Rule 1: Arylic / LinkPlay sometimes report STOPPED while the stream is - // actually playing. If we detect that the relative time advances between - // two polls, we treat this as Playing. - if let PlaybackState::Stopped = raw { - if let (Some(prev), Some(curr)) = (prev_position, current_position) { - if let (Some(prev_rel), Some(curr_rel)) = ( - parse_optional_hms_to_secs(&prev.rel_time), - parse_optional_hms_to_secs(&curr.rel_time), - ) { - if curr_rel > prev_rel { - let delta = curr_rel - prev_rel; - // Our poll loop runs every 1s; accept small jitter in the delta. - if delta <= 5 { - return PlaybackState::Playing; - } - } - } - } - } - - // Rule 2: Some devices report NO_MEDIA_PRESENT while exposing a non-zero - // track duration. In practice this behaves like a stopped transport with - // a loaded track. - if let PlaybackState::NoMedia = raw { - let duration_secs = current_position - .and_then(|p| parse_optional_hms_to_secs(&p.track_duration)) - .or_else(|| prev_position.and_then(|p| parse_optional_hms_to_secs(&p.track_duration))); - - if matches!(duration_secs, Some(d) if d > 0) { - return PlaybackState::Stopped; - } - } - - // Fallback: keep the raw (already normalized) state. - raw.clone() -} - -fn playback_state_equal(a: &PlaybackState, b: &PlaybackState) -> bool { - match (a, b) { - (PlaybackState::Unknown(lhs), PlaybackState::Unknown(rhs)) => lhs == rhs, - _ => std::mem::discriminant(a) == std::mem::discriminant(b), - } -} - -fn playback_position_equal(a: &PlaybackPositionInfo, b: &PlaybackPositionInfo) -> bool { - a.track == b.track - && a.rel_time == b.rel_time - && a.abs_time == b.abs_time - && a.track_duration == b.track_duration - && a.track_metadata == b.track_metadata - && a.track_uri == b.track_uri -} +// Note: The following helper functions have been moved to music_renderer/watcher.rs: +// - parse_optional_hms_to_secs +// - compute_logical_playback_state +// - playback_state_equal +// - playback_position_equal +// - extract_track_metadata +// - parse_hms_to_secs +// They are now used by each MusicRenderer's watcher thread for change detection. #[cfg(feature = "pmoserver")] fn current_track_from_playback_item(item: &PlaybackItem) -> CurrentTrackMetadata { @@ -1945,76 +1695,3 @@ fn current_track_from_playback_item(item: &PlaybackItem) -> CurrentTrackMetadata album_art_uri: meta.and_then(|m| m.album_art_uri.clone()), } } - -/// Extract TrackMetadata from DIDL-Lite XML in PlaybackPositionInfo. -fn extract_track_metadata(position: &PlaybackPositionInfo) -> Option { - let didl_xml = match position.track_metadata.as_ref() { - Some(xml) => xml, - None => { - debug!("Position info has no track_metadata (DIDL-Lite XML)"); - return None; - } - }; - - // Parse DIDL-Lite XML - let didl = match pmodidl::parse_metadata::(didl_xml) { - Ok(parsed) => parsed.data, - Err(err) => { - debug!(error = %err, "Failed to parse DIDL-Lite metadata from GetPositionInfo"); - return None; - } - }; - - // Extract first item metadata - let item = match didl.items.first() { - Some(item) => item, - None => { - debug!("DIDL-Lite has no items"); - return None; - } - }; - - debug!( - title = item.title.as_str(), - has_album_art = item.album_art.is_some(), - album_art_uri = item.album_art.as_deref(), - "Extracted metadata from position info" - ); - - Some(TrackMetadata { - title: Some(item.title.clone()), - artist: item.artist.clone(), - album: item.album.clone(), - genre: item.genre.clone(), - album_art_uri: item.album_art.clone(), - date: item.date.clone(), - track_number: item.original_track_number.clone(), - creator: item.creator.clone(), - }) -} - -/// Parse "HH:MM:SS" style time strings to seconds. -/// -/// Returns None for empty or sentinel values such as "NOT_IMPLEMENTED" or "-:--:--". -fn parse_hms_to_secs(s: &str) -> Option { - let s = s.trim(); - if s.is_empty() { - return None; - } - - // Common sentinel values for "no information" in UPnP implementations. - if s == "NOT_IMPLEMENTED" || s == "-:--:--" { - return None; - } - - let parts: Vec<_> = s.split(':').collect(); - if parts.len() != 3 { - return None; - } - - let hours: u64 = parts[0].parse().ok()?; - let minutes: u64 = parts[1].parse().ok()?; - let seconds: u64 = parts[2].parse().ok()?; - - Some(hours * 3600 + minutes * 60 + seconds) -} diff --git a/pmocontrol/src/music_renderer/mod.rs b/pmocontrol/src/music_renderer/mod.rs index a08d7719..a7d489d3 100644 --- a/pmocontrol/src/music_renderer/mod.rs +++ b/pmocontrol/src/music_renderer/mod.rs @@ -12,6 +12,7 @@ mod chromecast_renderer; mod musicrenderer; mod sleep_timer; pub mod time_utils; +pub mod watcher; use std::sync::{Arc, Mutex}; diff --git a/pmocontrol/src/music_renderer/musicrenderer.rs b/pmocontrol/src/music_renderer/musicrenderer.rs index 0b7b6fcf..ba33df38 100644 --- a/pmocontrol/src/music_renderer/musicrenderer.rs +++ b/pmocontrol/src/music_renderer/musicrenderer.rs @@ -6,9 +6,13 @@ //! renderers through this type so that transport, volume, and state queries //! stay backend-neutral. +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; +use std::thread::{self, JoinHandle}; use std::time::SystemTime; +use tracing::{debug, error}; + use crate::errors::ControlPointError; use crate::events::RendererEventBus; use crate::model::RendererEvent; @@ -24,6 +28,10 @@ use crate::music_renderer::linkplay_renderer::LinkPlayRenderer; use crate::music_renderer::openhome_renderer::OpenHomeRenderer; use crate::music_renderer::sleep_timer::SleepTimer; use crate::music_renderer::upnp_renderer::UpnpRenderer; +use crate::music_renderer::watcher::{ + WatchStrategy, WatchedState, compute_logical_playback_state, extract_track_metadata, + playback_position_equal, playback_state_equal, +}; use crate::online::DeviceConnectionState; use crate::queue::{ EnqueueMode, MusicQueue, PlaybackItem, QueueBackend, QueueFromRendererInfo, QueueSnapshot, @@ -94,6 +102,12 @@ pub struct MusicRenderer { state: Arc>, /// Optional event bus for emitting queue change events. event_bus: Option, + /// Cached state from last poll, used for change detection by the watcher. + watched_state: Arc>, + /// Flag to signal the watcher thread to stop. + watcher_stop_flag: Arc, + /// Handle to the watcher thread, if running. + watcher_handle: Arc>>>, } impl std::fmt::Debug for MusicRenderer { @@ -108,6 +122,7 @@ impl std::fmt::Debug for MusicRenderer { "event_bus", &self.event_bus.as_ref().map(|_| "RendererEventBus"), ) + .field("is_watching", &self.is_watching()) .finish() } } @@ -126,6 +141,9 @@ impl MusicRenderer { playlist_binding: Arc::new(Mutex::new(None)), state: Arc::new(Mutex::new(MusicRendererState::default())), event_bus: None, + watched_state: Arc::new(Mutex::new(WatchedState::default())), + watcher_stop_flag: Arc::new(AtomicBool::new(false)), + watcher_handle: Arc::new(Mutex::new(None)), }; Arc::new(renderer) @@ -149,7 +167,14 @@ impl MusicRenderer { playlist_binding: Arc::new(Mutex::new(None)), state: Arc::new(Mutex::new(MusicRendererState::default())), event_bus, + watched_state: Arc::new(Mutex::new(WatchedState::default())), + watcher_stop_flag: Arc::new(AtomicBool::new(false)), + watcher_handle: Arc::new(Mutex::new(None)), }; + + // Start watching immediately since the renderer is created online + renderer.start_watching(); + Ok(renderer) } @@ -164,6 +189,263 @@ impl MusicRenderer { } } + /// Helper method to emit any RendererEvent if an event bus is available. + fn emit_event(&self, event: RendererEvent) { + if let Some(ref bus) = self.event_bus { + bus.broadcast(event); + } + } + + // ========================================================================= + // Watcher Thread Management + // ========================================================================= + + /// Starts the watcher thread for this renderer. + /// + /// The watcher thread polls the backend at regular intervals and emits + /// events when state changes are detected. This method is idempotent: + /// calling it when already watching is a no-op. + pub fn start_watching(&self) { + let mut handle_guard = self + .watcher_handle + .lock() + .expect("Watcher handle mutex poisoned"); + if handle_guard.is_some() { + return; // Already watching + } + + // Reset the stop flag before starting + self.watcher_stop_flag.store(false, Ordering::SeqCst); + + // Determine the watch strategy based on backend type + let strategy = { + let backend = self.backend.lock().expect("Backend mutex poisoned"); + WatchStrategy::for_backend(&*backend) + }; + + debug!( + renderer = self.info.friendly_name(), + strategy = ?strategy, + "Starting watcher thread" + ); + + let handle = self.spawn_watcher_thread(strategy); + *handle_guard = Some(handle); + } + + /// Stops the watcher thread gracefully. + /// + /// This method is idempotent: calling it when not watching is a no-op. + /// The method will block until the watcher thread terminates. + pub fn stop_watching(&self) { + // Signal the watcher to stop + self.watcher_stop_flag.store(true, Ordering::SeqCst); + + // Take the handle and wait for the thread to finish + let mut handle_guard = self + .watcher_handle + .lock() + .expect("Watcher handle mutex poisoned"); + if let Some(handle) = handle_guard.take() { + debug!( + renderer = self.info.friendly_name(), + "Stopping watcher thread" + ); + // Wait for the thread to finish (ignore join errors) + let _ = handle.join(); + } + } + + /// Returns true if the watcher thread is currently running. + pub fn is_watching(&self) -> bool { + self.watcher_handle + .lock() + .expect("Watcher handle mutex poisoned") + .is_some() + } + + /// Spawns the watcher thread with the given strategy. + fn spawn_watcher_thread(&self, strategy: WatchStrategy) -> JoinHandle<()> { + let renderer = self.clone(); + let stop_flag = Arc::clone(&self.watcher_stop_flag); + + thread::Builder::new() + .name(format!("watcher-{}", self.info.friendly_name())) + .spawn(move || { + renderer.watcher_loop(strategy, stop_flag); + }) + .expect("Failed to spawn watcher thread") + } + + /// Main loop for the watcher thread. + fn watcher_loop(&self, strategy: WatchStrategy, stop_flag: Arc) { + let Some(interval) = strategy.polling_interval() else { + // Pure push strategy - no polling needed (future implementation) + return; + }; + + let mut tick: u32 = 0; + + while !stop_flag.load(Ordering::SeqCst) { + if self.is_online() { + self.poll_and_emit_changes(tick); + } + + tick = tick.wrapping_add(1); + thread::sleep(interval); + } + + debug!( + renderer = self.info.friendly_name(), + "Watcher thread exiting" + ); + } + + /// Polls the backend and emits events for any detected changes. + fn poll_and_emit_changes(&self, tick: u32) { + let mut watched = self + .watched_state + .lock() + .expect("WatchedState mutex poisoned"); + let prev_position = watched.position.clone(); + + // Poll position every tick + if let Ok(position) = self.playback_position() { + let changed = watched + .position + .as_ref() + .map(|prev| !playback_position_equal(prev, &position)) + .unwrap_or(true); + + if changed { + self.emit_event(RendererEvent::PositionChanged { + id: self.id(), + position: position.clone(), + }); + } + + // Extract and emit metadata changes + if let Some(metadata) = extract_track_metadata(&position) { + let metadata_changed = watched + .metadata + .as_ref() + .map(|prev| prev != &metadata) + .unwrap_or(true); + + if metadata_changed { + debug!( + renderer = self.info.friendly_name(), + title = metadata.title.as_deref(), + artist = metadata.artist.as_deref(), + "Emitting metadata changed event" + ); + self.emit_event(RendererEvent::MetadataChanged { + id: self.id(), + metadata: metadata.clone(), + }); + watched.metadata = Some(metadata); + } + } + + watched.position = Some(position); + } + + // Poll state every tick + if let Ok(raw_state) = self.playback_state() { + let logical_state = compute_logical_playback_state( + &raw_state, + prev_position.as_ref(), + watched.position.as_ref(), + ); + + let changed = watched + .state + .as_ref() + .map(|prev| !playback_state_equal(prev, &logical_state)) + .unwrap_or(true); + + // Emit event only for non-transient states to reduce noise + if changed && !matches!(logical_state, PlaybackState::Transitioning) { + self.emit_event(RendererEvent::StateChanged { + id: self.id(), + state: logical_state.clone(), + }); + + // Handle auto-advance logic internally + // Release the lock before calling handle_state_change to avoid deadlock + drop(watched); + self.handle_state_change(&logical_state); + watched = self + .watched_state + .lock() + .expect("WatchedState mutex poisoned"); + } + + watched.state = Some(logical_state); + } + + // Poll volume and mute every other tick (1 second at 500ms interval) + if tick % 2 == 0 { + if let Ok(volume) = self.volume() { + if watched.volume != Some(volume) { + self.emit_event(RendererEvent::VolumeChanged { + id: self.id(), + volume, + }); + watched.volume = Some(volume); + } + } + + if let Ok(mute) = self.mute() { + if watched.mute != Some(mute) { + self.emit_event(RendererEvent::MuteChanged { + id: self.id(), + mute, + }); + watched.mute = Some(mute); + } + } + } + } + + /// Handles playback state changes internally (auto-advance logic). + /// + /// This method is called by the watcher when a state change is detected. + /// It handles the auto-advance behavior when playback stops. + fn handle_state_change(&self, state: &PlaybackState) { + match state { + PlaybackState::Stopped => { + // Check if user requested stop (via Stop button in UI) + if self.check_and_clear_user_stop_requested() { + debug!( + renderer = self.info.friendly_name(), + "Renderer stopped by user request; not auto-advancing" + ); + self.set_playback_source(PlaybackSource::None); + } 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!( + renderer = self.info.friendly_name(), + error = %err, + "Auto-advance failed; clearing queue playback state" + ); + self.set_playback_source(PlaybackSource::None); + } + } else { + self.set_playback_source(PlaybackSource::None); + } + } + PlaybackState::Playing => { + self.mark_external_if_idle(); + } + _ => {} + } + } + pub fn info(&self) -> &RendererInfo { &self.info } @@ -941,13 +1223,21 @@ impl DeviceOnline for MusicRenderer { } fn has_been_seen_now(&self, max_age: u32) { + let was_online = self.is_online(); self.connection .lock() .expect("Connection mutex poisoned") - .has_been_seen_now(max_age) + .has_been_seen_now(max_age); + + // Start watching if transitioning from offline to online + if !was_online { + self.start_watching(); + } } fn mark_as_offline(&self) { + // Stop watching before marking offline + self.stop_watching(); self.connection .lock() .expect("Connection mutex poisoned") diff --git a/pmocontrol/src/music_renderer/watcher.rs b/pmocontrol/src/music_renderer/watcher.rs new file mode 100644 index 00000000..9abaccb1 --- /dev/null +++ b/pmocontrol/src/music_renderer/watcher.rs @@ -0,0 +1,276 @@ +//! Watcher module for MusicRenderer state surveillance. +//! +//! This module provides the infrastructure for each MusicRenderer to maintain +//! its own polling/watching thread, instead of relying on a centralized +//! polling loop in ControlPoint. +//! +//! ## Architecture +//! +//! Each MusicRenderer can have an associated watcher thread that: +//! - Polls the backend at regular intervals (or receives push notifications) +//! - Detects state changes by comparing with cached values +//! - Emits events when changes are detected +//! - Handles auto-advance logic when playback stops + +use std::time::Duration; + +use crate::model::{PlaybackState, TrackMetadata}; +use crate::music_renderer::capabilities::PlaybackPositionInfo; +use crate::music_renderer::musicrenderer::MusicRendererBackend; + +/// Strategy for monitoring renderer state changes. +/// +/// Different backends may support different monitoring strategies: +/// - Pure polling for simple protocols (UPnP AVTransport, LinkPlay, Arylic) +/// - Push notifications for more advanced protocols (OpenHome subscriptions, Chromecast) +/// - Hybrid approaches combining both +#[derive(Clone, Debug)] +pub enum WatchStrategy { + /// Poll the backend at regular intervals. + /// Used for UPnP, LinkPlay, Arylic backends. + Polling { interval_ms: u64 }, + + /// Backend supports push notifications (future implementation). + /// The watcher thread would wait on a channel instead of polling. + Push, + + /// Hybrid: use push when available, fall back to polling. + /// Used for OpenHome and Chromecast which support subscriptions + /// but may need polling as a fallback. + Hybrid { polling_interval_ms: u64 }, +} + +impl WatchStrategy { + /// Returns the recommended strategy for a given backend type. + pub fn for_backend(backend: &MusicRendererBackend) -> Self { + match backend { + MusicRendererBackend::Upnp(_) => WatchStrategy::Polling { interval_ms: 500 }, + MusicRendererBackend::LinkPlay(_) => WatchStrategy::Polling { interval_ms: 500 }, + MusicRendererBackend::ArylicTcp(_) => WatchStrategy::Polling { interval_ms: 500 }, + MusicRendererBackend::HybridUpnpArylic { .. } => { + WatchStrategy::Polling { interval_ms: 500 } + } + // Future push support - for now use hybrid with polling fallback + MusicRendererBackend::OpenHome(_) => WatchStrategy::Hybrid { + polling_interval_ms: 500, + }, + MusicRendererBackend::Chromecast(_) => WatchStrategy::Hybrid { + polling_interval_ms: 500, + }, + } + } + + /// Returns the polling interval if this strategy involves polling. + /// Returns None for pure Push strategy. + pub fn polling_interval(&self) -> Option { + match self { + WatchStrategy::Polling { interval_ms } => Some(Duration::from_millis(*interval_ms)), + WatchStrategy::Hybrid { + polling_interval_ms, + } => Some(Duration::from_millis(*polling_interval_ms)), + WatchStrategy::Push => None, + } + } +} + +/// Cached state from last poll, used for change detection. +/// +/// The watcher maintains this state to detect changes between polls +/// and only emit events when something actually changed. +#[derive(Clone, Default, Debug)] +pub struct WatchedState { + /// Last known playback state (Playing, Paused, Stopped, etc.) + pub state: Option, + /// Last known playback position info + pub position: Option, + /// Last known volume level (0-100) + pub volume: Option, + /// Last known mute state + pub mute: Option, + /// Last known track metadata + pub metadata: Option, +} + +// ============================================================================ +// Helper functions for change detection +// ============================================================================ + +/// Compares two PlaybackState values for equality. +/// +/// Handles the Unknown variant specially by comparing the inner string. +pub fn playback_state_equal(a: &PlaybackState, b: &PlaybackState) -> bool { + match (a, b) { + (PlaybackState::Unknown(lhs), PlaybackState::Unknown(rhs)) => lhs == rhs, + _ => std::mem::discriminant(a) == std::mem::discriminant(b), + } +} + +/// Compares two PlaybackPositionInfo values for equality. +pub fn playback_position_equal(a: &PlaybackPositionInfo, b: &PlaybackPositionInfo) -> bool { + a.track == b.track + && a.rel_time == b.rel_time + && a.abs_time == b.abs_time + && a.track_duration == b.track_duration + && a.track_metadata == b.track_metadata + && a.track_uri == b.track_uri +} + +/// Compute a logical playback state by combining the raw AVTransport state +/// with previous and current position information. +/// +/// This is designed to compensate for buggy LinkPlay/Arylic devices that +/// report: +/// - STOPPED while the time actually advances, +/// - NO_MEDIA_PRESENT while track duration is known. +pub fn compute_logical_playback_state( + raw: &PlaybackState, + prev_position: Option<&PlaybackPositionInfo>, + current_position: Option<&PlaybackPositionInfo>, +) -> PlaybackState { + // Rule 1: Arylic / LinkPlay sometimes report STOPPED while the stream is + // actually playing. If we detect that the relative time advances between + // two polls, we treat this as Playing. + if let PlaybackState::Stopped = raw { + if let (Some(prev), Some(curr)) = (prev_position, current_position) { + if let (Some(prev_rel), Some(curr_rel)) = ( + parse_optional_hms_to_secs(&prev.rel_time), + parse_optional_hms_to_secs(&curr.rel_time), + ) { + if curr_rel > prev_rel { + let delta = curr_rel - prev_rel; + // Our poll loop runs every 500ms; accept small jitter in the delta. + if delta <= 5 { + return PlaybackState::Playing; + } + } + } + } + } + + // Rule 2: Some devices report NO_MEDIA_PRESENT while exposing a non-zero + // track duration. In practice this behaves like a stopped transport with + // a loaded track. + if let PlaybackState::NoMedia = raw { + let duration_secs = current_position + .and_then(|p| parse_optional_hms_to_secs(&p.track_duration)) + .or_else(|| prev_position.and_then(|p| parse_optional_hms_to_secs(&p.track_duration))); + + if matches!(duration_secs, Some(d) if d > 0) { + return PlaybackState::Stopped; + } + } + + // Fallback: keep the raw (already normalized) state. + raw.clone() +} + +/// Parse an optional HH:MM:SS time string to seconds. +fn parse_optional_hms_to_secs(value: &Option) -> Option { + value.as_ref().and_then(|s| parse_hms_to_secs(s)) +} + +/// Parse "HH:MM:SS" style time strings to seconds. +/// +/// Returns None for empty or sentinel values such as "NOT_IMPLEMENTED" or "-:--:--". +fn parse_hms_to_secs(s: &str) -> Option { + let s = s.trim(); + if s.is_empty() { + return None; + } + + // Common sentinel values for "no information" in UPnP implementations. + if s == "NOT_IMPLEMENTED" || s == "-:--:--" { + return None; + } + + let parts: Vec<_> = s.split(':').collect(); + if parts.len() != 3 { + return None; + } + + let hours: u64 = parts[0].parse().ok()?; + let minutes: u64 = parts[1].parse().ok()?; + let seconds: u64 = parts[2].parse().ok()?; + + Some(hours * 3600 + minutes * 60 + seconds) +} + +/// Extract TrackMetadata from DIDL-Lite XML in PlaybackPositionInfo. +pub fn extract_track_metadata(position: &PlaybackPositionInfo) -> Option { + let didl_xml = position.track_metadata.as_ref()?; + + // Parse DIDL-Lite XML + let didl = match pmodidl::parse_metadata::(didl_xml) { + Ok(parsed) => parsed.data, + Err(_) => return None, + }; + + // Extract first item metadata + let item = didl.items.first()?; + + Some(TrackMetadata { + title: Some(item.title.clone()), + artist: item.artist.clone(), + album: item.album.clone(), + genre: item.genre.clone(), + album_art_uri: item.album_art.clone(), + date: item.date.clone(), + track_number: item.original_track_number.clone(), + creator: item.creator.clone(), + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_parse_hms_to_secs() { + assert_eq!(parse_hms_to_secs("00:00:00"), Some(0)); + assert_eq!(parse_hms_to_secs("00:01:30"), Some(90)); + assert_eq!(parse_hms_to_secs("01:00:00"), Some(3600)); + assert_eq!(parse_hms_to_secs("01:30:45"), Some(5445)); + assert_eq!(parse_hms_to_secs("NOT_IMPLEMENTED"), None); + assert_eq!(parse_hms_to_secs("-:--:--"), None); + assert_eq!(parse_hms_to_secs(""), None); + assert_eq!(parse_hms_to_secs("invalid"), None); + } + + #[test] + fn test_playback_state_equal() { + assert!(playback_state_equal( + &PlaybackState::Playing, + &PlaybackState::Playing + )); + assert!(playback_state_equal( + &PlaybackState::Stopped, + &PlaybackState::Stopped + )); + assert!(!playback_state_equal( + &PlaybackState::Playing, + &PlaybackState::Stopped + )); + assert!(playback_state_equal( + &PlaybackState::Unknown("foo".to_string()), + &PlaybackState::Unknown("foo".to_string()) + )); + assert!(!playback_state_equal( + &PlaybackState::Unknown("foo".to_string()), + &PlaybackState::Unknown("bar".to_string()) + )); + } + + #[test] + fn test_watch_strategy_polling_interval() { + let polling = WatchStrategy::Polling { interval_ms: 500 }; + assert_eq!(polling.polling_interval(), Some(Duration::from_millis(500))); + + let hybrid = WatchStrategy::Hybrid { + polling_interval_ms: 1000, + }; + assert_eq!(hybrid.polling_interval(), Some(Duration::from_millis(1000))); + + let push = WatchStrategy::Push; + assert_eq!(push.polling_interval(), None); + } +} diff --git a/pmocontrol/src/registry.rs b/pmocontrol/src/registry.rs index 2cf433e8..1c931863 100644 --- a/pmocontrol/src/registry.rs +++ b/pmocontrol/src/registry.rs @@ -164,6 +164,7 @@ impl DeviceRegistry { if let Some(entry) = self.devices.get_mut(&device_id) { if let Some(renderer) = &entry.music_renderer { let was_online = renderer.is_online(); + // has_been_seen_now() automatically calls start_watching() if offline→online renderer.has_been_seen_now(max_age); if !was_online { @@ -175,6 +176,7 @@ impl DeviceRegistry { return; } // Entry existe mais pas de renderer -> on l'ajoute + // Constructor automatically calls start_watching() if let Ok(new_renderer) = MusicRenderer::from_renderer_info_with_bus(info, Some(self.renderer_bus.clone())) { @@ -182,7 +184,6 @@ impl DeviceRegistry { self.udn_index .insert(info.udn().to_ascii_lowercase(), device_id.clone()); - // Broadcast sur le bon bus self.renderer_bus.broadcast(RendererEvent::Online { id: device_id.clone(), info: info.basic_info(), @@ -190,6 +191,7 @@ impl DeviceRegistry { } } else { // Entry n'existe pas -> on crée + // Constructor automatically calls start_watching() if let Ok(new_renderer) = MusicRenderer::from_renderer_info_with_bus(info, Some(self.renderer_bus.clone())) { @@ -202,7 +204,6 @@ impl DeviceRegistry { self.udn_index .insert(info.udn().to_ascii_lowercase(), device_id.clone()); - // Broadcast sur le bon bus self.renderer_bus.broadcast(RendererEvent::Online { id: device_id, info: info.basic_info(), @@ -265,15 +266,19 @@ impl DeviceRegistry { /// /// This is critical for keeping devices online when SSDP Alive messages arrive /// more frequently than the UDN cache refresh interval (max_age/2). + /// + /// Note: has_been_seen_now() automatically calls start_watching() for renderers + /// when transitioning from offline to online. pub fn refresh_device_presence(&mut self, udn: &str, max_age: u32) { let lookup = udn.to_ascii_lowercase(); if let Some(id) = self.udn_index.get(&lookup) { if let Some(device) = self.devices.get(id) { let was_online = device.is_online(); + // has_been_seen_now() automatically calls start_watching() if offline→online device.has_been_seen_now(max_age); - // If device was offline and came back online, broadcast Online event + // Broadcast Online event if device came back online if !was_online { if device.is_a_music_renderer() { if let Ok(renderer) = device.as_music_renderer() { @@ -301,9 +306,9 @@ impl DeviceRegistry { if let Some(id) = self.udn_index.get(&lookup) { if let Some(device) = self.devices.get(id) { + // mark_as_offline() automatically calls stop_watching() for renderers device.mark_as_offline(); - // Broadcast sur le bon bus if device.is_a_music_renderer() { self.renderer_bus .broadcast(RendererEvent::Offline { id: id.clone() }); @@ -323,9 +328,9 @@ impl DeviceRegistry { for (id, device) in &self.devices { if let Ok(elapsed) = now.duration_since(device.last_seen()) { if elapsed.as_secs() > device.max_age() as u64 { + // mark_as_offline() automatically calls stop_watching() for renderers device.mark_as_offline(); - // Broadcast sur le bon bus if device.is_a_music_renderer() { self.renderer_bus .broadcast(RendererEvent::Offline { id: id.clone() }); diff --git a/version.txt b/version.txt index 42045aca..c2c0004f 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.3.4 +0.3.5