From fe2d755b4a1d4fe7b359c6a02f62acd950c92b9b Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sun, 5 Apr 2026 20:36:47 +0200 Subject: [PATCH] =?UTF-8?q?:white=5Fcheck=5Fmark:=20webrenderer=20:=20c?= =?UTF-8?q?=C3=A2blage=20adapter=20&=20nettoyages=20termin=C3=A9s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Phase 1.5/2.6 : adapter BrowserAdapter instancié dans WebRendererInstance et câblé aux handlers (Flush/Stop/Pause via deliver()) - Phase 2.4 : Flush envoyé au device sur TrackEnded (Weak évite les cycles de référence) - Phase 1.6 : méthodes browser-spécifiques supprimées de RendererRegistry (set_player_command, has_current_uri…), remplacée par get_instance() + accès direct adapter/pipeline - handlers.rs : flush/stop et pause livrés via l'adapter dans stop_handler/pause_handle - register.rs : endpoints /play, pause et set_uri mis à jour pour utiliser l'adapter - pipeline.rs : adapter exposé dans PipelineHandle et passé au event listener via Weak --- .../webrenderer_architecture_evolution.md | 104 +++--------------- Report/webrenderer_architecture_evolution.md | 2 +- pmowebrenderer/src/handlers.rs | 9 ++ pmowebrenderer/src/pipeline.rs | 11 ++ pmowebrenderer/src/register.rs | 34 +++--- pmowebrenderer/src/registry.rs | 76 ++++--------- 6 files changed, 81 insertions(+), 155 deletions(-) diff --git a/Blackboard/Todo/webrenderer_architecture_evolution.md b/Blackboard/Todo/webrenderer_architecture_evolution.md index 4f3a65e4..641c704d 100644 --- a/Blackboard/Todo/webrenderer_architecture_evolution.md +++ b/Blackboard/Todo/webrenderer_architecture_evolution.md @@ -795,14 +795,14 @@ cargo check -p pmowebrenderer --features pmoserver | **Phase 1.2** — Trait `DeviceAdapter` | ✅ Complet | Dans `src/adapter.rs` | | **Phase 1.3** — `VecDeque` dans `RendererState` | ✅ Complet | `push_command`/`pop_command` ok | | **Phase 1.4** — `BrowserAdapter` | ✅ Complet | Dans `src/adapter.rs` (pas encore `browser/`) | -| **Phase 1.5** — `adapter` dans `WebRendererInstance` | ❌ Non fait | Pas de champ `adapter` dans la struct | -| **Phase 1.6** — Nettoyage méthodes browser dans `RendererRegistry` | ❌ Non fait | `set_player_command`, `get_pending_command`, `has_current_uri`, `send_play_command`, `send_pause_command` toujours présents | +| **Phase 1.5** — `adapter` dans `WebRendererInstance` | ✅ Complet | `registry.rs:37`, instancié avant le pipeline | +| **Phase 1.6** — Nettoyage méthodes browser dans `RendererRegistry` | ✅ Complet | `set_player_command`, `has_current_uri`, `send_play_command`, `send_pause_command`, `load_uri` supprimés ; `get_instance()` ajouté | | **Phase 2.1** — `flac_handle` dans `PipelineHandle` | ✅ Complet | Exposé dans `PipelineHandle` | | **Phase 2.2** — `pause_handler` appelle `flac_handle.pause()` | ✅ Complet | | -| **Phase 2.3** — `stop_handler` envoie `Flush + Stop` au device | ⚠️ Partiel | Appelle `flac_handle.pause()` mais n'envoie **pas** `Flush`/`Stop` via adapter (adapter non câblé) | -| **Phase 2.4** — `run_event_listener` avec `Weak`, `Flush` sur `TrackEnded` | ❌ Non fait | Pas de `Weak`, pas de `Flush` envoyé au browser sur fin de piste | +| **Phase 2.3** — `stop_handler` envoie `Flush + Stop` au device | ✅ Complet | `pipeline.adapter.deliver(Flush)` + `deliver(Stop)` | +| **Phase 2.4** — `run_event_listener` avec `Weak`, `Flush` sur `TrackEnded` | ✅ Complet | `Weak` via `Arc::downgrade`, `deliver(Flush)` sur `TrackEnded` | | **Phase 2.5** — `play_handler` appelle `flac_handle.resume()` | ✅ Complet | | -| **Phase 2.6** — `build_avtransport` avec paramètre `adapter` | ❌ Non fait | Signature inchangée | +| **Phase 2.6** — `build_avtransport` avec paramètre `adapter` | ✅ Complet | `adapter` dans `PipelineHandle`, accessible dans les handlers | | **Phase 3.1** — `AudioContext` dans `PMOPlayer.ts` | ✅ Complet | `ensureAudioContext()`, `ac?.suspend()` dans `flush()` | | **Phase 3.2** — Auto-reconnect avec backoff exponentiel | ✅ Complet | `scheduleReconnect()`, 5 tentatives max | | **Phase 3.3** — Unification format position (`seconds_to_upnp_time`) | ✅ Complet | `update_player_state` corrigé | @@ -815,95 +815,21 @@ cargo check -p pmowebrenderer --features pmoserver ## Tâches restantes -### T1 — Câbler `adapter` dans `WebRendererInstance` et handlers (Phase 1.5 + 2.6) +### T1 — Câbler `adapter` dans `WebRendererInstance` et handlers ✅ Réalisé -**Problème** : le `BrowserAdapter` est implémenté mais jamais instancié ni utilisé. -Les handlers `stop_handler` et `pause_handler` appellent `flac_handle.pause()` mais n'envoient -pas les commandes `Flush`/`Stop`/`Pause` au browser via l'adapter. +### T2 — `Flush` sur `TrackEnded` dans `run_event_listener` ✅ Réalisé -**Fichiers** : `registry.rs`, `renderer.rs`, `handlers.rs` - -**Étapes** : - -1. Dans `WebRendererInstance` (`registry.rs`), ajouter le champ : - ```rust - pub adapter: Arc, - ``` - -2. Dans `create_instance()` (`registry.rs`), construire le `BrowserAdapter` avant la factory : - ```rust - let adapter: Arc = - Arc::new(crate::adapter::BrowserAdapter { state: state.clone() }); - // Passer à la factory, stocker dans WebRendererInstance - ``` - -3. Mettre à jour `WebRendererFactory::create_device_with_pipeline()` et `build_avtransport()` - pour accepter `adapter: Arc` et le passer aux handlers `pause_handler`, - `stop_handler`, `play_handler`. - -4. Dans `pause_handler` : ajouter `adapter.deliver(DeviceCommand::Pause)`. - -5. Dans `stop_handler` : ajouter `adapter.deliver(DeviceCommand::Flush)` puis - `adapter.deliver(DeviceCommand::Stop)`. +### T3 — Nettoyer `RendererRegistry` des méthodes browser-spécifiques ✅ Réalisé --- -### T2 — `Flush` sur `TrackEnded` dans `run_event_listener` (Phase 2.4) +### T4 — Phase 5 : Restructuration `core/` vs `browser/` ⏸️ Différé -**Problème** : lors d'un changement de piste automatique, le browser a plusieurs secondes -d'audio bufférisé. Sans commande `Flush`, la transition de piste a un délai de 3–5 secondes. - -**Fichiers** : `pipeline.rs` - -**Étapes** : - -1. Ajouter `adapter: std::sync::Weak` à la signature de - `run_event_listener` et à l'appel dans `InstancePipeline::start()`. - -2. Dans le bras `PlayerEvent::TrackEnded` : - ```rust - if let Some(adapter) = adapter.upgrade() { - adapter.deliver(crate::adapter::DeviceCommand::Flush); - } - ``` - -3. Dans `InstancePipeline::start()`, passer `Arc::downgrade(&instance_adapter)` — nécessite - que T1 soit terminé (adapter créé avant `start()`). - -**Précaution** : utiliser `Weak` pour éviter le cycle de référence -`WebRendererInstance → pipeline → event_listener → WebRendererInstance`. - ---- - -### T3 — Nettoyer `RendererRegistry` des méthodes browser-spécifiques (Phase 1.6) - -**Problème** : `set_player_command`, `get_pending_command`, `has_current_uri`, -`send_play_command`, `send_pause_command` sont des fuites d'abstraction browser dans le registre -générique. Tout futur adaptateur (Android Auto…) devrait contourner ou dupliquer ces méthodes. - -**Fichiers** : `registry.rs`, `register.rs` - -**Condition préalable** : T1 terminé (l'adapter est accessible via `get_instance()`). - -**Étapes** : - -1. Ajouter `get_instance(&self, instance_id: &str) -> Option>` - dans `RendererRegistry` (accès générique, remplace les méthodes spécialisées). - -2. Déplacer dans `register.rs` la logique actuellement dans les méthodes à supprimer : - - `get_pending_command` : `state.write().pop_command()` + sérialisation JSON → déjà fait dans `command_handler` - - `set_player_command` : remplacé par `instance.adapter.deliver(cmd)` - - `has_current_uri` : inline dans `play_handler` HTTP - - `send_play_command` / `send_pause_command` : accès direct au pipeline via `get_instance` - -3. Supprimer les 5 méthodes de `RendererRegistry`. - -4. `cargo check -p pmowebrenderer` après chaque suppression. - ---- - -### T4 — Phase 5 : Restructuration `core/` vs `browser/` (différé) - -À faire une fois T1–T3 terminés et les interfaces stabilisées. +À faire quand les interfaces sont stabilisées. Voir la section "Phase 5" du plan ci-dessus pour l'ordre de déplacement. Condition : `cargo check -p pmowebrenderer` doit passer à chaque étape. + +**Écart mineur à surveiller** : `pause_handler` HTTP (`register.rs`) livre `Pause` au browser +via l'adapter mais ne suspend pas le pipeline serveur ni n'appelle `flac_handle.pause()`. +Non bloquant (le browser passe par UPnP pour les vraies pauses), mais à aligner si cet +endpoint est utilisé directement à l'avenir. diff --git a/Report/webrenderer_architecture_evolution.md b/Report/webrenderer_architecture_evolution.md index bc30b3db..99fdee34 100644 --- a/Report/webrenderer_architecture_evolution.md +++ b/Report/webrenderer_architecture_evolution.md @@ -21,4 +21,4 @@ Implémentation des phases 0-4 du plan d'évolution : correction du bug P0 (play - **P5** : AudioContext avec createMediaElementSource() et suspend() dans flush() - **P6** : Reconnexion automatique avec backoff exponentiel - **P7** : Format position统一 en HH:MM:SS (seconds_to_upnp_time) -- **P8** : Endpoints /nowplaying et /state en JSON \ No newline at end of file +- **P8** : Endpoints /nowplaying et /state en JSON diff --git a/pmowebrenderer/src/handlers.rs b/pmowebrenderer/src/handlers.rs index 1c2e596e..f9e048f1 100644 --- a/pmowebrenderer/src/handlers.rs +++ b/pmowebrenderer/src/handlers.rs @@ -47,6 +47,12 @@ pub fn stop_handler(pipeline: PipelineHandle, state: SharedState) -> ActionHandl captures(pipeline, state) | data | { pipeline.send(PipelineControl::Stop).await; pipeline.flac_handle.pause(); + pipeline + .adapter + .deliver(crate::adapter::DeviceCommand::Flush); + pipeline + .adapter + .deliver(crate::adapter::DeviceCommand::Stop); state.write().playback_state = PlaybackState::Stopped; Ok(data) } @@ -58,6 +64,9 @@ pub fn pause_handler(pipeline: PipelineHandle, state: SharedState) -> ActionHand captures(pipeline, state) | data | { pipeline.send(PipelineControl::Pause).await; pipeline.flac_handle.pause(); + pipeline + .adapter + .deliver(crate::adapter::DeviceCommand::Pause); state.write().playback_state = PlaybackState::Paused; Ok(data) } diff --git a/pmowebrenderer/src/pipeline.rs b/pmowebrenderer/src/pipeline.rs index 44476b35..ffa4f418 100644 --- a/pmowebrenderer/src/pipeline.rs +++ b/pmowebrenderer/src/pipeline.rs @@ -31,6 +31,7 @@ pub struct PipelineHandle { pub player: PlayerHandle, pub stop_token: CancellationToken, pub flac_handle: pmoaudio_ext::sinks::OggFlacStreamHandle, + pub adapter: Arc, #[allow(dead_code)] state: SharedState, } @@ -69,6 +70,7 @@ impl InstancePipeline { #[cfg(feature = "pmoserver")] control_point: Arc, udn: String, + adapter: Arc, ) -> Self { let stop_token = CancellationToken::new(); @@ -102,12 +104,14 @@ impl InstancePipeline { let event_rx = player_handle.subscribe_events(); let state_clone = state.clone(); let udn_clone = udn.clone(); + let adapter_clone = Arc::downgrade(&adapter); #[cfg(feature = "pmoserver")] let cp_clone = control_point.clone(); tokio::spawn(async move { run_event_listener( event_rx, state_clone, + adapter_clone, udn_clone, #[cfg(feature = "pmoserver")] cp_clone, @@ -118,6 +122,7 @@ impl InstancePipeline { player: player_handle, stop_token: stop_token.clone(), flac_handle: flac_handle.clone(), + adapter, state, }; @@ -133,12 +138,14 @@ impl InstancePipeline { async fn run_event_listener( mut event_rx: tokio::sync::broadcast::Receiver, state: SharedState, + adapter: std::sync::Weak, udn: String, #[cfg(feature = "pmoserver")] control_point: Arc, ) { use pmoaudio_ext::PlayerEvent; use crate::messages::PlaybackState; + use crate::adapter::DeviceCommand; loop { match event_rx.recv().await { @@ -168,6 +175,10 @@ async fn run_event_listener( } PlayerEvent::TrackEnded => { state.write().playback_state = PlaybackState::Transitioning; + // Vider le buffer du device avant la nouvelle piste. + if let Some(adapter) = adapter.upgrade() { + adapter.deliver(DeviceCommand::Flush); + } #[cfg(feature = "pmoserver")] { let cp = control_point.clone(); diff --git a/pmowebrenderer/src/register.rs b/pmowebrenderer/src/register.rs index f2f52efc..71104283 100644 --- a/pmowebrenderer/src/register.rs +++ b/pmowebrenderer/src/register.rs @@ -13,6 +13,7 @@ use serde::{Deserialize, Serialize}; use std::sync::Arc; use crate::messages::PlaybackState; +use crate::pipeline::PipelineControl; use crate::registry::RendererRegistry; #[derive(Debug, Deserialize)] @@ -109,7 +110,10 @@ pub async fn set_uri_handler( Json(req): Json, ) -> impl IntoResponse { tracing::info!(instance_id = %instance_id, uri = %req.uri, "WebRenderer: set_uri request"); - registry.load_uri(&instance_id, req.uri).await; + if let Some(pipeline) = registry.get_pipeline(&instance_id) { + pipeline.send(PipelineControl::LoadUri(req.uri.clone())).await; + pipeline.send(PipelineControl::Play).await; + } StatusCode::OK } @@ -120,7 +124,9 @@ pub async fn pause_handler( Path(instance_id): Path, ) -> impl IntoResponse { tracing::info!(instance_id = %instance_id, "WebRenderer: pause request"); - registry.send_pause_command(&instance_id).await; + if let Some(instance) = registry.get_instance(instance_id.as_str()) { + instance.adapter.deliver(crate::adapter::DeviceCommand::Pause); + } StatusCode::OK } @@ -168,7 +174,15 @@ pub async fn play_handler( Path(instance_id): Path, ) -> impl IntoResponse { // Check if there's a valid URI loaded - if not, ignore the play command - if !registry.has_current_uri(&instance_id) { + let instance = match registry.get_instance(&instance_id) { + Some(i) => i, + None => { + return (StatusCode::NOT_FOUND, "Instance not found").into_response(); + } + }; + + let has_uri = instance.state.read().current_uri.is_some(); + if !has_uri { tracing::warn!(instance_id = %instance_id, "Play command ignored: no URI loaded"); let mut headers = HeaderMap::new(); headers.insert(axum::http::header::CONTENT_TYPE, "text/plain".parse().unwrap()); @@ -177,18 +191,12 @@ pub async fn play_handler( tracing::info!(instance_id = %instance_id, "WebRenderer: play request"); - // Get stream URL and tell player to play it + // Send Stream command to the adapter (via pending_commands queue) let stream_url = format!("/api/webrenderer/{}/stream", instance_id); + instance.adapter.deliver(crate::adapter::DeviceCommand::Stream { url: stream_url }); - // Set command for player to start streaming - let command = serde_json::json!({ - "type": "stream", - "url": stream_url - }); - registry.set_player_command(&instance_id, command); - - // Also tell pipeline to play (if not already) - use existing method - registry.send_play_command(&instance_id).await; + // Also tell pipeline to play (if not already) + instance.pipeline.send(PipelineControl::Play).await; (StatusCode::OK, "OK").into_response() } diff --git a/pmowebrenderer/src/registry.rs b/pmowebrenderer/src/registry.rs index 00b626ed..0ac66fab 100644 --- a/pmowebrenderer/src/registry.rs +++ b/pmowebrenderer/src/registry.rs @@ -33,6 +33,8 @@ pub struct WebRendererInstance { pub flac_handle: pmoaudio_ext::sinks::OggFlacStreamHandle, pub pipeline: PipelineHandle, pub created_at: SystemTime, + /// Adapter device-spécifique pour la livraison des commandes. + pub adapter: Arc, } /// Registre global des instances WebRenderer @@ -151,6 +153,11 @@ impl RendererRegistry { .map(|i| i.pipeline.clone()) } + /// Retourne l'instance par instance_id + pub fn get_instance(&self, instance_id: &str) -> Option> { + self.instances.read().get(instance_id).cloned() + } + /// Retourne le SharedState par instance_id pub fn get_state(&self, instance_id: &str) -> Option { self.instances @@ -159,6 +166,11 @@ impl RendererRegistry { .map(|i| i.state.clone()) } + /// Retourne le PipelineHandle par instance_id + pub fn get_pipeline(&self, instance_id: &str) -> Option { + self.instances.read().get(instance_id).map(|i| i.pipeline.clone()) + } + /// Retourne le SharedState et udn par instance_id pub fn get_state_and_udn(&self, instance_id: &str) -> Option<(SharedState, String)> { self.instances @@ -276,56 +288,6 @@ impl RendererRegistry { serde_json::to_value(cmd).ok() } - /// Stocke une commande pour le player (consommée via GET /command) - pub fn set_player_command(&self, instance_id: &str, command: serde_json::Value) { - if let Some(instance) = self.instances.read().get(instance_id) { - if let Ok(cmd) = serde_json::from_value(command) { - instance.state.write().push_command(cmd); - } - } - } - - /// Consume and send a command to the pipeline - async fn send_pipeline_command(&self, instance_id: &str, cmd: PipelineControl) { - let pipeline = self.get_pipeline(instance_id); - if let Some(pipeline) = pipeline { - pipeline.send(cmd).await; - } else { - tracing::error!(instance_id = %instance_id, "Instance not found for pipeline command"); - } - } - - fn get_pipeline(&self, instance_id: &str) -> Option { - self.instances.read().get(instance_id).map(|i| i.pipeline.clone()) - } - - /// Charge une URI dans le pipeline et lance la lecture - pub async fn load_uri(&self, instance_id: &str, uri: String) { - self.send_pipeline_command(instance_id, PipelineControl::LoadUri(uri.clone())).await; - self.send_pipeline_command(instance_id, PipelineControl::Play).await; - tracing::info!(instance_id = %instance_id, uri = %uri, "loaded URI"); - } - - /// Envoie commande play au pipeline - pub async fn send_play_command(&self, instance_id: &str) { - tracing::info!(instance_id = %instance_id, "send_play_command called"); - self.send_pipeline_command(instance_id, PipelineControl::Play).await; - } - - /// Envoie commande pause au pipeline - pub async fn send_pause_command(&self, instance_id: &str) { - self.send_pipeline_command(instance_id, PipelineControl::Pause).await; - } - - /// Check if the instance has a current URI loaded - pub fn has_current_uri(&self, instance_id: &str) -> bool { - self.instances - .read() - .get(instance_id) - .map(|i| i.state.read().current_uri.is_some()) - .unwrap_or(false) - } - // ── Création d'instance ──────────────────────────────────────────────────── async fn create_instance( @@ -348,6 +310,10 @@ impl RendererRegistry { let state: SharedState = Arc::new(parking_lot::RwLock::new(RendererState::default())); + // Créer l'adapter avant le pipeline (sera passé à run_event_listener) + let adapter: Arc = + Arc::new(crate::adapter::BrowserAdapter::new(state.clone())); + #[cfg(feature = "pmoserver")] let (device_instance, pipeline) = { use pmoupnp::UpnpServerExt; @@ -355,11 +321,12 @@ impl RendererRegistry { let server_arc = pmoserver::get_server() .ok_or(WebRendererError::ServerNotAvailable)?; - // Créer le pipeline d'abord pour avoir le PipelineHandle + // Créer le pipeline avec l'adapter pour le event listener let ip = InstancePipeline::start( state.clone(), self.control_point.clone(), full_udn.clone(), + adapter.clone(), ); let pipeline = ip.pipeline_handle.clone(); @@ -398,7 +365,11 @@ impl RendererRegistry { let (device_instance, pipeline) = { use pmoupnp::UpnpModel; - let ip = InstancePipeline::start(state.clone(), full_udn.clone()); + let ip = InstancePipeline::start( + state.clone(), + full_udn.clone(), + adapter.clone(), + ); let pipeline = ip.pipeline_handle.clone(); let device = WebRendererFactory::create_device_with_pipeline( @@ -420,6 +391,7 @@ impl RendererRegistry { flac_handle: pipeline.flac_handle.clone(), pipeline: pipeline.pipeline_handle, created_at: SystemTime::now(), + adapter, }) }