From 76a1c89137385262ded76e35cf3ed48f436263b8 Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sun, 5 Apr 2026 14:32:19 +0200 Subject: [PATCH] :wrench: Refactor PMOPlayer and WebRenderer command handling - Extract beforeunload handler in useWebrenderer.ts to ensure consistent unregister() calls - Remove unused AudioContext from PMOPlayer and clean up destroy() - Add guaranteed final report via sendBeacon during destruction - Simplify playStream() in PMOPlayer by removing URL rewriting logic (now handled server-side) - Update play_handler to pass instance_id and store stream command atomically - Avoid race conditions when setting current_uri before sending Play control event - Remove obsolete WebSocket message types from messages.rs (now using HTTP polling) - Keep only core PlaybackState enum - Fix registry::get_player_command to avoid nested locks on HashMap + Sharedstate Arcs - Add missing PlaybackState::Transitioning in pipeline.rs TrackEnded event - Refactor renderer service variable registration using helper add_var() function --- .../webapp/src/composables/useWebRenderer.ts | 6 +- pmoapp/webapp/src/services/PMOPlayer.ts | 134 +++++++++--------- pmowebrenderer/src/error.rs | 6 - pmowebrenderer/src/handlers.rs | 35 +++-- pmowebrenderer/src/messages.rs | 120 +--------------- pmowebrenderer/src/pipeline.rs | 1 + pmowebrenderer/src/registry.rs | 8 +- pmowebrenderer/src/renderer.rs | 32 ++--- 8 files changed, 111 insertions(+), 231 deletions(-) diff --git a/pmoapp/webapp/src/composables/useWebRenderer.ts b/pmoapp/webapp/src/composables/useWebRenderer.ts index c5d013dd..46d90428 100644 --- a/pmoapp/webapp/src/composables/useWebRenderer.ts +++ b/pmoapp/webapp/src/composables/useWebRenderer.ts @@ -139,14 +139,16 @@ export function useWebRenderer() { // PMOPlayer doesn't control mute directly - handled by backend } + const beforeUnloadHandler = () => void unregister(); + onMounted(() => { void register(); - window.addEventListener("beforeunload", () => void unregister()); + window.addEventListener("beforeunload", beforeUnloadHandler); }); onUnmounted(() => { void unregister(); - window.removeEventListener("beforeunload", () => void unregister()); + window.removeEventListener("beforeunload", beforeUnloadHandler); }); return { diff --git a/pmoapp/webapp/src/services/PMOPlayer.ts b/pmoapp/webapp/src/services/PMOPlayer.ts index e0c21518..974c6a15 100644 --- a/pmoapp/webapp/src/services/PMOPlayer.ts +++ b/pmoapp/webapp/src/services/PMOPlayer.ts @@ -1,9 +1,9 @@ /** * PMOPlayer - Invisible remote-controlled audio player for browser - * + * * Receives commands from Web Media Renderer (backend) * Reports position/state back to backend via HTTP - * + * * Usage: * const player = new PMOPlayer('my-instance-id'); * player.on('play', () => console.log('playing')); @@ -35,9 +35,8 @@ export interface TrackInfo { export class PMOPlayer { private audio: HTMLAudioElement; - private ac: AudioContext | null = null; private instanceId: string; - + private state: PlayerState = 'stopped'; private positionInterval: number | null = null; private commandInterval: number | null = null; @@ -45,7 +44,7 @@ export class PMOPlayer { private debug: boolean = false; private pendingPlay = false; private unlockListener: (() => void) | null = null; - + constructor(instanceId: string) { this.instanceId = instanceId; console.log('[PMOPlayer] constructor called for:', instanceId); @@ -59,17 +58,17 @@ export class PMOPlayer { this.audio.style.height = '0'; this.audio.style.overflow = 'hidden'; document.body.appendChild(this.audio); - + this.setupAudioListeners(); this.startCommandPolling(); } - + private log(...args: unknown[]) { if (this.debug) { console.log('[PMOPlayer]', ...args); } } - + private async fetchCommand() { try { const resp = await fetch(`/api/webrenderer/${this.instanceId}/command`); @@ -91,31 +90,35 @@ export class PMOPlayer { this.log('fetch command error', err); } } - + private startCommandPolling() { this.commandInterval = window.setInterval(() => { + // ATTENTION : ne pas conditionner ce poll à l'état courant. + // L'état initial est 'stopped', et c'est dans cet état que le backend + // envoie la première commande 'stream' pour démarrer la lecture. + // Filtrer sur state !== 'stopped' casse le chemin nominal de démarrage. this.fetchCommand(); }, 500); } - + private setupAudioListeners() { this.audio.addEventListener('play', () => { this.log('play event'); this.setState('playing'); this.startPositionReporting(); }); - + this.audio.addEventListener('pause', () => { this.log('pause event'); this.setState('paused'); }); - + this.audio.addEventListener('ended', () => { this.log('ended event'); this.setState('stopped'); this.stopPositionReporting(); }); - + this.audio.addEventListener('error', () => { // Ignorer l'erreur produite par flush() (removeAttribute('src') + load()) if (!this.audio.getAttribute('src')) return; @@ -123,17 +126,17 @@ export class PMOPlayer { this.setState('error'); this.listeners.error?.(this.audio.error?.message || 'unknown error'); }); - + this.audio.addEventListener('waiting', () => { this.log('waiting event'); this.setState('buffering'); }); - + this.audio.addEventListener('canplay', () => { this.log('canplay event'); this.reportReadyState('can_play'); }); - + this.audio.addEventListener('durationchange', () => { const dur = this.audio.duration; if (isFinite(dur)) { @@ -141,28 +144,24 @@ export class PMOPlayer { this.reportDuration(dur); } }); - + this.audio.addEventListener('loadedmetadata', () => { this.log('loadedmetadata', this.audio.duration); this.reportReadyState('have_metadata'); }); - + this.audio.addEventListener('loadeddata', () => { this.log('loadeddata'); this.reportReadyState('have_current_data'); }); } - + private handleCommand(msg: Record) { const type = msg.type as string; switch (type) { case 'stream': { - let url = msg.url as string; - if (url.startsWith('/api/webrenderer/stream')) { - url = `/api/webrenderer/${this.instanceId}/stream`; - } - this.playStream(url); + this.playStream(msg.url as string); break; } case 'play': { @@ -184,21 +183,21 @@ export class PMOPlayer { break; } } - + // ─── Commands from backend ───────────────────────────────────────────── - + stream(url: string) { // URL stored in audio.src directly this.log('stream:', url); this.audio.src = url; this.audio.load(); } - + playStream(url: string) { this.stream(url); this.play(); } - + play() { this.log('play()'); this.audio.play().catch(err => { @@ -225,72 +224,71 @@ export class PMOPlayer { }; document.addEventListener('click', this.unlockListener); } - + pause() { this.log('pause()'); this.audio.pause(); } - + seek(timestamp: number) { this.log('seek:', timestamp); this.audio.currentTime = timestamp; } - + flush() { this.log('flush()'); this.audio.pause(); this.audio.removeAttribute('src'); this.audio.load(); - this.ac?.suspend(); this.listeners.flush?.(); } - + stop() { this.log('stop()'); this.flush(); this.setState('stopped'); } - + // ─── Reports to backend ────────────────────────────────────────── - + private setState(state: PlayerState) { if (this.state !== state) { this.state = state; this.log('state:', state); this.listeners.statechange?.(state); - + this.httpReport('report', { state: state, }); } } - + private reportPosition() { const pos = this.audio.currentTime; const dur = isFinite(this.audio.duration) ? this.audio.duration : null; - + this.listeners.positionchange?.(pos); - -this.httpReport('report', { - position_sec: pos, - duration_sec: dur, - state: this.state, - }); + + this.httpReport('report', { + position_sec: pos, + duration_sec: dur, + state: this.state, + }); } - + private reportDuration(dur: number) { this.listeners.durationchange?.(dur); } - + private reportReadyState(ready: ReadyState) { this.log('ready_state:', ready); this.listeners.readychange?.(ready); - -this.httpReport('report', { - ready_state: ready, - }); + + this.httpReport('report', { + ready_state: ready, + }); } - + private async httpReport(endpoint: string, data: Record) { try { await fetch(`/api/webrenderer/${this.instanceId}/${endpoint}`, { @@ -302,7 +300,7 @@ this.httpReport('report', { this.log('http report error', err); } } - + private startPositionReporting() { this.stopPositionReporting(); this.positionInterval = window.setInterval(() => { @@ -311,49 +309,48 @@ this.httpReport('report', { } }, 1000); } - + private stopPositionReporting() { if (this.positionInterval !== null) { clearInterval(this.positionInterval); this.positionInterval = null; } } - + // ─── Public API ─────────────────────────────────────────────────────────── - + on(event: K, fn: PlayerEvents[K]) { this.listeners[event] = fn; } - + off(event: K) { delete this.listeners[event]; } - + getState(): PlayerState { return this.state; } - + getPosition(): number { return this.audio.currentTime; } - + getDuration(): number | null { const dur = this.audio.duration; return isFinite(dur) ? dur : null; } - + // Track info from backend - not implemented yet getTrack(): null { return null; } - + setDebug(enabled: boolean) { this.debug = enabled; } - + destroy() { this.log('destroy()'); - this.stop(); this.stopPositionReporting(); if (this.commandInterval !== null) { clearInterval(this.commandInterval); @@ -363,7 +360,14 @@ this.httpReport('report', { document.removeEventListener('click', this.unlockListener); this.unlockListener = null; } + // Rapport final garanti via sendBeacon (fonctionne pendant beforeunload) + navigator.sendBeacon( + `/api/webrenderer/${this.instanceId}/report`, + JSON.stringify({ state: 'stopped' }) + ); + this.audio.pause(); + this.audio.removeAttribute('src'); + this.audio.load(); this.audio.remove(); - this.ac?.close(); } -} \ No newline at end of file +} diff --git a/pmowebrenderer/src/error.rs b/pmowebrenderer/src/error.rs index 2484df69..b67ec766 100644 --- a/pmowebrenderer/src/error.rs +++ b/pmowebrenderer/src/error.rs @@ -4,12 +4,6 @@ use thiserror::Error; #[derive(Error, Debug)] pub enum WebRendererError { - #[error("Session not found: {0}")] - SessionNotFound(String), - - #[error("Failed to send message to websocket: {0}")] - WebSocketSendError(String), - #[error("Invalid argument: {0}")] InvalidArgument(String), diff --git a/pmowebrenderer/src/handlers.rs b/pmowebrenderer/src/handlers.rs index be00caf8..a02abcd8 100644 --- a/pmowebrenderer/src/handlers.rs +++ b/pmowebrenderer/src/handlers.rs @@ -14,19 +14,25 @@ use crate::state::SharedState; // ─── AVTransport : commandes de transport ───────────────────────────────────── -pub fn play_handler(pipeline: PipelineHandle, state: SharedState) -> ActionHandler { - action_handler!(captures(pipeline, state) |data| { +pub fn play_handler(pipeline: PipelineHandle, state: SharedState, instance_id: String) -> ActionHandler { + action_handler!(captures(pipeline, state, instance_id) |data| { tracing::info!("[WebRenderer] UPnP Play action invoked"); - let has_uri = state.read().current_uri.is_some(); - state.write().playback_state = PlaybackState::Transitioning; + let has_uri = { + let mut s = state.write(); + let has = s.current_uri.is_some(); + s.playback_state = PlaybackState::Transitioning; + if has { + s.player_command = Some(serde_json::json!({ + "type": "stream", + "url": format!("/api/webrenderer/{}/stream", instance_id) + })); + tracing::info!("UPnP Play: stored stream command for frontend polling"); + } + has + }; if has_uri { - state.write().player_command = Some(serde_json::json!({ - "type": "stream", - "url": "/api/webrenderer/stream" - })); - tracing::info!("UPnP Play: stored stream command for frontend polling"); + pipeline.send(PipelineControl::Play).await; } - pipeline.send(PipelineControl::Play).await; Ok(data) }) } @@ -81,14 +87,13 @@ pub fn set_uri_handler(pipeline: PipelineHandle, state: SharedState) -> ActionHa .unwrap_or_default(); tracing::info!(uri = %uri, "SetAVTransportURI handler called - loading URI into pipeline"); - pipeline.send(PipelineControl::LoadUri(uri.clone())).await; - { let mut s = state.write(); - s.current_uri = Some(uri); + s.current_uri = Some(uri.clone()); s.current_metadata = Some(metadata); s.playback_state = PlaybackState::Transitioning; } + pipeline.send(PipelineControl::LoadUri(uri)).await; Ok(data) }) } @@ -100,12 +105,12 @@ pub fn set_next_uri_handler(pipeline: PipelineHandle, state: SharedState) -> Act .or_else(|_| get_value::(&data, "NextURIMetaData").map(|didl| didl.to_xml())) .unwrap_or_default(); - pipeline.send(PipelineControl::LoadNextUri(uri.clone())).await; { let mut s = state.write(); - s.next_uri = Some(uri); + s.next_uri = Some(uri.clone()); s.next_metadata = Some(metadata); } + pipeline.send(PipelineControl::LoadNextUri(uri)).await; Ok(data) }) } diff --git a/pmowebrenderer/src/messages.rs b/pmowebrenderer/src/messages.rs index 7a34cb02..6ad5e468 100644 --- a/pmowebrenderer/src/messages.rs +++ b/pmowebrenderer/src/messages.rs @@ -1,108 +1,7 @@ -//! Messages WebSocket pour la communication Backend ↔ Navigateur +//! Types de messages pour le WebRenderer use serde::{Deserialize, Serialize}; -/// Messages envoyés du Backend → Navigateur -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(tag = "type", rename_all = "snake_case")] -#[allow(dead_code)] -pub enum ServerMessage { - SessionCreated { - token: String, - renderer_info: RendererInfo, - }, - /// Envoyé après SessionCreated lors d'une reconnexion pour resynchroniser - /// l'état audio du navigateur (URI courante, état de lecture, etc.). - StateSync { - current_uri: Option, - current_metadata: Option, - next_uri: Option, - next_metadata: Option, - playback_state: PlaybackState, - position: Option, - volume: u16, - mute: bool, - }, - Command { - action: TransportAction, - #[serde(skip_serializing_if = "Option::is_none")] - params: Option, - }, - SetVolume { - volume: u16, - }, - SetMute { - mute: bool, - }, - Ping, -} - -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(rename_all = "snake_case")] -#[allow(dead_code)] -pub enum TransportAction { - Play, - Pause, - Stop, - Seek, - SetUri, - SetNextUri, - /// Flush buffer immediatement - pour reponse rapide - Flush, -} - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct CommandParams { - #[serde(skip_serializing_if = "Option::is_none")] - pub uri: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub metadata: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub position: Option, -} - -/// Messages envoyés du Navigateur → Backend -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(tag = "type", rename_all = "snake_case")] -#[allow(dead_code)] -pub enum ClientMessage { - Init { - capabilities: BrowserCapabilities, - }, - StateUpdate { - state: PlaybackState, - }, - PositionUpdate { - position: String, - duration: String, - }, - MetadataUpdate { - metadata: TrackMetadata, - }, - VolumeUpdate { - volume: u16, - mute: bool, - }, - /// Envoyé quand la piste courante se termine naturellement (gapless). - /// Le backend fait avancer current → next dans l'état partagé. - TrackEnded, - /// Ready state du player HTML5 audio - /// have_nothing, have_metadata, have_current_data, have_future_data, can_play, can_play_through - ReadyStateUpdate { - ready_state: String, - }, - Pong, -} - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct BrowserCapabilities { - /// Identifiant stable de l'instance navigateur (UUID stocké en localStorage). - /// Permet de réutiliser le même renderer UPnP après un reload de page. - pub instance_id: String, - pub user_agent: String, - pub supported_formats: Vec, -} - #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "SCREAMING_SNAKE_CASE")] pub enum PlaybackState { @@ -111,20 +10,3 @@ pub enum PlaybackState { Paused, Transitioning, } - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct TrackMetadata { - pub title: Option, - pub artist: Option, - pub album: Option, - pub duration: Option, - pub album_art_uri: Option, -} - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct RendererInfo { - pub udn: String, - pub friendly_name: String, - pub model_name: String, - pub description_url: String, -} diff --git a/pmowebrenderer/src/pipeline.rs b/pmowebrenderer/src/pipeline.rs index 5236adfd..9be9036f 100644 --- a/pmowebrenderer/src/pipeline.rs +++ b/pmowebrenderer/src/pipeline.rs @@ -165,6 +165,7 @@ async fn run_event_listener( state.write().position = Some(seconds_to_upnp_time(position_sec)); } PlayerEvent::TrackEnded => { + state.write().playback_state = PlaybackState::Transitioning; #[cfg(feature = "pmoserver")] { let cp = control_point.clone(); diff --git a/pmowebrenderer/src/registry.rs b/pmowebrenderer/src/registry.rs index f8a28431..7699d655 100644 --- a/pmowebrenderer/src/registry.rs +++ b/pmowebrenderer/src/registry.rs @@ -255,10 +255,10 @@ impl RendererRegistry { &self, instance_id: &str, ) -> Option { - self.instances - .read() - .get(instance_id) - .and_then(|instance| instance.state.write().player_command.take()) + // Extraire le Arc puis relâcher le read lock du HashMap + // avant d'acquérir le write lock sur state (évite double-lock imbriqué). + let state = self.instances.read().get(instance_id).map(|i| i.state.clone())?; + state.write().player_command.take() } /// Stocke une commande pour le player (consommée via GET /command) diff --git a/pmowebrenderer/src/renderer.rs b/pmowebrenderer/src/renderer.rs index 9291f520..b92796c5 100644 --- a/pmowebrenderer/src/renderer.rs +++ b/pmowebrenderer/src/renderer.rs @@ -117,7 +117,7 @@ impl WebRendererFactory { pipeline: PipelineHandle, state: SharedState, ) -> Result { - let avtransport = Self::build_avtransport(pipeline.clone(), state.clone())?; + let avtransport = Self::build_avtransport(pipeline.clone(), state.clone(), device_name)?; let renderingcontrol = Self::build_renderingcontrol(state.clone())?; let connectionmanager = Self::build_connectionmanager()?; @@ -145,6 +145,7 @@ impl WebRendererFactory { fn build_avtransport( pipeline: PipelineHandle, state: SharedState, + instance_id: &str, ) -> Result { let mut svc = Service::new("AVTransport".to_string()); @@ -176,7 +177,7 @@ impl WebRendererFactory { let mut play = Action::new("Play".to_string()); add_arg_in(&mut play, "InstanceID", &AVT_INSTANCE_ID)?; add_arg_in(&mut play, "Speed", &TRANSPORTPLAYSPEED)?; - play.set_handler(handlers::play_handler(pipeline.clone(), state.clone())); + play.set_handler(handlers::play_handler(pipeline.clone(), state.clone(), instance_id.to_string())); add_action(&mut svc, Arc::new(play))?; // Stop @@ -345,24 +346,15 @@ impl WebRendererFactory { fn build_connectionmanager() -> Result { let mut svc = Service::new("ConnectionManager".to_string()); - svc.add_variable(Arc::clone(&A_ARG_TYPE_CONNECTIONID)) - .map_err(|e| FactoryError::VariableError(e.to_string()))?; - svc.add_variable(Arc::clone(&A_ARG_TYPE_CONNECTIONSTATUS)) - .map_err(|e| FactoryError::VariableError(e.to_string()))?; - svc.add_variable(Arc::clone(&A_ARG_TYPE_DIRECTION)) - .map_err(|e| FactoryError::VariableError(e.to_string()))?; - svc.add_variable(Arc::clone(&A_ARG_TYPE_PROTOCOLINFO)) - .map_err(|e| FactoryError::VariableError(e.to_string()))?; - svc.add_variable(Arc::clone(&A_ARG_TYPE_RCSID)) - .map_err(|e| FactoryError::VariableError(e.to_string()))?; - svc.add_variable(Arc::clone(&A_ARG_TYPE_AVTRANSPORTID)) - .map_err(|e| FactoryError::VariableError(e.to_string()))?; - svc.add_variable(Arc::clone(&CURRENTCONNECTIONIDS)) - .map_err(|e| FactoryError::VariableError(e.to_string()))?; - svc.add_variable(Arc::clone(&SINKPROTOCOLINFO)) - .map_err(|e| FactoryError::VariableError(e.to_string()))?; - svc.add_variable(Arc::clone(&SOURCEPROTOCOLINFO)) - .map_err(|e| FactoryError::VariableError(e.to_string()))?; + add_var(&mut svc, &A_ARG_TYPE_CONNECTIONID)?; + add_var(&mut svc, &A_ARG_TYPE_CONNECTIONSTATUS)?; + add_var(&mut svc, &A_ARG_TYPE_DIRECTION)?; + add_var(&mut svc, &A_ARG_TYPE_PROTOCOLINFO)?; + add_var(&mut svc, &A_ARG_TYPE_RCSID)?; + add_var(&mut svc, &A_ARG_TYPE_AVTRANSPORTID)?; + add_var(&mut svc, &CURRENTCONNECTIONIDS)?; + add_var(&mut svc, &SINKPROTOCOLINFO)?; + add_var(&mut svc, &SOURCEPROTOCOLINFO)?; // GetProtocolInfo let mut get_proto = Action::new("GetProtocolInfo".to_string());