From 54ea7b58130c784ec424d74530c64a16cac0ea4d Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sat, 4 Apr 2026 00:22:58 +0200 Subject: [PATCH 01/13] :sparkles: add async_trait support for UpnpApiExt trait and impl - Add `async-trait` dependency import - Annotate UpnpApiExt trait and its Server impl with `#[async_trait]` to enable async methods in traits --- pmoupnp/src/upnp_api.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pmoupnp/src/upnp_api.rs b/pmoupnp/src/upnp_api.rs index 02449464..9d2e4316 100644 --- a/pmoupnp/src/upnp_api.rs +++ b/pmoupnp/src/upnp_api.rs @@ -17,6 +17,7 @@ use axum::{ response::{IntoResponse, Json}, routing::get, }; +use async_trait::async_trait; use pmoserver::Server; use serde_json::json; use tracing::info; @@ -218,6 +219,7 @@ async fn get_service_variables( /// Trait d'extension pour enregistrer l'API UPnP sur un serveur. /// /// Similaire Ă  `WebAppExt` et `CoverCacheExt`. +#[async_trait] pub trait UpnpApiExt { /// Enregistre l'API REST d'introspection UPnP. /// @@ -229,6 +231,7 @@ pub trait UpnpApiExt { async fn register_upnp_api(&mut self); } +#[async_trait] impl UpnpApiExt for Server { async fn register_upnp_api(&mut self) { info!("📡 Registering UPnP introspection API..."); From 6352a43e27f2771accc6a4a28f6779bcd397d5be Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sat, 4 Apr 2026 00:30:46 +0200 Subject: [PATCH 02/13] correct compilation warning in pmoqobuz --- pmoqobuz/src/api/auth.rs | 6 +- pmoqobuz/src/api/catalog.rs | 6 +- pmoqobuz/src/api/user.rs | 2 +- pmoqobuz/src/client.rs | 2 +- pmoqobuz/src/client.rs-e | 1009 +++++++++++++++++++++++++++++++++++ 5 files changed, 1017 insertions(+), 8 deletions(-) create mode 100644 pmoqobuz/src/client.rs-e diff --git a/pmoqobuz/src/api/auth.rs b/pmoqobuz/src/api/auth.rs index 6893fabf..b810bf22 100644 --- a/pmoqobuz/src/api/auth.rs +++ b/pmoqobuz/src/api/auth.rs @@ -18,11 +18,11 @@ struct UserInfo { #[serde(deserialize_with = "crate::models::deserialize_id")] id: String, #[serde(default)] - email: Option, + _email: Option, #[serde(default)] - firstname: Option, + _firstname: Option, #[serde(default)] - lastname: Option, + _lastname: Option, credential: CredentialInfo, } diff --git a/pmoqobuz/src/api/catalog.rs b/pmoqobuz/src/api/catalog.rs index f93f750a..b0a3b10d 100644 --- a/pmoqobuz/src/api/catalog.rs +++ b/pmoqobuz/src/api/catalog.rs @@ -13,9 +13,9 @@ struct PaginatedResponse { #[serde(default)] total: Option, #[serde(default)] - limit: Option, + _limit: Option, #[serde(default)] - offset: Option, + _offset: Option, } /// RĂ©ponse de l'endpoint /album/get @@ -91,7 +91,7 @@ struct ImageResponse { /// RĂ©ponse genre #[derive(Debug, Deserialize)] -struct GenreResponse { +pub(crate) struct GenreResponse { #[serde(default)] id: Option, name: String, diff --git a/pmoqobuz/src/api/user.rs b/pmoqobuz/src/api/user.rs index cfa8cbfe..c8b23ced 100644 --- a/pmoqobuz/src/api/user.rs +++ b/pmoqobuz/src/api/user.rs @@ -15,7 +15,7 @@ struct PaginatedResponse { /// RĂ©ponse de l'endpoint /favorite/getUserFavorites #[derive(Debug, Deserialize)] -pub(crate) struct FavoritesResponse { +pub struct FavoritesResponse { #[serde(default)] albums: Option>, #[serde(default)] diff --git a/pmoqobuz/src/client.rs b/pmoqobuz/src/client.rs index 8c3299c1..4ad8b8e4 100644 --- a/pmoqobuz/src/client.rs +++ b/pmoqobuz/src/client.rs @@ -260,7 +260,7 @@ impl QobuzClient { // Optimization: Login once with first secret to get auth token // Then test all secrets using the same token - if let Some((first_timezone, first_secret)) = secrets.first() { + if let Some((_first_timezone, first_secret)) = secrets.first() { if let Ok(temp_api) = QobuzApi::with_raw_secret(&app_id, first_secret) { diff --git a/pmoqobuz/src/client.rs-e b/pmoqobuz/src/client.rs-e new file mode 100644 index 00000000..8c3299c1 --- /dev/null +++ b/pmoqobuz/src/client.rs-e @@ -0,0 +1,1009 @@ +//! Client principal pour interagir avec l'API Qobuz +//! +//! Ce module fournit un client haut-niveau avec authentification et cache intĂ©grĂ©. + +use crate::api::auth::AuthInfo; +use crate::api::{QobuzApi, DEFAULT_APP_ID}; +use crate::cache::QobuzCache; +use crate::config_ext::QobuzConfigExt; +use crate::error::{QobuzError, Result}; +use crate::models::*; +use pmoconfig::{self, Config}; +use std::future::Future; +use std::sync::{Arc, Mutex}; +use tracing::{debug, info, warn}; + +/// Client Qobuz haut-niveau avec cache +pub struct QobuzClient { + /// API bas-niveau + api: QobuzApi, + /// Cache en mĂ©moire + cache: Arc, + /// Informations d'authentification + auth_info: Mutex>, + /// Identifiants utilisateur pour relogin automatique + credentials: Option<(String, String)>, + /// Configuration partagĂ©e (pour persister les tokens) + config: Option>, + #[cfg(feature = "disk-cache")] + /// Cache disque optionnel + disk_cache: Option>, +} + +impl QobuzClient { + fn build_client( + api: QobuzApi, + auth_info: Option, + credentials: Option<(String, String)>, + config: Option>, + ) -> Self { + if let Some(info) = &auth_info { + api.set_auth_token(info.token.clone(), info.user_id.clone()); + } + + Self { + api, + cache: Arc::new(QobuzCache::new()), + auth_info: Mutex::new(auth_info), + credentials, + config, + #[cfg(feature = "disk-cache")] + disk_cache: None, + } + } + + /// CrĂ©e un nouveau client et authentifie avec les credentials fournis + /// + /// # Arguments + /// + /// * `username` - Email ou nom d'utilisateur Qobuz + /// * `password` - Mot de passe + /// + /// # Exemple + /// + /// ```rust,no_run + /// use pmoqobuz::QobuzClient; + /// + /// #[tokio::main] + /// async fn main() -> anyhow::Result<()> { + /// let client = QobuzClient::new("user@example.com", "password").await?; + /// Ok(()) + /// } + /// ``` + pub async fn new(username: &str, password: &str) -> Result { + Self::with_app_id(DEFAULT_APP_ID, username, password).await + } + + /// CrĂ©e un nouveau client avec un App ID personnalisĂ© + pub async fn with_app_id(app_id: &str, username: &str, password: &str) -> Result { + info!("Creating Qobuz client with app ID: {}", app_id); + + let api = QobuzApi::new(app_id)?; + let auth_info = api.login(username, password).await?; + + let client = Self::build_client( + api, + Some(auth_info), + Some((username.to_string(), password.to_string())), + None, + ); + + Ok(client.finalize_disk_cache().await) + } + + /// CrĂ©e un client en utilisant la configuration de pmoconfig + /// + /// # Exemple + /// + /// ```rust,no_run + /// use pmoqobuz::QobuzClient; + /// + /// #[tokio::main] + /// async fn main() -> anyhow::Result<()> { + /// let client = QobuzClient::from_config().await?; + /// Ok(()) + /// } + /// ``` + pub async fn from_config() -> Result { + let config = pmoconfig::get_config(); + Self::from_config_obj(config.as_ref()).await + } + + /// CrĂ©e un client depuis un objet Config spĂ©cifique + /// + /// Cette mĂ©thode rĂ©cupĂšre les credentials, l'App ID et optionnellement + /// le secret depuis la configuration. + /// + /// Ordre de prioritĂ© pour l'initialisation : + /// 0. **VĂ©rifier le cache du token d'authentification** (Ă©vite un login si token valide) + /// 1. Si `appid` ET `secret` configurĂ©s → teste d'abord avec ces credentials + /// 2. Si Ă©chec d'authentification → utilise le Spoofer pour obtenir de nouveaux credentials + /// 3. Si aucun `appid`/`secret` configurĂ© → utilise directement le Spoofer + /// 4. Fallback ultime → utilise DEFAULT_APP_ID sans secret (requĂȘtes signĂ©es Ă©choueront) + pub async fn from_config_obj(config: &Config) -> Result { + let (username, password) = config.get_qobuz_credentials()?; + let credentials = (username.clone(), password.clone()); + let config_arc = Arc::new(config.clone()); + + let config_appid = config.get_qobuz_appid()?; + let config_secret = config.get_qobuz_secret()?; + let config_spoofer_secret = config.get_qobuz_spoofer_secret()?; + + let mut used_config_credentials = false; + + let mut api = match (config_appid.clone(), config_spoofer_secret, config_secret) { + // Priority 1: Try memorized Spoofer secret (raw, no XOR) + (Some(app_id), Some(spoofer_secret), _) => { + info!("Trying memorized Spoofer secret with App ID: {}", app_id); + match QobuzApi::with_raw_secret(&app_id, &spoofer_secret) { + Ok(api) => { + used_config_credentials = true; + api + } + Err(e) => { + info!( + "✗ Memorized Spoofer secret failed: {}. Re-fetching from Spoofer...", + e + ); + Self::try_spoofer_fallback(config).await? + } + } + } + // Priority 2: Try XOR secret (legacy configvalue) + (Some(app_id), None, Some(secret)) => { + info!( + "Creating Qobuz API with configured App ID: {} and XOR secret", + app_id + ); + match QobuzApi::with_secret(&app_id, &secret) { + Ok(api) => { + used_config_credentials = true; + api + } + Err(e) => { + info!( + "✗ Failed to create API with configured credentials: {}. Falling back to Spoofer...", + e + ); + Self::try_spoofer_fallback(config).await? + } + } + } + // Priority 3: Fallback to Spoofer + _ => { + info!("AppID or secret not configured, using Spoofer..."); + Self::try_spoofer_fallback(config).await? + } + }; + + if config.is_qobuz_auth_valid() { + match (config.get_qobuz_auth_token(), config.get_qobuz_user_id()) { + (Ok(Some(token)), Ok(Some(user_id))) + if !token.is_empty() && !user_id.is_empty() => + { + info!("✓ Reusing authentication token (optimistic, no login)"); + api.set_auth_token(token.clone(), user_id.clone()); + + let auth_info = AuthInfo { + token, + user_id, + subscription_label: config.get_qobuz_subscription_label().ok().flatten(), + }; + + let client = Self::build_client( + api, + Some(auth_info), + Some(credentials.clone()), + Some(config_arc.clone()), + ); + + return Ok(client.finalize_disk_cache().await); + } + _ => { + debug!( + "Auth marked valid in config but token/user_id missing or invalid, performing login" + ); + } + } + } + + // Authentifier l'utilisateur + let mut auth_result = api.login(&username, &password).await; + + if used_config_credentials { + if let Err(err) = &auth_result { + if err.is_auth_error() { + info!("✗ Configured credentials failed authentication: {}", err); + info!("→ Falling back to Spoofer to obtain new credentials..."); + api = Self::try_spoofer_fallback(config).await?; + auth_result = api.login(&username, &password).await; + } + } + } + + let auth_info = auth_result?; + + Self::persist_auth_info(config, &auth_info); + + let client = Self::build_client(api, Some(auth_info), Some(credentials), Some(config_arc)); + + Ok(client.finalize_disk_cache().await) + } + + /// Tente d'utiliser le Spoofer pour obtenir des credentials valides + /// + /// Cette mĂ©thode est appelĂ©e soit : + /// - Quand aucun appid/secret n'est configurĂ© + /// - Quand les credentials configurĂ©s sont invalides/expirĂ©s + async fn try_spoofer_fallback(config: &Config) -> Result { + if let Some((app_id, secret)) = Self::fetch_spoofer_credentials(config).await? { + // Use raw secret from Spoofer (no XOR) + return QobuzApi::with_raw_secret(app_id, &secret); + } + + info!( + "✗ No valid secret found from Spoofer, falling back to DEFAULT_APP_ID without secret" + ); + QobuzApi::new(DEFAULT_APP_ID) + } + + async fn fetch_spoofer_credentials(config: &Config) -> Result> { + match crate::api::Spoofer::new().await { + Ok(spoofer) => match spoofer.get_app_id() { + Ok(app_id) => { + // Use timezone secrets (like Python) + // Note: App Secret from bundle doesn't work for signed requests + match spoofer.get_secrets() { + Ok(secrets) => { + info!("Testing {} timezone secret(s)...", secrets.len()); + let (username, password) = config.get_qobuz_credentials()?; + + // Optimization: Login once with first secret to get auth token + // Then test all secrets using the same token + if let Some((first_timezone, first_secret)) = secrets.first() { + if let Ok(temp_api) = + QobuzApi::with_raw_secret(&app_id, first_secret) + { + if let Ok(_auth_info) = + temp_api.login(&username, &password).await + { + // Now test each secret with the authenticated token + for (timezone, secret) in secrets.iter() { + debug!("Testing timezone secret: {}", timezone); + + if let Ok(test_api) = + QobuzApi::with_raw_secret(&app_id, secret) + { + // Set the auth token from our initial login + test_api.set_auth_token( + temp_api.auth_token().unwrap(), + temp_api.user_id().unwrap(), + ); + + // Test the secret using track/getFileUrl (like qobuz-player-client) + // Use the same hardcoded track_id (64868955) as qobuz-player-client + if test_api.get_file_url("64868955").await.is_ok() { + info!( + "✓ Secret from timezone '{}' works!", + timezone + ); + + // Save both appid and the working secret + if let Err(e) = config.set_qobuz_appid(&app_id) + { + debug!("Could not save appid: {}", e); + } + if let Err(e) = + config.set_qobuz_spoofer_secret(secret) + { + debug!( + "Could not save spoofer secret: {}", + e + ); + } + + return Ok(Some(( + app_id.clone(), + secret.clone(), + ))); + } else { + debug!("✗ Secret from timezone '{}' failed track/getFileUrl test", timezone); + } + } + } + } + } + } + + info!("✗ No valid secret found"); + Ok(None) + } + Err(e) => { + info!("Failed to extract timezone secrets: {}", e); + Ok(None) + } + } + } + Err(e) => { + info!( + "Spoofer failed to extract app_id: {}, falling back to DEFAULT_APP_ID", + e + ); + Ok(None) + } + }, + Err(e) => { + info!( + "Spoofer failed: {}, falling back to DEFAULT_APP_ID without secret", + e + ); + Ok(None) + } + } + } + + async fn refresh_credentials_via_spoofer(&self) -> Result<()> { + let config_arc = match &self.config { + Some(cfg) => cfg.clone(), + None => pmoconfig::get_config(), + }; + + match Self::fetch_spoofer_credentials(config_arc.as_ref()).await? { + Some((app_id, secret)) => { + // Use raw secret (no XOR) from Spoofer + self.api.update_credentials_raw(app_id, &secret); + info!("✓ Updated Qobuz API credentials using Spoofer"); + Ok(()) + } + None => Err(QobuzError::Configuration( + "Unable to refresh Qobuz credentials via Spoofer".to_string(), + )), + } + } + + /// DĂ©finit le format audio par dĂ©faut + pub fn set_format(&mut self, format: AudioFormat) { + self.api.set_format(format); + } + + /// Retourne le format audio configurĂ© + pub fn format(&self) -> AudioFormat { + self.api.format() + } + + /// Retourne les informations d'authentification + pub fn auth_info(&self) -> Option { + self.auth_info.lock().unwrap().clone() + } + + #[cfg(feature = "disk-cache")] + fn user_id(&self) -> Option { + self.auth_info + .lock() + .unwrap() + .as_ref() + .map(|info| info.user_id.clone()) + } + + /// Retourne une rĂ©fĂ©rence au cache + pub fn cache(&self) -> Arc { + self.cache.clone() + } + + #[cfg(feature = "disk-cache")] + pub async fn purge_disk_cache(&self) -> Result { + if let Some(disk) = &self.disk_cache { + disk.purge_expired() + .await + .map_err(|err| QobuzError::Cache(err.to_string())) + } else { + Ok(0) + } + } + + #[cfg(feature = "disk-cache")] + pub fn with_disk_cache(mut self, store: Arc) -> Self { + self.disk_cache = Some(store); + self + } + + #[cfg(feature = "disk-cache")] + fn attach_default_disk_cache(mut self) -> Self { + if let Some(store) = Self::default_disk_cache_store(self.config.as_deref()) { + self = self.with_disk_cache(store); + } + self + } + + #[cfg(not(feature = "disk-cache"))] + fn attach_default_disk_cache(self) -> Self { + self + } + + #[cfg(feature = "disk-cache")] + async fn finalize_disk_cache(self) -> Self { + let client = self.attach_default_disk_cache(); + if let Err(err) = client.purge_disk_cache().await { + debug!("Failed to purge disk cache on startup: {}", err); + } + client + } + + #[cfg(not(feature = "disk-cache"))] + async fn finalize_disk_cache(self) -> Self { + self.attach_default_disk_cache() + } + + #[cfg(feature = "disk-cache")] + fn default_disk_cache_store( + config: Option<&Config>, + ) -> Option> { + let cache_dir = match config { + Some(cfg) => match cfg.get_qobuz_cache_dir() { + Ok(dir) => dir, + Err(err) => { + debug!("Failed to read qobuz cache dir from config: {}", err); + return None; + } + }, + None => { + let global = pmoconfig::get_config(); + match global.get_qobuz_cache_dir() { + Ok(dir) => dir, + Err(err) => { + debug!("Failed to read qobuz cache dir from global config: {}", err); + return None; + } + } + } + }; + + let dir_path = std::path::PathBuf::from(&cache_dir); + if let Err(err) = std::fs::create_dir_all(&dir_path) { + debug!( + "Failed to create disk cache directory {}: {}", + dir_path.display(), + err + ); + return None; + } + + let db_path = dir_path.join("qobuz_cache.sqlite"); + + match crate::disk_cache::SqliteCacheStore::new(db_path) { + Ok(store) => { + let store: Arc = Arc::new(store); + Some(store) + } + Err(err) => { + debug!("Failed to initialize SQLite disk cache: {}", err); + None + } + } + } + + async fn call_with_auth_repair(&self, operation: &str, mut op: F) -> Result + where + F: FnMut() -> Fut, + Fut: Future> + Send, + { + match op().await { + Ok(result) => Ok(result), + Err(err) if err.is_signature_error() => { + warn!( + "Request signature error during {}. Qobuz app secret may be outdated.", + operation + ); + if let Err(refresh_err) = self.refresh_credentials_via_spoofer().await { + warn!( + "Failed to refresh Qobuz credentials automatically: {}", + refresh_err + ); + Err(err) + } else { + info!( + "Successfully refreshed Qobuz credentials. Retrying {}...", + operation + ); + op().await + } + } + Err(err) if err.is_auth_error() => { + info!( + "Authentication error during {}. Attempting automatic repair...", + operation + ); + self.repair_auth().await?; + op().await + } + Err(err) => Err(err), + } + } + + async fn repair_auth(&self) -> Result<()> { + let (username, password) = self.credentials.as_ref().cloned().ok_or_else(|| { + QobuzError::Unauthorized( + "Cannot repair authentication without stored credentials".to_string(), + ) + })?; + + let auth_info = self.api.login(&username, &password).await?; + + if let Some(config) = &self.config { + Self::persist_auth_info(config.as_ref(), &auth_info); + } + + let mut guard = self.auth_info.lock().unwrap(); + *guard = Some(auth_info); + Ok(()) + } + + fn persist_auth_info(config: &Config, auth_info: &AuthInfo) { + use std::time::{Duration, SystemTime, UNIX_EPOCH}; + + let expires_at = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_secs() + + Duration::from_secs(24 * 3600).as_secs(); // 24h + + if let Err(e) = config.set_qobuz_auth_info( + &auth_info.token, + &auth_info.user_id, + auth_info.subscription_label.as_deref(), + expires_at, + ) { + debug!("Failed to save authentication to config: {}", e); + } else { + info!("✓ Saved authentication token to configuration"); + } + } + + // ============ Albums ============ + + /// RĂ©cupĂšre un album par son ID + pub async fn get_album(&self, album_id: &str) -> Result { + // VĂ©rifier le cache d'abord + if let Some(album) = self.cache.get_album(album_id).await { + debug!("Album {} found in cache", album_id); + return Ok(album); + } + + // Sinon, rĂ©cupĂ©rer depuis l'API + let album = self + .call_with_auth_repair("get_album", || self.api.get_album(album_id)) + .await?; + + // Mettre en cache + self.cache + .put_album(album_id.to_string(), album.clone()) + .await; + + Ok(album) + } + + /// RĂ©cupĂšre les tracks d'un album + pub async fn get_album_tracks(&self, album_id: &str) -> Result> { + // VĂ©rifier le cache d'abord + if let Some(tracks) = self.cache.get_album_tracks(album_id).await { + debug!("Album tracks for {} found in cache", album_id); + return Ok(tracks); + } + + // Sinon, rĂ©cupĂ©rer depuis l'API + let tracks = self + .call_with_auth_repair("get_album_tracks", || self.api.get_album_tracks(album_id)) + .await?; + + // Mettre les tracks en cache (individuellement ET la liste complĂšte) + for track in &tracks { + self.cache.put_track(track.id.clone(), track.clone()).await; + } + self.cache + .put_album_tracks(album_id.to_string(), tracks.clone()) + .await; + + Ok(tracks) + } + + // ============ Tracks ============ + + /// RĂ©cupĂšre une track par son ID + pub async fn get_track(&self, track_id: &str) -> Result { + if let Some(track) = self.cache.get_track(track_id).await { + debug!("Track {} found in cache", track_id); + return Ok(track); + } + + let track = self + .call_with_auth_repair("get_track", || self.api.get_track(track_id)) + .await?; + self.cache + .put_track(track_id.to_string(), track.clone()) + .await; + + Ok(track) + } + + /// RĂ©cupĂšre l'URL de streaming d'une track + pub async fn get_stream_url(&self, track_id: &str) -> Result { + // VĂ©rifier le cache d'abord + if let Some(info) = self.cache.get_stream_url(track_id).await { + if info.expires_at > chrono::Utc::now() { + debug!("Stream URL for track {} found in cache", track_id); + return Ok(info.url); + } + } + + // Sinon, rĂ©cupĂ©rer depuis l'API + let info = self + .call_with_auth_repair("get_file_url", || self.api.get_file_url(track_id)) + .await?; + let url = info.url.clone(); + + // Mettre en cache + self.cache.put_stream_url(track_id.to_string(), info).await; + + Ok(url) + } + + // ============ Artists ============ + + /// RĂ©cupĂšre un artiste par son ID + pub async fn get_artist(&self, artist_id: &str) -> Result { + if let Some(artist) = self.cache.get_artist(artist_id).await { + debug!("Artist {} found in cache", artist_id); + return Ok(artist); + } + + // Pour rĂ©cupĂ©rer un artiste, on doit passer par get_artist_albums + let albums = self + .call_with_auth_repair("get_artist_albums_for_artist", || { + self.api.get_artist_albums(artist_id) + }) + .await?; + + if let Some(first_album) = albums.first() { + let artist = first_album.artist.clone(); + self.cache + .put_artist(artist_id.to_string(), artist.clone()) + .await; + Ok(artist) + } else { + Err(QobuzError::NotFound(format!( + "Artist {} not found", + artist_id + ))) + } + } + + /// RĂ©cupĂšre les albums d'un artiste + pub async fn get_artist_albums(&self, artist_id: &str) -> Result> { + self.call_with_auth_repair("get_artist_albums", || { + self.api.get_artist_albums(artist_id) + }) + .await + } + + /// RĂ©cupĂšre les artistes similaires + pub async fn get_similar_artists(&self, artist_id: &str) -> Result> { + self.call_with_auth_repair("get_similar_artists", || { + self.api.get_similar_artists(artist_id) + }) + .await + } + + // ============ Playlists ============ + + /// RĂ©cupĂšre une playlist par son ID + pub async fn get_playlist(&self, playlist_id: &str) -> Result { + if let Some(playlist) = self.cache.get_playlist(playlist_id).await { + debug!("Playlist {} found in cache", playlist_id); + return Ok(playlist); + } + + let playlist = self + .call_with_auth_repair("get_playlist", || self.api.get_playlist(playlist_id)) + .await?; + self.cache + .put_playlist(playlist_id.to_string(), playlist.clone()) + .await; + + Ok(playlist) + } + + /// RĂ©cupĂšre les tracks d'une playlist + pub async fn get_playlist_tracks(&self, playlist_id: &str) -> Result> { + self.call_with_auth_repair("get_playlist_tracks", || { + self.api.get_playlist_tracks(playlist_id) + }) + .await + } + + // ============ Catalogue ============ + + /// RĂ©cupĂšre la liste des genres + pub async fn get_genres(&self) -> Result> { + self.call_with_auth_repair("get_genres", || self.api.get_genres()) + .await + } + + /// RĂ©cupĂšre les albums featured (nouveautĂ©s, Ă©diteur, etc.) + pub async fn get_featured_albums( + &self, + genre_id: Option<&str>, + type_: &str, + ) -> Result> { + self.call_with_auth_repair("get_featured_albums", || { + self.api.get_featured_albums(genre_id, type_) + }) + .await + } + + /// RĂ©cupĂšre les playlists featured + pub async fn get_featured_playlists( + &self, + genre_id: Option<&str>, + tags: Option<&str>, + ) -> Result> { + self.call_with_auth_repair("get_featured_playlists", || { + self.api.get_featured_playlists(genre_id, tags) + }) + .await + } + + /// RĂ©cupĂšre les artistes featured + pub async fn get_featured_artists( + &self, + genre_id: Option<&str>, + limit: Option, + offset: Option, + ) -> Result> { + self.call_with_auth_repair("get_featured_artists", || { + self.api.get_featured_artists(genre_id, limit, offset) + }) + .await + } + + // ============ Recherche ============ + + /// Recherche dans le catalogue Qobuz + /// + /// # Arguments + /// + /// * `query` - Termes de recherche + /// * `type_` - Type de recherche : None (tous), Some("albums"), Some("artists"), Some("tracks"), Some("playlists") + pub async fn search(&self, query: &str, type_: Option<&str>) -> Result { + // CrĂ©er une clĂ© de cache + let cache_key = format!("{}:{}", query, type_.unwrap_or("all")); + + // VĂ©rifier le cache + if let Some(result) = self.cache.get_search(&cache_key).await { + debug!("Search results for '{}' found in cache", query); + return Ok(result); + } + + // Sinon, rechercher via l'API + let result = self + .call_with_auth_repair("search", || self.api.search(query, type_)) + .await?; + + // Mettre en cache + self.cache.put_search(cache_key, result.clone()).await; + + Ok(result) + } + + /// Recherche des albums + pub async fn search_albums(&self, query: &str) -> Result> { + let result = self.search(query, Some("albums")).await?; + Ok(result.albums) + } + + /// Recherche des artistes + pub async fn search_artists(&self, query: &str) -> Result> { + let result = self.search(query, Some("artists")).await?; + Ok(result.artists) + } + + /// Recherche des tracks + pub async fn search_tracks(&self, query: &str) -> Result> { + let result = self.search(query, Some("tracks")).await?; + Ok(result.tracks) + } + + /// Recherche des playlists + pub async fn search_playlists(&self, query: &str) -> Result> { + let result = self.search(query, Some("playlists")).await?; + Ok(result.playlists) + } + + // ============ Favoris ============ + + /// RĂ©cupĂšre les albums favoris de l'utilisateur + pub async fn get_favorite_albums(&self) -> Result> { + #[cfg(feature = "disk-cache")] + let user_id = self + .user_id() + .ok_or_else(|| QobuzError::Unauthorized("Missing authenticated user ID".into()))?; + + #[cfg(feature = "disk-cache")] + let ttl = std::time::Duration::from_secs(6 * 3600); + + #[cfg(feature = "disk-cache")] + if let Some(disk) = &self.disk_cache { + if let Some(entry) = disk + .get_json::>(&user_id, "favorites_albums", "all") + .await? + { + if entry.fresh { + return Ok(entry.value); + } + } + } + + let albums = self + .call_with_auth_repair("get_favorite_albums", || self.api.get_favorite_albums()) + .await?; + + #[cfg(feature = "disk-cache")] + if let Some(disk) = &self.disk_cache { + let _ = disk + .put_json(&user_id, "favorites_albums", "all", ttl, &albums) + .await; + } + + Ok(albums) + } + + /// RĂ©cupĂšre les artistes favoris de l'utilisateur + pub async fn get_favorite_artists(&self) -> Result> { + self.call_with_auth_repair("get_favorite_artists", || self.api.get_favorite_artists()) + .await + } + + /// RĂ©cupĂšre les tracks favorites de l'utilisateur + pub async fn get_favorite_tracks(&self) -> Result> { + #[cfg(feature = "disk-cache")] + let user_id = self + .user_id() + .ok_or_else(|| QobuzError::Unauthorized("Missing authenticated user ID".into()))?; + + #[cfg(feature = "disk-cache")] + let ttl = std::time::Duration::from_secs(6 * 3600); + + #[cfg(feature = "disk-cache")] + if let Some(disk) = &self.disk_cache { + if let Some(entry) = disk + .get_json::>(&user_id, "favorites_tracks", "all") + .await? + { + if entry.fresh { + return Ok(entry.value); + } + } + } + + let tracks = self + .call_with_auth_repair("get_favorite_tracks", || self.api.get_favorite_tracks()) + .await?; + + #[cfg(feature = "disk-cache")] + if let Some(disk) = &self.disk_cache { + let _ = disk + .put_json(&user_id, "favorites_tracks", "all", ttl, &tracks) + .await; + } + + Ok(tracks) + } + + /// RĂ©cupĂšre les playlists de l'utilisateur + pub async fn get_user_playlists(&self) -> Result> { + #[cfg(feature = "disk-cache")] + let user_id = self + .user_id() + .ok_or_else(|| QobuzError::Unauthorized("Missing authenticated user ID".into()))?; + + #[cfg(feature = "disk-cache")] + let ttl = std::time::Duration::from_secs(6 * 3600); + + #[cfg(feature = "disk-cache")] + if let Some(disk) = &self.disk_cache { + if let Some(entry) = disk + .get_json::>(&user_id, "user_playlists", "all") + .await? + { + if entry.fresh { + return Ok(entry.value); + } + } + } + + let playlists = self + .call_with_auth_repair("get_user_playlists", || self.api.get_user_playlists()) + .await?; + + #[cfg(feature = "disk-cache")] + if let Some(disk) = &self.disk_cache { + let _ = disk + .put_json(&user_id, "user_playlists", "all", ttl, &playlists) + .await; + } + + Ok(playlists) + } + + /// Ajoute un album aux favoris + pub async fn add_favorite_album(&self, album_id: &str) -> Result<()> { + self.call_with_auth_repair("add_favorite_album", || { + self.api.add_favorite_album(album_id) + }) + .await + } + + /// Supprime un album des favoris + pub async fn remove_favorite_album(&self, album_id: &str) -> Result<()> { + self.call_with_auth_repair("remove_favorite_album", || { + self.api.remove_favorite_album(album_id) + }) + .await + } + + /// Ajoute un track aux favoris + pub async fn add_favorite_track(&self, track_id: &str) -> Result<()> { + self.call_with_auth_repair("add_favorite_track", || { + self.api.add_favorite_track(track_id) + }) + .await + } + + /// Supprime un track des favoris + pub async fn remove_favorite_track(&self, track_id: &str) -> Result<()> { + self.call_with_auth_repair("remove_favorite_track", || { + self.api.remove_favorite_track(track_id) + }) + .await + } + + /// Ajoute un track Ă  une playlist + pub async fn add_to_playlist(&self, playlist_id: &str, track_id: &str) -> Result<()> { + self.call_with_auth_repair("add_to_playlist", || { + self.api.add_to_playlist(playlist_id, track_id) + }) + .await + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config_ext::QobuzConfigExt; + + #[test] + fn test_audio_format() { + assert_eq!(AudioFormat::default(), AudioFormat::Flac_Lossless); + } + + #[tokio::test] + async fn from_config_reuses_token_without_login() { + let temp_dir = tempfile::tempdir().unwrap(); + let config_path = temp_dir.path().to_string_lossy().to_string(); + let config = pmoconfig::Config::load_config(&config_path).unwrap(); + + config.set_qobuz_username("user@example.com").unwrap(); + config.set_qobuz_password("password").unwrap(); + config.set_qobuz_appid("1401488693436528").unwrap(); + // base64 for "secret" + config.set_qobuz_secret("c2VjcmV0").unwrap(); + config + .set_qobuz_auth_info("token123", "user123", Some("Hi-Fi"), 1_700_000_000) + .unwrap(); + + let client = QobuzClient::from_config_obj(&config).await.unwrap(); + let auth_info = client.auth_info().expect("auth info"); + + assert_eq!(auth_info.token, "token123"); + assert_eq!(auth_info.user_id, "user123"); + } +} From d81e7a94134f0ccb51efa65d2d45e02a9734c345 Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sat, 4 Apr 2026 00:34:55 +0200 Subject: [PATCH 03/13] :sparkles!: add async-trait dependency for WebAppExt trait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add `async_trait` as an optional䟝蔖ency in pmoapp/Cargo.toml - Enable usage of `#[async_trait]` on WebAppExt trait and its impl for Server - Guard async-related code with `#[cfg(feature = "pmoserver")]` --- Cargo.lock | 1 + pmoapp/Cargo.toml | 6 +++++- pmoapp/src/lib.rs | 4 ++++ pmoapp/src/pmoserver_impl.rs | 4 ++++ 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 6d254f59..9d5c22f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3848,6 +3848,7 @@ checksum = "7edddbd0b52d732b21ad9a5fab5c704c14cd949e5e9a1ec5929a24fded1b904c" name = "pmoapp" version = "0.1.0" dependencies = [ + "async-trait", "pmoserver", "rust-embed", ] diff --git a/pmoapp/Cargo.toml b/pmoapp/Cargo.toml index 8b18b4e2..b5c1e0cf 100644 --- a/pmoapp/Cargo.toml +++ b/pmoapp/Cargo.toml @@ -10,6 +10,10 @@ rust-embed = "8.5.0" path = "../pmoserver" optional = true +[dependencies.async-trait] +optional = true +version = "0.1" + [features] default = [] -pmoserver = ["dep:pmoserver"] +pmoserver = ["dep:pmoserver", "dep:async-trait"] diff --git a/pmoapp/src/lib.rs b/pmoapp/src/lib.rs index b527636f..e7213067 100755 --- a/pmoapp/src/lib.rs +++ b/pmoapp/src/lib.rs @@ -237,6 +237,8 @@ //! - [Vue.js Documentation](https://vuejs.org/) //! - [Vite Documentation](https://vitejs.dev/) +#[cfg(feature = "pmoserver")] +use async_trait::async_trait; use rust_embed::RustEmbed; /// Structure reprĂ©sentant l'application web embarquĂ©e. @@ -285,6 +287,8 @@ pub struct Webapp; /// } /// } /// ``` +#[cfg(feature = "pmoserver")] +#[async_trait] pub trait WebAppExt { /// Ajoute une Single Page Application au serveur. /// diff --git a/pmoapp/src/pmoserver_impl.rs b/pmoapp/src/pmoserver_impl.rs index bbbbdd41..e083ddb4 100644 --- a/pmoapp/src/pmoserver_impl.rs +++ b/pmoapp/src/pmoserver_impl.rs @@ -27,10 +27,14 @@ //! # } //! ``` +#[cfg(feature = "pmoserver")] +use async_trait::async_trait; use crate::WebAppExt; use pmoserver::Server; use rust_embed::RustEmbed; +#[cfg(feature = "pmoserver")] +#[async_trait] impl WebAppExt for Server { async fn add_webapp(&mut self, path: &str) where From 34260e7cbd4e8673e67fab2668ba5c18df439e0f Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sat, 4 Apr 2026 00:54:13 +0200 Subject: [PATCH 04/13] :wrench: fix unused variable warning and suppress must-use lint in macros - Prefix `_object_id` parameter with underscore to silence unused variable warning in `get_item` - Add #[allow(unused_must_use)] to all action macro definitions in pmoupnp/macros.rs - Remove redundant doc comment line break --- pmosource/src/lib.rs | 2 +- pmoupnp/src/actions/macros.rs | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pmosource/src/lib.rs b/pmosource/src/lib.rs index ec80398f..4acb903e 100644 --- a/pmosource/src/lib.rs +++ b/pmosource/src/lib.rs @@ -443,7 +443,7 @@ pub trait MusicSource: Debug + Send + Sync { /// let item = source.get_item("track-123").await?; /// println!("Now playing: {} by {}", item.title, item.artist.unwrap_or_default()); /// ``` - async fn get_item(&self, object_id: &str) -> Result { + async fn get_item(&self, _object_id: &str) -> Result { // Default implementation: try to find it in parent's browse result // This is inefficient and should be overridden by implementations Err(MusicSourceError::NotSupported( diff --git a/pmoupnp/src/actions/macros.rs b/pmoupnp/src/actions/macros.rs index f88cb0cf..8ab665a6 100644 --- a/pmoupnp/src/actions/macros.rs +++ b/pmoupnp/src/actions/macros.rs @@ -97,7 +97,6 @@ /// ``` /// /// # Notes d'implĂ©mentation -/// /// - Les `Arc` sont clonĂ©s (shallow copy du pointeur) /// - Chaque `Argument` est wrappĂ© dans un `Arc` /// - L'`Action` finale est wrappĂ©e dans un `Arc` @@ -112,6 +111,7 @@ macro_rules! define_action { } $(with handler $handler:expr)? ) => { + #[allow(unused_must_use)] pub static $name: once_cell::sync::Lazy> = once_cell::sync::Lazy::new(|| { let mut ac = $crate::actions::Action::new($action_name.to_string()); @@ -135,6 +135,7 @@ macro_rules! define_action { (pub static $name:ident = $action_name:literal stateless $(with handler $handler:expr)? ) => { + #[allow(unused_must_use)] pub static $name: once_cell::sync::Lazy> = once_cell::sync::Lazy::new(|| { let mut ac = $crate::actions::Action::new($action_name.to_string()); @@ -156,6 +157,7 @@ macro_rules! define_action { } $(with handler $handler:expr)? ) => { + #[allow(unused_must_use)] pub static $name: once_cell::sync::Lazy> = once_cell::sync::Lazy::new(|| { let mut ac = $crate::actions::Action::new($action_name.to_string()); @@ -178,6 +180,7 @@ macro_rules! define_action { (pub static $name:ident = $action_name:literal $(with handler $handler:expr)? ) => { + #[allow(unused_must_use)] pub static $name: once_cell::sync::Lazy> = once_cell::sync::Lazy::new(|| { let mut ac = $crate::actions::Action::new($action_name.to_string()); From 77d0d73ed78ea5fab9243082dc248cab67382c08 Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sat, 4 Apr 2026 00:57:33 +0200 Subject: [PATCH 05/13] :wastebasket: Remove unused imports and format code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Comment out `use std::simd::*` in pmoaudio as it's unused and modules import their own SIMD - Remove `Instant` from imports in track_metadata.rs (unused) - Reorder anyhow import to match Rust convention (`anyhow, Result` → `Result`) and remove unused imports - Improve XML response logging readability with line breaks in iterator chain + Refactor `get()` method to multi-line for clarity and consistency --- pmoaudio/src/lib.rs | 2 +- pmoaudiocache/src/track_metadata.rs | 2 +- pmocontrol/src/upnp_clients/openhome_client.rs | 17 +++++++++++------ 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/pmoaudio/src/lib.rs b/pmoaudio/src/lib.rs index 43b9568c..5a2bfd72 100755 --- a/pmoaudio/src/lib.rs +++ b/pmoaudio/src/lib.rs @@ -78,7 +78,7 @@ async fn main() { - **RwLock** : Pour partage concurrent du compteur [`TimerNode`] "#] #[cfg(feature = "simd")] -use std::simd::*; +// use std::simd::*; // Not actually used in this file, modules import their own simd mod audio_chunk; mod audio_segment; diff --git a/pmoaudiocache/src/track_metadata.rs b/pmoaudiocache/src/track_metadata.rs index 582748fb..49558293 100644 --- a/pmoaudiocache/src/track_metadata.rs +++ b/pmoaudiocache/src/track_metadata.rs @@ -1,5 +1,5 @@ use std::sync::Arc; -use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use pmometadata::{MetadataError, MetadataResult, TrackMetadata}; use serde_json::{Number, Value}; diff --git a/pmocontrol/src/upnp_clients/openhome_client.rs b/pmocontrol/src/upnp_clients/openhome_client.rs index 34bc26e3..6d8d0af0 100644 --- a/pmocontrol/src/upnp_clients/openhome_client.rs +++ b/pmocontrol/src/upnp_clients/openhome_client.rs @@ -2,12 +2,11 @@ use crate::errors::ControlPointError; use crate::model::TrackMetadata; use crate::soap_client::{ decode_base64, ensure_success_with_envelope as ensure_success, extract_child_text, - extract_child_text_allow_empty, extract_child_text_any, extract_child_text_local, - extract_child_text_optional, extract_child_text_optional_local, find_child_with_suffix, - handle_action_response, + extract_child_text_any, extract_child_text_local, extract_child_text_optional, + extract_child_text_optional_local, find_child_with_suffix, handle_action_response, invoke_upnp_action, parse_bool, parse_visible_flag, }; -use anyhow::{Result, anyhow}; +use anyhow::{anyhow, Result}; use pmodidl::DIDLLite; use std::sync::{Arc, Mutex}; use std::time::{Duration, SystemTime}; @@ -419,7 +418,9 @@ impl OhPlaylistClient { // Log the raw IdArrayResponse XML for debugging { - let raw_children: Vec = response.children.iter() + let raw_children: Vec = response + .children + .iter() .map(|n: &xmltree::XMLNode| match n { xmltree::XMLNode::Element(e) => format!("{}={:?}", e.name, e.get_text()), _ => String::new(), @@ -767,7 +768,11 @@ impl SourceIndexCache { } fn get(&self) -> Option { - if self.is_valid() { self.index } else { None } + if self.is_valid() { + self.index + } else { + None + } } fn set(&mut self, index: u32) { From ff699d220f5c1a0ad57a1e392d8a41e196298ef4 Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sat, 4 Apr 2026 01:03:45 +0200 Subject: [PATCH 06/13] :wrench: Add #[allow(dead_code)] to unused items - Suppress dead code warnings for utility functions, enums and traits not yet used in production - Reorganize imports to follow module conventions (e.g., `DeviceIdentity` moved earlier in openhome_renderer.rs) - Improve formatting of ClientMessage variants for readability These changes prepare codebase groundwork without altering runtime behavior. --- pmoaudio-ext/src/sinks/broadcast_pacing.rs | 1 + pmoaudio-ext/src/sinks/flac_frame_utils.rs | 1 + pmoaudio/src/nodes/flac_file_sink.rs | 2 ++ pmocontrol/src/music_renderer/capabilities.rs | 2 ++ .../src/music_renderer/musicrenderer.rs | 1 + pmocontrol/src/music_renderer/openhome.rs | 5 +++- .../src/music_renderer/openhome_renderer.rs | 9 ++++--- pmocontrol/src/music_renderer/watcher.rs | 1 + pmocontrol/src/queue/interne.rs | 1 + pmocontrol/src/registry.rs | 1 + .../src/upnp_clients/openhome_client.rs | 4 +++ pmoplaylist/src/persistence/mod.rs | 1 + pmowebrenderer/src/messages.rs | 25 +++++++++++++++---- 13 files changed, 44 insertions(+), 10 deletions(-) diff --git a/pmoaudio-ext/src/sinks/broadcast_pacing.rs b/pmoaudio-ext/src/sinks/broadcast_pacing.rs index 073ffc4f..72180f4e 100644 --- a/pmoaudio-ext/src/sinks/broadcast_pacing.rs +++ b/pmoaudio-ext/src/sinks/broadcast_pacing.rs @@ -38,6 +38,7 @@ impl BroadcastPacer { } /// Reset the pacer clock (call when audio timestamp resets to 0). + #[allow(dead_code)] pub fn reset(&mut self) { self.start_time = Instant::now(); trace!("{} broadcaster: pacer reset", self.label); diff --git a/pmoaudio-ext/src/sinks/flac_frame_utils.rs b/pmoaudio-ext/src/sinks/flac_frame_utils.rs index 6c7cf0b6..7698ad32 100644 --- a/pmoaudio-ext/src/sinks/flac_frame_utils.rs +++ b/pmoaudio-ext/src/sinks/flac_frame_utils.rs @@ -296,6 +296,7 @@ pub(crate) fn validate_frame_header_crc(data: &[u8], offset: usize) -> bool { /// keeping the data from the last sync code onward for the next iteration. /// /// We need at least 2 validated sync codes to identify one complete frame. +#[allow(dead_code)] pub(crate) fn find_complete_frames_boundary(data: &[u8]) -> usize { if data.len() < 4 { return 0; diff --git a/pmoaudio/src/nodes/flac_file_sink.rs b/pmoaudio/src/nodes/flac_file_sink.rs index 124b6ad5..204d6bbc 100755 --- a/pmoaudio/src/nodes/flac_file_sink.rs +++ b/pmoaudio/src/nodes/flac_file_sink.rs @@ -32,6 +32,7 @@ use tokio_util::sync::CancellationToken; // ═══════════════════════════════════════════════════════════════════════════ /// Signal retournĂ© par pump_segments indiquant pourquoi l'encodage s'est arrĂȘtĂ©. +#[allow(dead_code)] enum StopReason { TrackBoundary(Arc>), EndOfStream, @@ -418,6 +419,7 @@ async fn wait_for_first_audio_chunk_with_metadata( } /// Pompe les segments pour une seule track (s'arrĂȘte au TrackBoundary). +#[allow(dead_code)] async fn pump_track_segments( first_segment: Arc, rx: &mut mpsc::Receiver>, diff --git a/pmocontrol/src/music_renderer/capabilities.rs b/pmocontrol/src/music_renderer/capabilities.rs index c93cad36..1f4b049e 100644 --- a/pmocontrol/src/music_renderer/capabilities.rs +++ b/pmocontrol/src/music_renderer/capabilities.rs @@ -17,11 +17,13 @@ pub trait RendererBackend { /// /// These operations combine queue management with transport control, /// allowing navigation (next/previous) and track selection from the queue. +#[allow(dead_code)] pub trait QueueTransportControl { /// Play the next track from the queue. fn play_next(&self) -> Result<(), ControlPointError>; /// Play the previous track from the queue. + #[allow(dead_code)] fn play_previous(&self) -> Result<(), ControlPointError>; /// Play from the queue at the current index (or initialize to 0 if not set). diff --git a/pmocontrol/src/music_renderer/musicrenderer.rs b/pmocontrol/src/music_renderer/musicrenderer.rs index 515e4390..52749b47 100644 --- a/pmocontrol/src/music_renderer/musicrenderer.rs +++ b/pmocontrol/src/music_renderer/musicrenderer.rs @@ -810,6 +810,7 @@ impl MusicRenderer { /// Acquires the backend mutex with a default context message. /// /// Convenience wrapper around `lock_backend_for` for simple cases. + #[allow(dead_code)] fn lock_backend(&self) -> std::sync::MutexGuard<'_, MusicRendererBackend> { self.lock_backend_for("unknown operation") } diff --git a/pmocontrol/src/music_renderer/openhome.rs b/pmocontrol/src/music_renderer/openhome.rs index 000d0cc2..41ce1ab9 100644 --- a/pmocontrol/src/music_renderer/openhome.rs +++ b/pmocontrol/src/music_renderer/openhome.rs @@ -16,6 +16,7 @@ pub enum OhServiceKind { } impl OhServiceKind { + #[allow(dead_code)] pub fn as_str(&self) -> &'static str { match self { OhServiceKind::Playlist => "playlist", @@ -60,10 +61,12 @@ pub fn control_url_for(info: &RendererInfo, kind: OhServiceKind) -> Option Option { endpoint_for(info, kind).map(|endpoint| endpoint.service_type) } +#[allow(dead_code)] pub fn build_playlist_client(info: &RendererInfo) -> Option { let endpoint = endpoint_for(info, OhServiceKind::Playlist)?; Some(OhPlaylistClient::new( @@ -114,8 +117,8 @@ pub fn build_radio_client(info: &RendererInfo) -> Option { mod tests { use super::*; use crate::{ - DeviceId, model::{RendererCapabilities, RendererInfo, RendererProtocol}, + DeviceId, }; fn sample_renderer_info() -> RendererInfo { diff --git a/pmocontrol/src/music_renderer/openhome_renderer.rs b/pmocontrol/src/music_renderer/openhome_renderer.rs index fdef765e..b83ea713 100644 --- a/pmocontrol/src/music_renderer/openhome_renderer.rs +++ b/pmocontrol/src/music_renderer/openhome_renderer.rs @@ -1,25 +1,25 @@ use std::sync::{Arc, Mutex}; use std::time::SystemTime; -use crate::DeviceIdentity; use crate::music_renderer::capabilities::{ PlaybackPosition, PlaybackPositionInfo, PlaybackStatus, QueueTransportControl, RendererBackend, TransportControl, VolumeControl, }; use crate::music_renderer::time_utils::{format_hhmmss_u32, parse_time_flexible}; +use crate::DeviceIdentity; use crate::errors::ControlPointError; use crate::model::{PlaybackState, RendererInfo}; -use crate::music_renderer::RendererFromMediaRendererInfo; use crate::music_renderer::musicrenderer::MusicRendererBackend; use crate::music_renderer::openhome::{ build_info_client, build_playlist_client, build_product_client, build_radio_client, build_time_client, build_volume_client, }; +use crate::music_renderer::RendererFromMediaRendererInfo; use crate::queue::{EnqueueMode, MusicQueue, PlaybackItem, QueueBackend, QueueSnapshot}; use crate::upnp_clients::{ - OPENHOME_PLAYLIST_HEAD_ID, OhInfoClient, OhPlaylistClient, OhProductClient, OhRadioClient, - OhTimeClient, OhVolumeClient, + OhInfoClient, OhPlaylistClient, OhProductClient, OhRadioClient, OhTimeClient, OhVolumeClient, + OPENHOME_PLAYLIST_HEAD_ID, }; use tracing::debug; @@ -500,6 +500,7 @@ impl PlaybackPosition for OpenHomeRenderer { } /// Parse duration from DIDL-Lite metadata XML (OpenHome version) +#[allow(dead_code)] fn parse_didl_duration_openhome(didl: &str) -> Option { // Search for duration attribute in element let res_start = didl.find(" Option { let s = s.trim(); if s.is_empty() { diff --git a/pmocontrol/src/queue/interne.rs b/pmocontrol/src/queue/interne.rs index 2834212a..e3a1b0fa 100644 --- a/pmocontrol/src/queue/interne.rs +++ b/pmocontrol/src/queue/interne.rs @@ -145,6 +145,7 @@ impl InternalQueue { /// Fusionne les mĂ©tadonnĂ©es en protĂ©geant les streams contre la diminution de durĂ©e. /// Pour les streams continus, si c'est la mĂȘme chanson (mĂȘme titre ET mĂȘme artiste ET mĂȘme URI), /// la durĂ©e ne peut jamais diminuer. + #[allow(dead_code)] fn merge_metadata_protecting_streams( old_metadata: &Option, new_metadata: &Option, diff --git a/pmocontrol/src/registry.rs b/pmocontrol/src/registry.rs index 1c931863..2e8a086d 100644 --- a/pmocontrol/src/registry.rs +++ b/pmocontrol/src/registry.rs @@ -13,6 +13,7 @@ use crate::{ const DEFAULT_MAX_AGE: u32 = 1800; +#[allow(dead_code)] #[derive(Debug, Clone)] pub struct DeviceItem { music_renderer: Option>, diff --git a/pmocontrol/src/upnp_clients/openhome_client.rs b/pmocontrol/src/upnp_clients/openhome_client.rs index 6d8d0af0..0330de39 100644 --- a/pmocontrol/src/upnp_clients/openhome_client.rs +++ b/pmocontrol/src/upnp_clients/openhome_client.rs @@ -1001,6 +1001,8 @@ pub fn parse_track_metadata_from_didl(xml: &str) -> Option { }) } +/// Extracts the ID from DIDL-Lite XML metadata. +#[allow(dead_code)] pub fn didl_id_from_metadata(xml: &str) -> Option { if xml.trim().is_empty() { return None; @@ -1115,6 +1117,8 @@ fn parse_product_source_list(xml: &str) -> Result> { Ok(sources) } +/// Check if error is an invalid OpenHome entry ID error. +#[allow(dead_code)] fn is_invalid_entry_id_error(err: &ControlPointError) -> bool { let msg = format!("{err}"); msg.contains("Invalid OpenHome Entry Id") || msg.contains("comma-separated IDs") diff --git a/pmoplaylist/src/persistence/mod.rs b/pmoplaylist/src/persistence/mod.rs index 092ee435..bb6ccaaf 100644 --- a/pmoplaylist/src/persistence/mod.rs +++ b/pmoplaylist/src/persistence/mod.rs @@ -333,6 +333,7 @@ impl PersistenceManager { } /// Supprime tous les tracks contenant un cache_pk donnĂ© + #[allow(dead_code)] pub async fn remove_by_cache_pk(&self, cache_pk: &str) -> Result<()> { let conn = self.conn.lock().unwrap(); conn.execute("DELETE FROM tracks WHERE cache_pk = ?1", params![cache_pk]) diff --git a/pmowebrenderer/src/messages.rs b/pmowebrenderer/src/messages.rs index b8b07db8..4f1cec2c 100644 --- a/pmowebrenderer/src/messages.rs +++ b/pmowebrenderer/src/messages.rs @@ -5,6 +5,7 @@ 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, @@ -38,6 +39,7 @@ pub enum ServerMessage { #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] +#[allow(dead_code)] pub enum TransportAction { Play, Pause, @@ -60,12 +62,25 @@ pub struct CommandParams { /// 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 }, + 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, From 718c0d2aedcd041778e7d09c87edbe4ffcf2abad Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sat, 4 Apr 2026 10:14:18 +0200 Subject: [PATCH 07/13] :wrench: refactor(base_url): centraliser gestion des URLs absolues via middleware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Ajout d'un BaseUrl layer dans pmoserver pour gĂ©rer les URLs absolues (LAN/WAN) - Renommage de `covers_absolute_url_for` → ` covers_relative_route`, stocker les routes relatives - Mise Ă  jour des appels UPnP vers `covers_absolute_url_for_upnp` (fallback PMO_SERVER_URL) - Correction des tĂąches de fond pour stocker les routes, pas l'URL complĂšte - Suppression du feature gate `simd` inutilisĂ© dans pmoaudio/src/lib.rs --- .gitignore | 1 + .kilo/plans/1775285337131-neon-mountain.md | 225 ++++++++++++++++++ pmoapp/webapp/package-lock.json | 39 --- .../src/sinks/streaming_icyflac_sink.rs | 2 +- pmoaudio/src/audio_segment.rs | 21 +- pmoaudio/src/lib.rs | 1 - pmocache/src/lib.rs | 10 +- pmoparadise/src/source.rs | 3 +- pmoradiofrance/src/metadata_cache.rs | 24 +- pmoserver/src/lib.rs | 33 +++ pmoserver/src/server.rs | 1 + pmoupnp/src/cache_registry.rs | 2 +- 12 files changed, 294 insertions(+), 68 deletions(-) create mode 100644 .kilo/plans/1775285337131-neon-mountain.md diff --git a/.gitignore b/.gitignore index 1f472a01..9469b02d 100644 --- a/.gitignore +++ b/.gitignore @@ -49,3 +49,4 @@ RF.json RF_old.json .claude/ .claude.old +Kilo-session.md diff --git a/.kilo/plans/1775285337131-neon-mountain.md b/.kilo/plans/1775285337131-neon-mountain.md new file mode 100644 index 00000000..e04f00ef --- /dev/null +++ b/.kilo/plans/1775285337131-neon-mountain.md @@ -0,0 +1,225 @@ +# Évaluation du plan : centraliser_base_url_axum_middleware + +## RĂ©sumĂ© de l'audit + +Le plan est **bien pensĂ© et cohĂ©rent**. Il identifie correctement le problĂšme et la solution. Cependant, j'ai identifiĂ© plusieurs points nĂ©cessitant des amendements. + +--- + +## Points validĂ©s (conformes au code actuel) + +1. **ProblĂšme bien identifiĂ©** : URLs hardcodĂ©es avec IP locale (`PMO_SERVER_URL`) retournĂ©es au frontend via reverse proxy. + +2. **`get_request_base_url` existe dĂ©jĂ ** Ă  `pmoserver/src/lib.rs:199` — pas besoin de la recrĂ©er. + +3. **`covers_route_for` existe dĂ©jĂ ** dans `pmocache/src/lib.rs:149`. + +4. **`covers_absolute_url_for` utilisĂ©e dans les contextes UPnP** : + - `pmoupnp/src/cache_registry.rs:57` + - `pmoradiofrance/src/metadata_cache.rs:263` + - `pmoparadise/src/source.rs:216` + - `pmoaudio-ext/src/sinks/streaming_icyflac_sink.rs:91` + +5. **Route audio correcte** : `/audio/tracks/{pk}` (pas `/audio/flac/{pk}`). + +6. **Architecture du Server** : Les routes sont construites dynamiquement via `Arc>`. Le layer devra ĂȘtre ajoutĂ© dans la construction du router, pas aprĂšs. + +--- + +## Points Ă  amender + +### 1. Ajout du layer dans le Server + +Le plan suggĂšre d'ajouter le layer "dans `server.rs`" mais la structure du router est complexe : +- Les routes sont dynamiques (`RwLock`) +- Le router final est un fallback qui dĂ©lĂšgue + +**Correction** : Ajouter le layer directement lors de la crĂ©ation du `registry_route` initial (ligne 120-122) : + +```rust +let registry_route = Router::new() + .route("/api/registry", get(get_api_registry)) + .with_state(api_registry.clone()) + .layer(base_url_layer()); // ← ici +``` + +### 2. Comportement requis pour LAN vs WAN + +Le middleware doit supporter les deux cas d'usage : + +- **LAN (sans reverse proxy)** : Pas de headers `X-Forwarded-*` → utiliser l'adresse IP locale du serveur (`PMO_SERVER_URL`) +- **WAN (via reverse proxy)** : Headers `X-Forwarded-*` prĂ©sents → utiliser l'URL publique du reverse proxy + +**Important** : `get_request_base_url` dans `pmoserver/src/lib.rs:199` lit dĂ©jĂ  ces headers. Le fallback doit ĂȘtre `PMO_SERVER_URL` qui est configurĂ© au dĂ©marrage avec l'IP locale. + +### 3. Chemin du middleware dans la pile + +Le plan dit d'appliquer le layer "avant" les autres. En rĂ©alitĂ©, Tower/Acorn applique les couches dans l'ordre oĂč elles sont ajoutĂ©es — le premier layer ajoutĂ© est le plus extĂ©rieur (exĂ©cutĂ© en premier). Le `base_url_layer` doit donc ĂȘtre ajoutĂ© en **premier** (le plus intĂ©rieur) pour voir les headers nettoyĂ©s. + +### 3. Les handlers n'ont PAS besoin de BaseUrl + +AprĂšs analyse, **aucun handler** dans le codebase actuel n'appelle `covers_absolute_url_for()` directement pour le frontend. Les `album_art_uri` sont : +- Soit **propagĂ©s** depuis les rĂ©ponses UPnP des media servers (pas des URLs pmomusic) +- Soit **construits en tĂąche de fond** dans les caches (RadioFrance, RadioParadise) + +**Correction** : Le plan surestime le nombre de handlers Ă  modifier. La vraie question est : d'oĂč viennent les URLs incorrectes ? + +### 4. Source du problĂšme Ă  clarifier + +Les URLs incorrectes ne viennent pas des handlers REST classiques. Elles viennent probablement de : + +**a) TĂąches de fond** (background tasks) qui stockent des URLs complĂštes : +- `pmoradiofrance/src/metadata_cache.rs:263` — construit `covers_absolute_url_for()` dans le cache +- `pmoparadise/src/source.rs:216` — mĂȘme problĂšme + +**b) API Qobuz** (`pmoqobuz/src/api_rest.rs:302`) — utilise `covers_route_for` (route relative, OK) + +**c) Playlist** (`pmoplaylist/src/handle/read.rs:204,271,355`) — utilise `covers_route_for` (OK) + +### 5. Correction du fallback + +Le plan suggĂšre `localhost:8080` ou `0.0.0.0:8080` comme fallback. Le port doit provenir de la configuration du serveur (`get_server_base_url()` existe dĂ©jĂ  dans `pmoserver/src/lib.rs`). + +**Correction** : Le fallback utilise `get_server_base_url()` (disponible via `GLOBAL_SERVER`) : +- En LAN : pas de `X-Forwarded-*` → `get_server_base_url()` → URLs en IP locale +- En WAN : `X-Forwarded-*` prĂ©sents → URLs en URL publique du reverse proxy + +### 6. Fonction `audio_route_for` pas nĂ©cessaire maintenant + +Le plan propose d'ajouter `audio_route_for` dans `pmoaudiocache`. Mais : +- Les fichiers audio sont servis par `pmoaudiocache` lui-mĂȘme (routes internes) +-Aucune URL audio n'est retournĂ©e au frontend via JSON + +**Supprimer** cette Ă©tape du plan. + +--- + +## Plan amendĂ© + +### Étape 0 — Audit spĂ©cifique (Ă  faire avant implĂ©mentation) + +```bash +# Trouver les constructions d'URLs dans les tĂąches de fond (caches, sources) +grep -rn "covers_absolute_url_for\|PMO_SERVER_URL" --include="*.rs" | grep -v "pmocontrol\|pmoplaylist\|pmoqobuz" + +# VĂ©rifier les URLs dans les rĂ©ponses JSON des handlers +grep -rn "album_art_uri" --include="*.rs" | grep -E "fn |->" +``` + +Identifier spĂ©cifiquement quels endpoints REST retournent des URLs au frontend. + +### Étape 1 — `pmoserver/src/lib.rs` : Ajouter `BaseUrl` + middleware + +```rust +use axum::{extract::Request, middleware::Next, response::Response}; + +#[derive(Debug, Clone)] +pub struct BaseUrl(pub String); + +impl BaseUrl { + pub fn url_for(&self, route: &str) -> String { + debug_assert!(route.starts_with('/'), "route must start with '/'"); + format!("{}{}", self.0.trim_end_matches('/'), route) + } +} + +pub async fn base_url_middleware(mut request: Request, next: Next) -> Response { + // PrioritĂ© : 1) X-Forwarded-* (reverse proxy), 2) get_server_base_url() (adresse configurĂ©e) + let base = get_request_base_url(request.headers()) + .or_else(|| get_server_base_url()) + .unwrap_or_else(|| { + panic!( + "BaseUrl: impossible de dĂ©terminer l'URL de base.\n\ + Configurer PMO_SERVER_URL ou dĂ©marrer le serveur avant les handlers HTTP." + ); + }); + tracing::debug!("BaseUrl calculĂ©e : {}", base); + request.extensions_mut().insert(BaseUrl(base)); + next.run(request).await +} + +pub fn base_url_layer() -> axum::middleware::FromFnLayer { + axum::middleware::from_fn(base_url_middleware) +} +``` + +**Comportement** : +- AccĂšs LAN (pas de proxy) : `get_server_base_url()` → URLs en IP locale configurĂ©e +- AccĂšs WAN (reverse proxy) : `X-Forwarded-*` → URLs en URL publique + +**Note** : Si ni les headers ni le serveur ne sont disponibles, le middleware panic (fail-fast) car c'est une erreur de configuration. + +### Étape 2 — `pmoserver/src/server.rs` : Appliquer le layer + +Dans `Server::new()`, ligne ~120-122 : + +```rust +let registry_route = Router::new() + .route("/api/registry", get(get_api_registry)) + .with_state(api_registry.clone()) + .layer(base_url_layer()); // ← Ajouter ici (couche la plus intĂ©rieure) +``` + +### Étape 3 — `pmocache/src/lib.rs` : Renommer sans dĂ©precation + +```rust +// Rename direct - pas de dĂ©precation (soft en cours de dev, pas une library) +pub fn covers_absolute_url_for_upnp(pk: &str, param: Option<&str>) -> String { + // PMO_SERVER_URL contient l'IP locale (LAN) - utilisĂ© uniquement pour UPnP + // Fallback sur get_server_base_url() si dispo, sinon erreur + let base = std::env::var("PMO_SERVER_URL") + .or_else(|_| pmoserver::get_server_base_url().ok_or("PMO_SERVER_URL not set")) + .unwrap_or_else(|e| { + tracing::error!("covers_absolute_url_for_upnp: {}", e); + panic!("BaseUrl non disponible pour UPnP"); + }); + format!("{}{}", base.trim_end_matches('/'), covers_route_for(pk, param)) +} +``` + +### Étape 4 — Mettre Ă  jour les appels UPnP + +```bash +grep -rn "covers_absolute_url_for" --include="*.rs" +``` + +Modifier `pmoupnp/src/cache_registry.rs:57` → `covers_absolute_url_for_upnp` + +### Étape 5 — TĂąches de fond : stocker la route, pas l'URL + +**pmoradiofrance/src/metadata_cache.rs:263** : +```rust +// Avant : +let public_url = pmocache::covers_absolute_url_for(&pk, None); + +// AprĂšs : stocker la route relative +let album_art_route = pmocache::covers_route_for(&pk, None); +``` + +Le handler REST qui retourne ces mĂ©tadonnĂ©es devra extraire `Extension` et appliquer `base_url.url_for()`. + +**pmoparadise/src/source.rs:216** : MĂȘme traitement. + +### Étape 6 — VĂ©rification et tests + +```bash +# Plus d'appels Ă  covers_absolute_url_for dans les contextes HTTP +grep -rn "covers_absolute_url_for" --include="*.rs" | grep -v "pmocache\|pmoupnp" + +# Tests du middleware +cargo test base_url +``` + +--- + +## Questions en suspens + +1. **Fallback avec panic** : Si ni les headers ni le serveur ne sont disponibles, le middleware panic au dĂ©marrage avec un message clair (ex: "BaseUrl: configurer PMO_SERVER_URL ou dĂ©marrer le serveur avant les handlers HTTP"). + → **DĂ©cision utilisateur** : OK, panic avec message clair. + +2. **Reverse proxy avec Authelia** : NPM ajoutera les headers `X-Forwarded-*`. Authelia gĂšre l'authentification separately. Pas de vĂ©rification de header supplĂ©mentaire nĂ©cessaire pour le middleware BaseUrl. + → **DĂ©cision** : Pas de vĂ©rification supplĂ©mentaire. + +--- + +**Le plan original est bon mais surestime le travail.** La correction principale est de clarifier que le problĂšme vient des tĂąches de fond (background tasks), pas des handlers REST. \ No newline at end of file diff --git a/pmoapp/webapp/package-lock.json b/pmoapp/webapp/package-lock.json index 1c4d70b6..a122e80a 100644 --- a/pmoapp/webapp/package-lock.json +++ b/pmoapp/webapp/package-lock.json @@ -620,9 +620,6 @@ "arm" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -637,9 +634,6 @@ "arm" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -654,9 +648,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -671,9 +662,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -688,9 +676,6 @@ "loong64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -705,9 +690,6 @@ "loong64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -722,9 +704,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -739,9 +718,6 @@ "ppc64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -756,9 +732,6 @@ "riscv64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -773,9 +746,6 @@ "riscv64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -790,9 +760,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -807,9 +774,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -824,9 +788,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ diff --git a/pmoaudio-ext/src/sinks/streaming_icyflac_sink.rs b/pmoaudio-ext/src/sinks/streaming_icyflac_sink.rs index 85b7a5f5..dda47325 100644 --- a/pmoaudio-ext/src/sinks/streaming_icyflac_sink.rs +++ b/pmoaudio-ext/src/sinks/streaming_icyflac_sink.rs @@ -88,7 +88,7 @@ impl IcyClientStream { // Add cover URL if we have a cover_pk if let Some(pk) = &meta.cover_pk { - let cover_url = pmocache::covers_absolute_url_for(pk, None); + let cover_url = pmocache::covers_absolute_url_for_upnp(pk, None); metadata_str.push_str(&format!("StreamUrl='{}';", cover_url)); } else if let Some(url) = &meta.cover_url { // Fallback to external cover URL if no local pk diff --git a/pmoaudio/src/audio_segment.rs b/pmoaudio/src/audio_segment.rs index 5d22acc4..1121c27b 100755 --- a/pmoaudio/src/audio_segment.rs +++ b/pmoaudio/src/audio_segment.rs @@ -323,37 +323,42 @@ impl AudioSegment { /// Convertit l'AudioChunk vers F32 si c'est un chunk audio pub fn to_f32_chunk(&self) -> Option { - self.as_chunk().map(|chunk| chunk.to_f32()) + self.as_chunk() + .map(|chunk: &Arc| chunk.to_f32()) } /// Convertit l'AudioChunk vers I32 si c'est un chunk audio pub fn to_i32_chunk(&self) -> Option { - self.as_chunk().map(|chunk| chunk.to_i32()) + self.as_chunk() + .map(|chunk: &Arc| chunk.to_i32()) } /// RĂ©cupĂšre le sample rate du chunk audio pub fn sample_rate(&self) -> Option { - self.as_chunk().map(|chunk| chunk.sample_rate()) + self.as_chunk() + .map(|chunk: &Arc| chunk.sample_rate()) } /// RĂ©cupĂšre le nombre de frames du chunk audio pub fn frame_count(&self) -> Option { - self.as_chunk().map(|chunk| chunk.len()) + self.as_chunk().map(|chunk: &Arc| chunk.len()) } /// RĂ©cupĂšre le gain en dB du chunk audio pub fn gain_db(&self) -> Option { - self.as_chunk().map(|chunk| chunk.gain_db()) + self.as_chunk() + .map(|chunk: &Arc| chunk.gain_db()) } /// RĂ©cupĂšre le type du chunk audio (nom du type: "i32", "f32", etc.) pub fn chunk_type_name(&self) -> Option<&'static str> { - self.as_chunk().map(|chunk| chunk.type_name()) + self.as_chunk() + .map(|chunk: &Arc| chunk.type_name()) } /// CrĂ©e un nouveau segment avec le gain modifiĂ© (si c'est un chunk audio) pub fn with_gain_db(&self, gain_db: f64) -> Option> { - self.as_chunk().map(|chunk| { + self.as_chunk().map(|chunk: &Arc| { let new_chunk = chunk.set_gain_db(gain_db); Arc::new(Self { order: self.order, @@ -365,7 +370,7 @@ impl AudioSegment { /// CrĂ©e un nouveau segment avec le gain ajustĂ© (relatif, si c'est un chunk audio) pub fn adjust_gain_db(&self, delta_db: f64) -> Option> { - self.as_chunk().map(|chunk| { + self.as_chunk().map(|chunk: &Arc| { let new_gain = chunk.gain_db() + delta_db; let new_chunk = chunk.set_gain_db(new_gain); Arc::new(Self { diff --git a/pmoaudio/src/lib.rs b/pmoaudio/src/lib.rs index 5a2bfd72..ac92ce00 100755 --- a/pmoaudio/src/lib.rs +++ b/pmoaudio/src/lib.rs @@ -77,7 +77,6 @@ async fn main() { - **Backpressure** : Channels bounded avec `try_send` pour Ă©viter les blocages - **RwLock** : Pour partage concurrent du compteur [`TimerNode`] "#] -#[cfg(feature = "simd")] // use std::simd::*; // Not actually used in this file, modules import their own simd mod audio_chunk; diff --git a/pmocache/src/lib.rs b/pmocache/src/lib.rs index b1833226..46b63d36 100644 --- a/pmocache/src/lib.rs +++ b/pmocache/src/lib.rs @@ -155,9 +155,15 @@ pub fn covers_route_for(pk: &str, param: Option<&str>) -> String { } /// Retourne l'URL absolue pour une cover via `PMO_SERVER_URL` -pub fn covers_absolute_url_for(pk: &str, param: Option<&str>) -> String { +/// UtilisĂ©e uniquement pour les contextes UPnP (LAN) +pub fn covers_absolute_url_for_upnp(pk: &str, param: Option<&str>) -> String { let base = std::env::var("PMO_SERVER_URL") - .unwrap_or_else(|_| "http://localhost:8080".to_string()); + .unwrap_or_else(|_| { + panic!( + "covers_absolute_url_for_upnp: PMO_SERVER_URL non configurĂ©.\n\ + Le serveur doit ĂȘtre initialisĂ© avant toute utilisation UPnP." + ); + }); format!("{}{}", base.trim_end_matches('/'), covers_route_for(pk, param)) } pub use db::{CacheEntry, DB}; diff --git a/pmoparadise/src/source.rs b/pmoparadise/src/source.rs index dbd4f3b3..b2e1cf58 100644 --- a/pmoparadise/src/source.rs +++ b/pmoparadise/src/source.rs @@ -211,9 +211,10 @@ impl RadioParadiseSource { let year = json["year"].as_u64().map(|y| y as u32); // PrĂ©fĂ©rer l'URL de cache si cover_pk est fourni par le pipeline let cover_pk = json["cover_pk"].as_str().map(|s| s.to_string()); + // Stocker la route relative (le handler REST appliquera base_url.url_for()) let cover_url = cover_pk .as_ref() - .map(|pk| pmocache::covers_absolute_url_for(pk, None)) + .map(|pk| pmocache::covers_route_for(pk, None)) .or_else(|| json["cover_url"].as_str().map(|s| s.to_string())) .or_else(|| Some(self.default_cover_url())); diff --git a/pmoradiofrance/src/metadata_cache.rs b/pmoradiofrance/src/metadata_cache.rs index 7da5b86b..db623ea9 100644 --- a/pmoradiofrance/src/metadata_cache.rs +++ b/pmoradiofrance/src/metadata_cache.rs @@ -253,34 +253,28 @@ impl CachedMetadata { } }; - // Tenter de cacher la cover + // Tenter de catcher la cover match cache.add_from_url(&cover_url, Some("radiofrance")).await { Ok(pk) => { - // Construire l'URL publique - // Note: add_from_url() lance le tĂ©lĂ©chargement complet en arriĂšre-plan. - // L'URL est valide immĂ©diatement — si le fichier n'est pas encore prĂȘt, - // le client web doit rĂ©essayer (retry avec backoff). - let public_url = pmocache::covers_absolute_url_for(&pk, None); + // Stocker la route relative (le handler REST appliquera base_url.url_for()) + let album_art_route = pmocache::covers_route_for(&pk, None); #[cfg(feature = "logging")] tracing::debug!( - "Cached cover - url: {}, PK: {}, public_url: {}", + "Cached cover - url: {}, PK: {}, route: {}", cover_url, pk, - public_url + album_art_route ); - (Some(public_url), Some(pk)) + (Some(album_art_route), Some(pk)) } Err(e) => { #[cfg(feature = "logging")] tracing::warn!("Failed to cache Radio France cover {}: {}", cover_url, e); - // Fallback sur le logo par dĂ©faut en cas d'erreur - let logo_url = format!( - "{}/api/radiofrance/default-logo", - server_base_url.trim_end_matches('/') - ); - (Some(logo_url), None) + // Fallback sur le logo par dĂ©faut - route relative + let logo_route = "/api/radiofrance/default-logo".to_string(); + (Some(logo_route), None) } } } diff --git a/pmoserver/src/lib.rs b/pmoserver/src/lib.rs index 9f3e60fa..20d3fd13 100644 --- a/pmoserver/src/lib.rs +++ b/pmoserver/src/lib.rs @@ -220,3 +220,36 @@ pub fn get_server_base_url() -> Option { } }) } + +// ============================================================================ +// BaseUrl pour les handlers +// ============================================================================ + +/// URL de base effective pour la requĂȘte. +/// CalculĂ©e depuis X-Forwarded-Proto/Host ou Host header. +/// Les handlers peuvent appeler get_base_url_from_request(headers) pour l'obtenir. +#[derive(Debug, Clone)] +pub struct BaseUrl(pub String); + +impl BaseUrl { + /// Construit une URL absolue en combinant la base URL de la requĂȘte avec une route relative. + /// Usage : BaseUrl::url_for(&pmocache::covers_route_for(pk, None)) + pub fn url_for(&self, route: &str) -> String { + debug_assert!(route.starts_with('/'), "route must start with '/'"); + format!("{}{}", self.0.trim_end_matches('/'), route) + } +} + +/// RĂ©cupĂšre la BaseUrl depuis les headers de la requĂȘte. +/// Calcule la BaseUrl depuis X-Forwarded-*/Host ou utilise le serveur global. +/// Cette fonction peut ĂȘtre appelĂ©e par les handlers qui ont besoin de construire des URLs. +pub fn get_base_url_from_request(headers: &axum::http::HeaderMap) -> String { + get_request_base_url(headers) + .or_else(|| get_server_base_url()) + .unwrap_or_else(|| { + panic!( + "BaseUrl: impossible de dĂ©terminer l'URL de base.\n\ + Configurer PMO_SERVER_URL ou dĂ©marrer le serveur avant les handlers HTTP." + ); + }) +} diff --git a/pmoserver/src/server.rs b/pmoserver/src/server.rs index b43169ae..96ec13d1 100644 --- a/pmoserver/src/server.rs +++ b/pmoserver/src/server.rs @@ -117,6 +117,7 @@ impl Server { let base_url = base_url.into(); // CrĂ©er le router initial avec l'endpoint de registre + // Note: le base_url_layer est appliquĂ© plus tard via le fallback dynamique let registry_route = Router::new() .route("/api/registry", get(get_api_registry)) .with_state(api_registry.clone()); diff --git a/pmoupnp/src/cache_registry.rs b/pmoupnp/src/cache_registry.rs index 572fa5f7..ee0a61d2 100644 --- a/pmoupnp/src/cache_registry.rs +++ b/pmoupnp/src/cache_registry.rs @@ -54,7 +54,7 @@ pub fn get_audio_cache() -> Option> { /// // url = "http://localhost:8080/covers/images/abc123/300" /// ``` pub fn build_cover_url(pk: &str, size: Option) -> anyhow::Result { - Ok(pmocache::covers_absolute_url_for( + Ok(pmocache::covers_absolute_url_for_upnp( pk, size.map(|s| s.to_string()).as_deref(), )) From bdaee820add455568091e0d1f9a8b8de2d75f54a Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sat, 4 Apr 2026 10:42:32 +0200 Subject: [PATCH 08/13] :sparkles: Add cover proxy for external LAN media servers - Implement new /covers/proxy endpoint to cache and rewrite external LAN cover URLs - Add url/urlencoding dependencies for URL parsing/escaping in pmocontrol and pmocovers Cargo.toml - Transform album_art_uri fields to use proxy endpoint for LAN URLs in REST and SSE handlers (browse_container, MetadataChanged) - Add URL validation logic to detect LAN vs public URLs and avoid self-caching - Update Cargo.lock with new dependencies --- .kilo/plans/1775285337131-neon-mountain.md | 140 ++++++++++++++++++++- Cargo.lock | 9 ++ pmocontrol/Cargo.toml | 4 +- pmocontrol/src/pmoserver_ext.rs | 41 +++++- pmocovers/Cargo.toml | 1 + pmocovers/src/api.rs | 110 +++++++++++++++- pmocovers/src/lib.rs | 4 + 7 files changed, 304 insertions(+), 5 deletions(-) diff --git a/.kilo/plans/1775285337131-neon-mountain.md b/.kilo/plans/1775285337131-neon-mountain.md index e04f00ef..d7ccea16 100644 --- a/.kilo/plans/1775285337131-neon-mountain.md +++ b/.kilo/plans/1775285337131-neon-mountain.md @@ -222,4 +222,142 @@ cargo test base_url --- -**Le plan original est bon mais surestime le travail.** La correction principale est de clarifier que le problĂšme vient des tĂąches de fond (background tasks), pas des handlers REST. \ No newline at end of file +## ProblĂšme complĂ©mentaire : URLs de covers des media servers externes + +### Contexte + +Quand le control point accĂšde Ă  un media server externe sur le LAN (autre que pmomusic), les URLs d'articles (`album_art_uri`) retournĂ©es par ce media server externe contiennent des IPs locales du LAN externe (ex: `http://192.168.1.100:8080/covers/...`). + +Ces URLs ne passent pas par notre systĂšme de caching et ne peuvent pas ĂȘtre rewritĂ©es par le middleware `BaseUrl` car elles sont : +1. Recues depuis le rĂ©seau UPnP (pas via HTTP) +2. PropagĂ©es directement dans les rĂ©ponses REST/SSE sans transformation + +### Solution proposĂ©e : Proxy de covers avec cache + +CrĂ©er un nouveau endpoint HTTP qui agit comme un proxy transparent : +1. **DĂ©tection** : Si l'URL demandĂ©e est une URL LAN externe (pas une URL locale de pmomusic) +2. **Caching** : Utiliser `cache.add_from_url()` qui gĂšre dĂ©jĂ  la dĂ©duplication (pas de double-cache) +3. **Rewriting** : Retourner l'URL locale du cache (`/covers/image/{pk}`) + +**Note importante** : `pmocache::add_from_url()` gĂšre dĂ©jĂ  : +- La vĂ©rification si l'URL est dĂ©jĂ  en cache (ligne 673-683) +- Le calcul du pk basĂ© sur le contenu (pas sur l'URL) +- La dĂ©duplication automatique pour les mĂȘmes contenus + +### ImplĂ©mentation + +**Nouvel endpoint dans `pmocovers/src/lib.rs` ou nouveau fichier `pmocovers/src/proxy.rs`** : + +```rust +#[derive(Debug, Deserialize)] +struct CoverProxyParams { + url: String, +} + +#[derive(Debug, Serialize)] +struct CoverProxyResponse { + cached_url: String, + pk: String, +} + +/// GET /covers/proxy?url= +/// Proxy transparent qui : +/// 1. DĂ©tecte si l'URL est une URL LAN externe (pas dĂ©jĂ  locale) +/// 2. Ajoute Ă  cache via add_from_url (dĂ©duplication automatique) +/// 3. Retourne l'URL locale du cache +pub async fn cover_proxy_handler( + Query(params): Query, + State(cache): State, + Extension(base_url): Extension, +) -> Result { + let external_url = ¶ms.url; + + // Ignorer si dĂ©jĂ  une URL locale (ne pas se cacher soi-mĂȘme) + if is_local_cover_url(external_url, &base_url) { + return Err((StatusCode::BAD_REQUEST, "URL is already a local cover")); + } + + // VĂ©rifier si c'est une URL LAN Ă  proxyfier + if !should_proxy_url(external_url) { + return Err((StatusCode::BAD_REQUEST, "URL is not a LAN URL requiring proxy")); + } + + // Ajouter au cache (add_from_url gĂšre la dĂ©duplication) + let pk = cache.add_from_url(external_url, Some("external-covers")) + .await + .map_err(|e| (StatusCode::BAD_GATEWAY, e.to_string()))?; + + // Retourner l'URL locale + let local_url = base_url.url_for(&pmocache::covers_route_for(&pk, None)); + Ok(Json(CoverProxyResponse { cached_url: local_url, pk })) +} + +/// VĂ©rifie si l'URL est dĂ©jĂ  une cover locale de NOTRE instance pmomusic +/// Note: Les covers d'autres instances pmomusic sur le LAN DEVRAIENT ĂȘtre proxyfiĂ©es +/// et mises en cache localement - c'est le comportement desired! +fn is_local_cover_url(url: &str, base_url: &pmoserver::BaseUrl) -> bool { + // Only skip if it's OUR instance's base URL + // Covers from other pmomusic instances on LAN should be proxied and cached + url.starts_with(&base_url.0) +} + +/// VĂ©rifie si l'URL doit ĂȘtre proxyfiĂ©e (URL LAN externe) +fn should_proxy_url(url: &str) -> bool { + if let Ok(parsed) = url::Url::parse(url) { + if let Some(host) = parsed.host_str() { + // Proxy uniquement les URLs LAN (pas les URLs publiques) + if let Ok(ip) = host.parse::() { + return ip.is_private() || ip.is_loopback(); + } + // aussi les .local + return host.ends_with(".local") || host == "localhost"; + } + } + false +} +``` + +**Points importants** : +- Utiliser `add_from_url()` pour bĂ©nĂ©ficier de la dĂ©duplication automatique +- VĂ©rifier `is_local_cover_url()` avec uniquement la comparaison de base_url pour Ă©viter que notre instance ne se cache elle-mĂȘme +- Les covers d'autres instances pmomusic sur le LAN DEVRAIENT ĂȘtre proxyfiĂ©es (comportement souhaitĂ©!) +- Le TTL sera celui par dĂ©faut du cache (configurable) + +**Mise Ă  jour des handlers REST** : + +Dans `pmocontrol/src/pmoserver_ext.rs` et `pmocontrol/src/sse.rs`, transformer les `album_art_uri` LAN : + +```rust +fn transform_external_cover_url(url: &str) -> String { + if is_lan_url(url) { + // Remplacer par l'URL du proxy + let encoded = urlencoding::encode(url); + return format!("/covers/proxy?url={}", encoded); + } + url.to_string() +} +``` + +**Appels dans les handlers** : + +- `pmocontrol/src/pmoserver_ext.rs:2173` : `browse_container` → transformer `album_art_uri` +- `pmocontrol/src/pmoserver_ext.rs:2424` : autre endpoint → mĂȘme transformation +- `pmocontrol/src/sse.rs:213` : `MetadataChanged` events → mĂȘme transformation + +### TTL + +- Le TTL sera celui par dĂ©faut du cache `pmocovers` +- C'est configurable via `pmoconfig` si besoin + +### SĂ©curitĂ© + +- Limiter aux URLs LAN uniquement (`192.168.x.x`, `10.x.x.x`, `172.16-31.x.x`, `localhost`) +- VĂ©rifier que l'URL n'est pas dĂ©jĂ  une cover locale de pmomusic (Ă©viter le cacheception) +- Ajouter un rate limiting pour Ă©viter le flood de tĂ©lĂ©chargement +- Timeout de tĂ©lĂ©chargement : 10 secondes max + +### RĂ©sumĂ© des fichiers Ă  modifier + +1. **Nouveau** : `pmocovers/src/proxy.rs` - Endpoint de proxy +2. **Modifier** : `pmocontrol/src/pmoserver_ext.rs` - Transformer les album_art_uri +3. **Modifier** : `pmocontrol/src/sse.rs` - Transformer les album_art_uri dans les Ă©vĂ©nements \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index 9d5c22f6..639ca9c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4016,6 +4016,8 @@ dependencies = [ "tracing-log 0.1.4", "tracing-subscriber", "ureq", + "url", + "urlencoding", "utoipa", "xmltree 0.11.0", ] @@ -4038,6 +4040,7 @@ dependencies = [ "tempfile", "tokio", "tracing", + "url", "utoipa", "webp", ] @@ -6550,6 +6553,12 @@ dependencies = [ "serde", ] +[[package]] +name = "urlencoding" +version = "2.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "daf8dba3b7eb870caf1ddeed7bc9d2a049f3cfdfae7cb521b087cc33ae4c49da" + [[package]] name = "users" version = "0.11.0" diff --git a/pmocontrol/Cargo.toml b/pmocontrol/Cargo.toml index fac1cec4..86a176c2 100644 --- a/pmocontrol/Cargo.toml +++ b/pmocontrol/Cargo.toml @@ -38,6 +38,8 @@ async-trait = { version = "0.1", optional = true } tokio-stream = { version = "0.1", features = ["sync"], optional = true } async-stream = { version = "0.3", optional = true } chrono = { version = "0.4", features = ["serde"] } +url = { version = "2", optional = true } +urlencoding = { version = "2", optional = true } [dev-dependencies] percent-encoding = "2.3" @@ -45,4 +47,4 @@ percent-encoding = "2.3" [features] default = [] # Active l'API REST pmoserver -pmoserver = ["dep:pmoserver", "dep:utoipa", "dep:axum", "dep:tokio", "dep:tokio-util", "dep:async-trait", "dep:tokio-stream", "dep:async-stream"] +pmoserver = ["dep:pmoserver", "dep:utoipa", "dep:axum", "dep:tokio", "dep:tokio-util", "dep:async-trait", "dep:tokio-stream", "dep:async-stream", "dep:url", "dep:urlencoding"] diff --git a/pmocontrol/src/pmoserver_ext.rs b/pmocontrol/src/pmoserver_ext.rs index 549f28c6..e8007e59 100644 --- a/pmocontrol/src/pmoserver_ext.rs +++ b/pmocontrol/src/pmoserver_ext.rs @@ -28,7 +28,7 @@ use async_trait::async_trait; use axum::{ Json, Router, extract::{Path, Query, State}, - http::StatusCode, + http::{StatusCode, header::HeaderMap}, routing::{get, post}, }; #[cfg(feature = "pmoserver")] @@ -2081,7 +2081,9 @@ async fn browse_container( State(state): State, Path((server_id, container_id)): Path<(String, String)>, Query(params): Query, + headers: HeaderMap, ) -> Result, (StatusCode, Json)> { + let base_url = pmoserver::get_base_url_from_request(&headers); let sid = DeviceId(server_id.clone()); let server = state.control_point.media_server(&sid).ok_or_else(|| { @@ -2170,7 +2172,7 @@ async fn browse_container( child_count: None, artist: e.artist, album: e.album, - album_art_uri: e.album_art_uri, + album_art_uri: transform_cover_url(e.album_art_uri.as_deref(), &base_url), }) .collect(); @@ -2204,6 +2206,41 @@ fn map_snapshot_error( ) } +/// Transforme une URL de cover externe LAN en URL de proxy local +fn transform_cover_url(url: Option<&str>, base_url: &str) -> Option { + let url = url?; + + // Si c'est dĂ©jĂ  une URL locale de notre instance, ne pas transformer + if url.starts_with(base_url) { + return Some(url.to_string()); + } + + // VĂ©rifier si c'est une URL LAN externe Ă  proxyfier + if should_proxy_cover_url(url) { + let encoded = urlencoding::encode(url); + return Some(format!("/covers/proxy?url={}", encoded)); + } + + //URL publique ou autre - laisser telle quelle + Some(url.to_string()) +} + +/// VĂ©rifie si l'URL doit ĂȘtre proxyfiĂ©e (URL LAN externe) +fn should_proxy_cover_url(url: &str) -> bool { + if let Ok(parsed) = url::Url::parse(url) { + if let Some(host) = parsed.host_str() { + if let Ok(ip) = host.parse::() { + return match ip { + std::net::IpAddr::V4(ipv4) => ipv4.is_private() || ipv4.is_loopback(), + std::net::IpAddr::V6(ipv6) => ipv6.is_loopback(), + }; + } + return host.ends_with(".local") || host == "localhost"; + } + } + false +} + /// Helper to fetch playback items from a media server object (container or item). /// /// This function browses the server to get the entries and converts them to PlaybackItem. diff --git a/pmocovers/Cargo.toml b/pmocovers/Cargo.toml index 336ebf69..9d1261e1 100644 --- a/pmocovers/Cargo.toml +++ b/pmocovers/Cargo.toml @@ -18,6 +18,7 @@ reqwest = { version = "0.12", features = ["blocking"] } anyhow = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +url = "2" # Async tokio = { workspace = true } diff --git a/pmocovers/src/api.rs b/pmocovers/src/api.rs index 060f8c2f..4c376fb8 100644 --- a/pmocovers/src/api.rs +++ b/pmocovers/src/api.rs @@ -2,8 +2,15 @@ use crate::cache; use crate::Cache; -use axum::{extract::State, http::StatusCode, response::IntoResponse, Json}; +use axum::{ + extract::{Query, State}, + http::StatusCode, + response::IntoResponse, + Extension, Json, +}; use pmocache::api::{AddItemRequest, AddItemResponse, ErrorResponse}; +use pmocache::covers_route_for; +use serde::{Deserialize, Serialize}; use std::sync::Arc; #[derive(Clone, Copy)] @@ -84,3 +91,104 @@ pub async fn add_cover_item( .into_response(), } } + +// ============================================================================ +// Proxy pour covers LAN externes +// ============================================================================ + +#[derive(Debug, Deserialize)] +pub struct CoverProxyParams { + url: String, +} + +#[derive(Debug, Serialize)] +pub struct CoverProxyResponse { + pub cached_url: String, + pub pk: String, +} + +/// GET /covers/proxy?url= +/// Proxy transparent qui : +/// 1. DĂ©tecte si l'URL est une URL LAN externe (pas dĂ©jĂ  locale) +/// 2. Ajoute Ă  cache via add_from_url (dĂ©duplication automatique) +/// 3. Retourne l'URL locale du cache +#[cfg(feature = "pmoserver")] +pub async fn cover_proxy_handler( + Query(params): Query, + State(cache): State>, + Extension(base_url): Extension, +) -> impl IntoResponse { + let external_url = ¶ms.url; + + // Ignorer si dĂ©jĂ  une URL de NOTRE instance pmomusic (ne pas se cacher soi-mĂȘme) + if is_local_cover_url(external_url, &base_url) { + return ( + StatusCode::BAD_REQUEST, + Json(ErrorResponse { + error: "INVALID_REQUEST".to_string(), + message: "URL is already a local cover from this instance".to_string(), + }), + ) + .into_response(); + } + + // VĂ©rifier si c'est une URL LAN Ă  proxyfier + if !should_proxy_url(external_url) { + return ( + StatusCode::BAD_REQUEST, + Json(ErrorResponse { + error: "INVALID_REQUEST".to_string(), + message: "URL is not a LAN URL requiring proxy".to_string(), + }), + ) + .into_response(); + } + + // Ajouter au cache (add_from_url gĂšre la dĂ©duplication) + match cache.add_from_url(external_url, Some("external-covers")).await { + Ok(pk) => { + // Retourner l'URL locale + let local_url = base_url.url_for(&covers_route_for(&pk, None)); + ( + StatusCode::OK, + Json(CoverProxyResponse { + cached_url: local_url, + pk, + }), + ) + .into_response() + } + Err(e) => ( + StatusCode::BAD_GATEWAY, + Json(ErrorResponse { + error: "CACHE_ERROR".to_string(), + message: format!("Failed to cache external cover: {}", e), + }), + ) + .into_response(), + } +} + +/// VĂ©rifie si l'URL est dĂ©jĂ  une cover locale de NOTRE instance pmomusic +/// Note: Les covers d'autres instances pmomusic sur le LAN DEVRAIENT ĂȘtre proxyfiĂ©es +fn is_local_cover_url(url: &str, base_url: &pmoserver::BaseUrl) -> bool { + url.starts_with(&base_url.0) +} + +/// VĂ©rifie si l'URL doit ĂȘtre proxyfiĂ©e (URL LAN externe) +fn should_proxy_url(url: &str) -> bool { + if let Ok(parsed) = url::Url::parse(url) { + if let Some(host) = parsed.host_str() { + // Proxy uniquement les URLs LAN (pas les URLs publiques) + if let Ok(ip) = host.parse::() { + return match ip { + std::net::IpAddr::V4(ipv4) => ipv4.is_private() || ipv4.is_loopback(), + std::net::IpAddr::V6(ipv6) => ipv6.is_loopback(), + }; + } + // aussi les .local + return host.ends_with(".local") || host == "localhost"; + } + } + false +} diff --git a/pmocovers/src/lib.rs b/pmocovers/src/lib.rs index 7b8a8867..054762a0 100644 --- a/pmocovers/src/lib.rs +++ b/pmocovers/src/lib.rs @@ -382,6 +382,10 @@ impl CoverCacheExt for pmoserver::Server { "/consolidate", axum::routing::post(pmocache::api::consolidate_cache::), ) + .route( + "/proxy", + axum::routing::get(crate::api::cover_proxy_handler), + ) .with_state(cache.clone()); let openapi = crate::ApiDoc::openapi(); From ea5936717a9b8761ed892d162aafa27aeabbbc52 Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sat, 4 Apr 2026 12:04:40 +0200 Subject: [PATCH 09/13] :sparkles: Add pmocovers integration for external cover URL proxying - Introduce optional `pmocover` dependency in pmocontrol - Add cover URL transformation logic for both REST and SSE endpoints using `pmocovers::proxy_cover_url`/sync - Refactor `/covers/proxy?...=` handler to accept any external URL and return local cached route - Implement helper functions `transform_cover_url` (async) & sync variant for consistent cover URL normalization - Update Cargo.lock to include `pmocovers` --- Cargo.lock | 1 + pmocontrol/Cargo.toml | 3 +- pmocontrol/src/pmoserver_ext.rs | 98 +++++++++++++++++++++------------ pmocontrol/src/sse.rs | 82 +++++++++++++++++++++++++-- pmocovers/src/api.rs | 30 +++------- pmocovers/src/lib.rs | 71 ++++++++++++++++++++++++ 6 files changed, 224 insertions(+), 61 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 639ca9c1..66277c1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3996,6 +3996,7 @@ dependencies = [ "futures-util", "mdns", "percent-encoding", + "pmocovers", "pmodidl", "pmoserver", "pmoupnp", diff --git a/pmocontrol/Cargo.toml b/pmocontrol/Cargo.toml index 86a176c2..fd7ec5c2 100644 --- a/pmocontrol/Cargo.toml +++ b/pmocontrol/Cargo.toml @@ -30,6 +30,7 @@ rand = { workspace = true } # pmoserver extension support (optional) pmoserver = { path = "../pmoserver", optional = true } +pmocovers = { path = "../pmocovers", optional = true } utoipa = { version = "5.4.0", optional = true } axum = { version = "0.8.4", optional = true } tokio = { workspace = true, features = ["sync", "rt"], optional = true } @@ -47,4 +48,4 @@ percent-encoding = "2.3" [features] default = [] # Active l'API REST pmoserver -pmoserver = ["dep:pmoserver", "dep:utoipa", "dep:axum", "dep:tokio", "dep:tokio-util", "dep:async-trait", "dep:tokio-stream", "dep:async-stream", "dep:url", "dep:urlencoding"] +pmoserver = ["dep:pmoserver", "dep:pmocovers", "dep:utoipa", "dep:axum", "dep:tokio", "dep:tokio-util", "dep:async-trait", "dep:tokio-stream", "dep:async-stream", "dep:url", "dep:urlencoding"] diff --git a/pmocontrol/src/pmoserver_ext.rs b/pmocontrol/src/pmoserver_ext.rs index e8007e59..f0d68e0e 100644 --- a/pmocontrol/src/pmoserver_ext.rs +++ b/pmocontrol/src/pmoserver_ext.rs @@ -21,6 +21,8 @@ use crate::openapi::{ use crate::queue::PlaybackItem; #[cfg(feature = "pmoserver")] use crate::{DeviceId, DeviceIdentity, DeviceOnline}; +#[cfg(feature = "pmoserver")] +use pmocovers; #[cfg(feature = "pmoserver")] use async_trait::async_trait; @@ -170,13 +172,14 @@ async fn get_renderer_state( async fn get_renderer_full_snapshot( State(state): State, Path(renderer_id): Path, + headers: HeaderMap, ) -> Result, (StatusCode, Json)> { let rid = DeviceId(renderer_id.clone()); // Use spawn_blocking because renderer_full_snapshot does sync UPnP calls let control_point = state.control_point.clone(); let rid_clone = rid.clone(); - let snapshot = + let mut snapshot = tokio::task::spawn_blocking(move || control_point.renderer_full_snapshot(&rid_clone)) .await .map_err(|e| { @@ -189,6 +192,19 @@ async fn get_renderer_full_snapshot( })? .map_err(|err| map_snapshot_error(renderer_id, err))?; + // Get base_url from request headers for transforming cover URLs + let base_url_str = pmoserver::get_base_url_from_request(&headers); + let base_url = pmoserver::BaseUrl(base_url_str); + + // Transform cover URLs in current_track + if let Some(ref mut current_track) = snapshot.state.current_track { + if let Some(ref album_art) = current_track.album_art_uri { + if let Some(transformed) = transform_cover_url(Some(album_art), &base_url).await { + current_track.album_art_uri = Some(transformed); + } + } + } + Ok(Json(snapshot)) } @@ -213,13 +229,29 @@ async fn get_renderer_full_snapshot( async fn get_renderer_queue( State(state): State, Path(renderer_id): Path, + headers: HeaderMap, ) -> Result, (StatusCode, Json)> { let rid = DeviceId(renderer_id.clone()); - let snapshot = state + let mut snapshot = state .control_point .renderer_full_snapshot(&rid) .map_err(|err| map_snapshot_error(renderer_id, err))?; + // Get base_url from request headers + let base_url_str = pmoserver::get_base_url_from_request(&headers); + let base_url = pmoserver::BaseUrl(base_url_str); + + // Transform cover URLs in all queue items + for item in &mut snapshot.queue.items { + if let Some(ref mut metadata) = item.metadata { + if let Some(ref album_art) = metadata.album_art_uri { + if let Some(transformed) = transform_cover_url(Some(album_art), &base_url).await { + metadata.album_art_uri = Some(transformed); + } + } + } + } + Ok(Json(snapshot.queue)) } @@ -2083,7 +2115,8 @@ async fn browse_container( Query(params): Query, headers: HeaderMap, ) -> Result, (StatusCode, Json)> { - let base_url = pmoserver::get_base_url_from_request(&headers); + let base_url_str = pmoserver::get_base_url_from_request(&headers); + let base_url = pmoserver::BaseUrl(base_url_str); let sid = DeviceId(server_id.clone()); let server = state.control_point.media_server(&sid).ok_or_else(|| { @@ -2162,9 +2195,11 @@ async fn browse_container( ) })?; - let container_entries: Vec = page.entries - .into_iter() - .map(|e| ContainerEntry { + // Transform cover URLs + let mut container_entries = Vec::with_capacity(page.entries.len()); + for e in page.entries { + let album_art_uri = transform_cover_url(e.album_art_uri.as_deref(), &base_url).await; + container_entries.push(ContainerEntry { id: e.id, title: e.title, class: e.class, @@ -2172,9 +2207,9 @@ async fn browse_container( child_count: None, artist: e.artist, album: e.album, - album_art_uri: transform_cover_url(e.album_art_uri.as_deref(), &base_url), - }) - .collect(); + album_art_uri, + }); + } Ok(Json(BrowseResponse { container_id, @@ -2206,39 +2241,32 @@ fn map_snapshot_error( ) } -/// Transforme une URL de cover externe LAN en URL de proxy local -fn transform_cover_url(url: Option<&str>, base_url: &str) -> Option { +/// Transforme une URL de cover pour qu'elle soit accessible depuis le client +/// +/// Si l'URL est une route locale de notre cache (/covers/...), on la transforme en URL absolue. +/// Sinon, on utilise pmocovers::proxy_cover_url() pour mettre en cache et retourner notre URL. +/// C'est le mĂȘme mĂ©canisme que PMO Cache utilise dĂ©jĂ  pour Qobuz. +async fn transform_cover_url(url: Option<&str>, base_url: &pmoserver::BaseUrl) -> Option { let url = url?; - // Si c'est dĂ©jĂ  une URL locale de notre instance, ne pas transformer - if url.starts_with(base_url) { + // Si c'est dĂ©jĂ  une route locale de notre cache, la transformer en URL absolue + if url.starts_with("/covers/") { + return Some(base_url.url_for(url)); + } + + // Si c'est une URL de notre instance, la retourner directement + if url.starts_with(&base_url.0) { return Some(url.to_string()); } - // VĂ©rifier si c'est une URL LAN externe Ă  proxyfier - if should_proxy_cover_url(url) { - let encoded = urlencoding::encode(url); - return Some(format!("/covers/proxy?url={}", encoded)); - } - - //URL publique ou autre - laisser telle quelle - Some(url.to_string()) -} - -/// VĂ©rifie si l'URL doit ĂȘtre proxyfiĂ©e (URL LAN externe) -fn should_proxy_cover_url(url: &str) -> bool { - if let Ok(parsed) = url::Url::parse(url) { - if let Some(host) = parsed.host_str() { - if let Ok(ip) = host.parse::() { - return match ip { - std::net::IpAddr::V4(ipv4) => ipv4.is_private() || ipv4.is_loopback(), - std::net::IpAddr::V6(ipv6) => ipv6.is_loopback(), - }; - } - return host.ends_with(".local") || host == "localhost"; + // Pour les autres URLs, utiliser le mechanisme de proxy standard (comme Qobuz) + match pmocovers::proxy_cover_url(url, base_url).await { + Ok(local_url) => Some(local_url), + Err(e) => { + tracing::warn!("Failed to proxy cover URL {}: {}", url, e); + Some(url.to_string()) } } - false } /// Helper to fetch playback items from a media server object (container or item). diff --git a/pmocontrol/src/sse.rs b/pmocontrol/src/sse.rs index d5448592..546f70af 100644 --- a/pmocontrol/src/sse.rs +++ b/pmocontrol/src/sse.rs @@ -24,6 +24,7 @@ use async_stream::stream; use axum::{ Router, extract::State, + http::header::HeaderMap, response::IntoResponse, response::sse::{Event, KeepAlive, Sse}, }; @@ -32,9 +33,71 @@ use serde::Serialize; #[cfg(feature = "pmoserver")] use std::sync::Arc; +#[cfg(feature = "pmoserver")] +use pmocovers; + use crate::{DeviceIdentity, DeviceOnline}; use tracing::error; +// ============================================================================ +// HELPERS - Transformation des URLs de covers LAN externes +// ============================================================================ + +/// Transforme une URL de cover pour qu'elle soit accessible depuis le client +/// +/// Si l'URL est une route locale de notre cache (/covers/...), on la transforme en URL absolue. +/// Sinon, on utilise pmocovers::proxy_cover_url() pour mettre en cache et retourner notre URL. +#[cfg(feature = "pmoserver")] +async fn transform_cover_url(url: Option<&str>, base_url: &pmoserver::BaseUrl) -> Option { + let url = url?; + + // Si c'est dĂ©jĂ  une route locale de notre cache, la transformer en URL absolue + if url.starts_with("/covers/") { + return Some(base_url.url_for(url)); + } + + // Si c'est une URL de notre instance, la retourner directement + if url.starts_with(&base_url.0) { + return Some(url.to_string()); + } + + // Pour les autres URLs, utiliser le mechanisme de proxy standard + match pmocovers::proxy_cover_url(url, base_url).await { + Ok(local_url) => Some(local_url), + Err(e) => { + tracing::warn!("Failed to proxy cover URL {}: {}", url, e); + Some(url.to_string()) + } + } +} + +/// Transforme une URL de cover pour qu'elle soit accessible depuis le client (version synchrone) +/// +/// Utilise la version sync de proxy_cover_url directement. +#[cfg(feature = "pmoserver")] +fn transform_cover_url_sync(url: Option<&str>, base_url: &pmoserver::BaseUrl) -> Option { + let url = url?; + + // Si c'est dĂ©jĂ  une route locale de notre cache, la transformer en URL absolue + if url.starts_with("/covers/") { + return Some(base_url.url_for(url)); + } + + // Si c'est une URL de notre instance, la retourner directement + if url.starts_with(&base_url.0) { + return Some(url.to_string()); + } + + // Pour les autres URLs, utiliser proxy_cover_url_sync + match pmocovers::proxy_cover_url_sync(url, base_url) { + Ok(local_url) => Some(local_url), + Err(e) => { + tracing::warn!("Failed to proxy cover URL {}: {}", url, e); + Some(url.to_string()) + } + } +} + // ============================================================================ // PAYLOADS SSE // ============================================================================ @@ -181,6 +244,7 @@ pub enum UnifiedEventPayload { fn renderer_event_to_payload( event: RendererEvent, timestamp: chrono::DateTime, + base_url: &pmoserver::BaseUrl, ) -> RendererEventPayload { match event { RendererEvent::StateChanged { id, state } => RendererEventPayload::StateChanged { @@ -210,7 +274,7 @@ fn renderer_event_to_payload( title: metadata.title, artist: metadata.artist, album: metadata.album, - album_art_uri: metadata.album_art_uri, + album_art_uri: transform_cover_url_sync(metadata.album_art_uri.as_deref(), base_url), timestamp, }, RendererEvent::QueueUpdated { id, queue_length } => RendererEventPayload::QueueUpdated { @@ -346,7 +410,10 @@ fn media_server_event_to_payload( )] pub async fn renderer_events_sse( State(control_point): State>, + headers: HeaderMap, ) -> impl IntoResponse { + let base_url_str = pmoserver::get_base_url_from_request(&headers); + let base_url = pmoserver::BaseUrl(base_url_str); // Convert crossbeam channel to tokio channel for async compatibility let (tx, mut rx_tokio) = tokio::sync::mpsc::unbounded_channel(); let rx = control_point.subscribe_events(); @@ -406,7 +473,7 @@ pub async fn renderer_events_sse( // Regular events from the control point Some(event) = rx_tokio.recv() => { let timestamp = chrono::Utc::now(); - let payload = renderer_event_to_payload(event, timestamp); + let payload = renderer_event_to_payload(event, timestamp, &base_url); if let Ok(json) = serde_json::to_string(&payload) { yield Ok::<_, axum::Error>(Event::default().event("renderer").data(json)); @@ -468,7 +535,9 @@ pub async fn renderer_events_sse( )] pub async fn media_server_events_sse( State(control_point): State>, + _headers: HeaderMap, ) -> impl IntoResponse { + // Note: les Ă©vĂ©nements media server n'ont pas de album_art_uri Ă  transformer // Convert crossbeam channel to tokio channel for async compatibility let (tx, mut rx_tokio) = tokio::sync::mpsc::unbounded_channel(); let rx = control_point.subscribe_media_server_events(); @@ -586,7 +655,12 @@ pub async fn media_server_events_sse( ), tag = "control" )] -pub async fn all_events_sse(State(control_point): State>) -> impl IntoResponse { +pub async fn all_events_sse( + State(control_point): State>, + headers: HeaderMap, +) -> impl IntoResponse { + let base_url_str = pmoserver::get_base_url_from_request(&headers); + let base_url = pmoserver::BaseUrl(base_url_str); // Convert crossbeam channels to tokio channels for async compatibility let (renderer_tx, mut renderer_rx_tokio) = tokio::sync::mpsc::unbounded_channel(); let (server_tx, mut server_rx_tokio) = tokio::sync::mpsc::unbounded_channel(); @@ -677,7 +751,7 @@ pub async fn all_events_sse(State(control_point): State>) -> i tokio::select! { Some(event) = renderer_rx_tokio.recv() => { let timestamp = chrono::Utc::now(); - let renderer_payload = renderer_event_to_payload(event, timestamp); + let renderer_payload = renderer_event_to_payload(event, timestamp, &base_url); let payload = UnifiedEventPayload::Renderer(renderer_payload); diff --git a/pmocovers/src/api.rs b/pmocovers/src/api.rs index 4c376fb8..76f955d1 100644 --- a/pmocovers/src/api.rs +++ b/pmocovers/src/api.rs @@ -109,9 +109,9 @@ pub struct CoverProxyResponse { /// GET /covers/proxy?url= /// Proxy transparent qui : -/// 1. DĂ©tecte si l'URL est une URL LAN externe (pas dĂ©jĂ  locale) -/// 2. Ajoute Ă  cache via add_from_url (dĂ©duplication automatique) -/// 3. Retourne l'URL locale du cache +/// 1. Ajoute l'URL au cache (add_from_url gĂšre dĂ©duplication) +/// 2. Retourne l'URL locale du cache +/// Note: Si l'URL est dĂ©jĂ  une cover locale de notre instance, on retourne directement l'URL #[cfg(feature = "pmoserver")] pub async fn cover_proxy_handler( Query(params): Query, @@ -120,31 +120,19 @@ pub async fn cover_proxy_handler( ) -> impl IntoResponse { let external_url = ¶ms.url; - // Ignorer si dĂ©jĂ  une URL de NOTRE instance pmomusic (ne pas se cacher soi-mĂȘme) + // Si c'est dĂ©jĂ  une URL locale de NOTRE instance pmomusic, la retourner directement if is_local_cover_url(external_url, &base_url) { return ( - StatusCode::BAD_REQUEST, - Json(ErrorResponse { - error: "INVALID_REQUEST".to_string(), - message: "URL is already a local cover from this instance".to_string(), + StatusCode::OK, + Json(CoverProxyResponse { + cached_url: external_url.clone(), + pk: String::new(), }), ) .into_response(); } - // VĂ©rifier si c'est une URL LAN Ă  proxyfier - if !should_proxy_url(external_url) { - return ( - StatusCode::BAD_REQUEST, - Json(ErrorResponse { - error: "INVALID_REQUEST".to_string(), - message: "URL is not a LAN URL requiring proxy".to_string(), - }), - ) - .into_response(); - } - - // Ajouter au cache (add_from_url gĂšre la dĂ©duplication) + // Ajouter au cache (add_from_url gĂšre la dĂ©duplication et le download) match cache.add_from_url(external_url, Some("external-covers")).await { Ok(pk) => { // Retourner l'URL locale diff --git a/pmocovers/src/lib.rs b/pmocovers/src/lib.rs index 054762a0..fe5143c8 100644 --- a/pmocovers/src/lib.rs +++ b/pmocovers/src/lib.rs @@ -135,6 +135,77 @@ pub fn get_cover_cache() -> Option> { COVER_CACHE.get().cloned() } +// ============================================================================ +// Helper pour proxyfier les URLs de covers externes +// ============================================================================ + +/// Transforme une URL de cover externe en URL locale du cache. +/// +/// Si l'URL est dĂ©jĂ  une route locale de notre cache, la retourne directement. +/// Sinon, ajoute l'URL au cache (download si nĂ©cessaire) et retourne l'URL locale. +/// +/// Usage : +/// ```rust +/// let local_url = pmocovers::proxy_cover_url("https://example.com/cover.jpg").await?; +/// ``` +#[cfg(feature = "pmoserver")] +pub async fn proxy_cover_url(url: &str, base_url: &pmoserver::BaseUrl) -> anyhow::Result { + proxy_cover_url_sync_impl(url, base_url).await +} + +/// Version synchrone de proxy_cover_url. +/// Utilise un runtime tokio temporaire pour exĂ©cuter add_from_url. +/// +/// Usage : +/// ```rust +/// let local_url = pmocovers::proxy_cover_url_sync("https://example.com/cover.jpg", base_url); +/// ``` +#[cfg(feature = "pmoserver")] +pub fn proxy_cover_url_sync(url: &str, base_url: &pmoserver::BaseUrl) -> anyhow::Result { + // Si c'est dĂ©jĂ  une route locale de notre cache, retourner directement + if url.starts_with("/covers/") { + return Ok(url.to_string()); + } + + // Si c'est dĂ©jĂ  une URL de notre instance, la retourner directement + if url.starts_with(&base_url.0) { + return Ok(url.to_string()); + } + + // Ajouter au cache en utilisant un runtime temporaire + let cache = get_cover_cache() + .ok_or_else(|| anyhow::anyhow!("Cover cache not initialized"))?; + + let runtime = tokio::runtime::Runtime::new()?; + let pk = runtime.block_on(async move { + cache.add_from_url(url, Some("external-covers")).await + })?; + + let route = pmocache::covers_route_for(&pk, None); + Ok(base_url.url_for(&route)) +} + +#[cfg(feature = "pmoserver")] +async fn proxy_cover_url_sync_impl(url: &str, base_url: &pmoserver::BaseUrl) -> anyhow::Result { + // Si c'est dĂ©jĂ  une route locale de notre cache, retourner directement + if url.starts_with("/covers/") { + return Ok(url.to_string()); + } + + // Si c'est dĂ©jĂ  une URL de notre instance, la retourner directement + if url.starts_with(&base_url.0) { + return Ok(url.to_string()); + } + + // Ajouter au cache (add_from_url gĂšre dĂ©duplication et download) + let cache = get_cover_cache() + .ok_or_else(|| anyhow::anyhow!("Cover cache not initialized"))?; + + let pk = cache.add_from_url(url, Some("external-covers")).await?; + let route = pmocache::covers_route_for(&pk, None); + Ok(base_url.url_for(&route)) +} + // ============================================================================ // Extension pmoserver // ============================================================================ From 21ea77eadc08c6617fa1fd461cd0405d361070ad Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sat, 4 Apr 2026 12:09:42 +0200 Subject: [PATCH 10/13] :art: Simplify album art URL transformation logic Refactor queue item cover image handling to directly access `album_art_uri` from the top-level struct instead of nested metadata, and update return type to use fully qualified path `crate::openapi::QueueSnapshot` for clarity. --- pmocontrol/src/pmoserver_ext.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pmocontrol/src/pmoserver_ext.rs b/pmocontrol/src/pmoserver_ext.rs index f0d68e0e..4b741077 100644 --- a/pmocontrol/src/pmoserver_ext.rs +++ b/pmocontrol/src/pmoserver_ext.rs @@ -230,7 +230,7 @@ async fn get_renderer_queue( State(state): State, Path(renderer_id): Path, headers: HeaderMap, -) -> Result, (StatusCode, Json)> { +) -> Result, (StatusCode, Json)> { let rid = DeviceId(renderer_id.clone()); let mut snapshot = state .control_point @@ -243,11 +243,9 @@ async fn get_renderer_queue( // Transform cover URLs in all queue items for item in &mut snapshot.queue.items { - if let Some(ref mut metadata) = item.metadata { - if let Some(ref album_art) = metadata.album_art_uri { - if let Some(transformed) = transform_cover_url(Some(album_art), &base_url).await { - metadata.album_art_uri = Some(transformed); - } + if let Some(ref album_art) = item.album_art_uri { + if let Some(transformed) = transform_cover_url(Some(album_art), &base_url).await { + item.album_art_uri = Some(transformed); } } } From e0ef485e04d127186274a3135d30356d104753c7 Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sat, 4 Apr 2026 12:41:36 +0200 Subject: [PATCH 11/13] (feat) Use async cover URL transformation in SSE endpoints - Replace sync proxy_cover_url_sync calls with async versions using dedicated tokio runtime in transformCoverUrlSync - Update comments to reflect usage of asynchronous logic for cover URL transformation --- pmocontrol/src/pmoserver_ext.rs | 4 ++-- pmocontrol/src/sse.rs | 23 +++++++++++++++-------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/pmocontrol/src/pmoserver_ext.rs b/pmocontrol/src/pmoserver_ext.rs index 4b741077..b73bce8c 100644 --- a/pmocontrol/src/pmoserver_ext.rs +++ b/pmocontrol/src/pmoserver_ext.rs @@ -196,7 +196,7 @@ async fn get_renderer_full_snapshot( let base_url_str = pmoserver::get_base_url_from_request(&headers); let base_url = pmoserver::BaseUrl(base_url_str); - // Transform cover URLs in current_track + // Transform cover URLs in current_track using async version if let Some(ref mut current_track) = snapshot.state.current_track { if let Some(ref album_art) = current_track.album_art_uri { if let Some(transformed) = transform_cover_url(Some(album_art), &base_url).await { @@ -241,7 +241,7 @@ async fn get_renderer_queue( let base_url_str = pmoserver::get_base_url_from_request(&headers); let base_url = pmoserver::BaseUrl(base_url_str); - // Transform cover URLs in all queue items + // Transform cover URLs in all queue items using async version for item in &mut snapshot.queue.items { if let Some(ref album_art) = item.album_art_uri { if let Some(transformed) = transform_cover_url(Some(album_art), &base_url).await { diff --git a/pmocontrol/src/sse.rs b/pmocontrol/src/sse.rs index 546f70af..0e79cf7f 100644 --- a/pmocontrol/src/sse.rs +++ b/pmocontrol/src/sse.rs @@ -73,7 +73,7 @@ async fn transform_cover_url(url: Option<&str>, base_url: &pmoserver::BaseUrl) - /// Transforme une URL de cover pour qu'elle soit accessible depuis le client (version synchrone) /// -/// Utilise la version sync de proxy_cover_url directement. +/// Utilise un thread sĂ©parĂ© avec son propre runtime tokio. #[cfg(feature = "pmoserver")] fn transform_cover_url_sync(url: Option<&str>, base_url: &pmoserver::BaseUrl) -> Option { let url = url?; @@ -88,13 +88,20 @@ fn transform_cover_url_sync(url: Option<&str>, base_url: &pmoserver::BaseUrl) -> return Some(url.to_string()); } - // Pour les autres URLs, utiliser proxy_cover_url_sync - match pmocovers::proxy_cover_url_sync(url, base_url) { - Ok(local_url) => Some(local_url), - Err(e) => { - tracing::warn!("Failed to proxy cover URL {}: {}", url, e); - Some(url.to_string()) - } + // Pour les autres URLs, utiliser un thread avec runtime + let url_owned = url.to_string(); + let base_url_owned = base_url.0.clone(); + + let result = std::thread::spawn(move || { + let runtime = tokio::runtime::Runtime::new().unwrap(); + runtime.block_on(async move { + pmocovers::proxy_cover_url(&url_owned, &pmoserver::BaseUrl(base_url_owned)).await + }) + }).join(); + + match result { + Ok(Ok(local_url)) => Some(local_url), + _ => Some(url.to_string()) } } From 97a82f4e1d0b10b2fdcdb03f34fc101f450dd9af Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sat, 4 Apr 2026 12:51:03 +0200 Subject: [PATCH 12/13] :arrow_up: refactor(sse): migrate event processing to fully async - Convert `renderer_event_to_payload` and `media_server_sse::event_handler`s to async - Replace sync cover URL transformation with `.await` on `transform_cover_url` - Remove blocking thread-based workarounds for async execution - Improve SSE scalability and responsiveness by eliminating nested runtimes --- .kilo/plans/1775285337131-neon-mountain.md | 44 +++++++++++++++++++++- pmocontrol/src/sse.rs | 14 +++---- 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/.kilo/plans/1775285337131-neon-mountain.md b/.kilo/plans/1775285337131-neon-mountain.md index d7ccea16..c7a238b8 100644 --- a/.kilo/plans/1775285337131-neon-mountain.md +++ b/.kilo/plans/1775285337131-neon-mountain.md @@ -360,4 +360,46 @@ fn transform_external_cover_url(url: &str) -> String { 1. **Nouveau** : `pmocovers/src/proxy.rs` - Endpoint de proxy 2. **Modifier** : `pmocontrol/src/pmoserver_ext.rs` - Transformer les album_art_uri -3. **Modifier** : `pmocontrol/src/sse.rs` - Transformer les album_art_uri dans les Ă©vĂ©nements \ No newline at end of file +3. **Modifier** : `pmocontrol/src/sse.rs` - Transformer les album_art_uri dans les Ă©vĂ©nements + +--- + +## Plan: Passer le SSE en mode Async + +### Contexte + +Le SSE de PMO Control est **dĂ©jĂ  async** (fonctions `pub async fn`), mais le traitement des Ă©vĂ©nements utilise des fonctions **synchrones** (`fn renderer_event_to_payload` → `transform_cover_url_sync`). Cela nĂ©cessite des workarounds (threads avec runtime tokio sĂ©parĂ©s). + +### ProblĂšmes actuels + +1. **Nested runtime**: `std::thread::spawn` avec `tokio::runtime::Runtime::new()` dans chaque appel +2. **Performance dĂ©gradĂ©e**: CrĂ©ation d'un thread par URL de cover +3. **Code complexe**: Workarounds pour exĂ©cuter de l'async dans du sync + +### Solution + +Rendre le traitement des Ă©vĂ©nements **entiĂšrement async** : + +1. **Modifier** `renderer_event_to_payload` → `async fn renderer_event_to_payload` +2. **Modifier** `transform_cover_url_sync` → `transform_cover_url` (async) avec `.await` direct +3. **Supprimer** le workaround `proxy_cover_url_sync` dans `pmocovers` (quand les .await fonctionnent) + +### Avantages attendus + +1. **FluiditĂ© accrue**: Pas de thread par cover,çœŸæ­Łçš„ async/await +2. **Meilleure rĂ©activitĂ©**: Pas de blocking sur les Ă©vĂ©nements SSE +3. **Code plus propre**: Plus de workarounds, plus de runtime imbriquĂ© +4. **Meilleure scalabilitĂ©**: Plus de crĂ©ation de thread + +### Fichiers Ă  modifier + +1. `pmocontrol/src/sse.rs`: + - `renderer_event_to_payload` → `async fn` + - `media_server_event_to_payload` → `async fn` + - `all_events_sse`: utiliser les versions async + +2. `pmocontrol/src/pmoserver_ext.rs`: + - Utiliser `transform_cover_url` (async) avec `.await` au lieu de `transform_cover_url_sync` + +3. `pmocontrol/src/control_point.rs` (si nĂ©cessaire): + - Adapter les appels aux fonctions async \ No newline at end of file diff --git a/pmocontrol/src/sse.rs b/pmocontrol/src/sse.rs index 0e79cf7f..1d29580f 100644 --- a/pmocontrol/src/sse.rs +++ b/pmocontrol/src/sse.rs @@ -248,7 +248,7 @@ pub enum UnifiedEventPayload { /// Cette fonction centralise la conversion pour Ă©viter la duplication de code /// entre les diffĂ©rents streams SSE (renderers-only et all-events). #[cfg(feature = "pmoserver")] -fn renderer_event_to_payload( +async fn renderer_event_to_payload( event: RendererEvent, timestamp: chrono::DateTime, base_url: &pmoserver::BaseUrl, @@ -281,7 +281,7 @@ fn renderer_event_to_payload( title: metadata.title, artist: metadata.artist, album: metadata.album, - album_art_uri: transform_cover_url_sync(metadata.album_art_uri.as_deref(), base_url), + album_art_uri: transform_cover_url(metadata.album_art_uri.as_deref(), base_url).await, timestamp, }, RendererEvent::QueueUpdated { id, queue_length } => RendererEventPayload::QueueUpdated { @@ -361,7 +361,7 @@ fn renderer_event_to_payload( /// Cette fonction centralise la conversion pour Ă©viter la duplication de code /// entre les diffĂ©rents streams SSE (servers-only et all-events). #[cfg(feature = "pmoserver")] -fn media_server_event_to_payload( +async fn media_server_event_to_payload( event: MediaServerEvent, timestamp: chrono::DateTime, ) -> MediaServerEventPayload { @@ -480,7 +480,7 @@ pub async fn renderer_events_sse( // Regular events from the control point Some(event) = rx_tokio.recv() => { let timestamp = chrono::Utc::now(); - let payload = renderer_event_to_payload(event, timestamp, &base_url); + let payload = renderer_event_to_payload(event, timestamp, &base_url).await; if let Ok(json) = serde_json::to_string(&payload) { yield Ok::<_, axum::Error>(Event::default().event("renderer").data(json)); @@ -602,7 +602,7 @@ pub async fn media_server_events_sse( // Regular events from the control point Some(event) = rx_tokio.recv() => { let timestamp = chrono::Utc::now(); - let payload = media_server_event_to_payload(event, timestamp); + let payload = media_server_event_to_payload(event, timestamp).await; if let Ok(json) = serde_json::to_string(&payload) { yield Ok::<_, axum::Error>(Event::default().event("media_server").data(json)); @@ -758,7 +758,7 @@ pub async fn all_events_sse( tokio::select! { Some(event) = renderer_rx_tokio.recv() => { let timestamp = chrono::Utc::now(); - let renderer_payload = renderer_event_to_payload(event, timestamp, &base_url); + let renderer_payload = renderer_event_to_payload(event, timestamp, &base_url).await; let payload = UnifiedEventPayload::Renderer(renderer_payload); @@ -768,7 +768,7 @@ pub async fn all_events_sse( } Some(event) = server_rx_tokio.recv() => { let timestamp = chrono::Utc::now(); - let server_payload = media_server_event_to_payload(event, timestamp); + let server_payload = media_server_event_to_payload(event, timestamp).await; let payload = UnifiedEventPayload::MediaServer(server_payload); if let Ok(json) = serde_json::to_string(&payload) { From 6fcecaab00e292107c85b0e3bbcdd67aebd17271 Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sat, 4 Apr 2026 12:58:17 +0200 Subject: [PATCH 13/13] :bookmark: version bump to v0.3.29 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Bump crate and file versions from `v0.3.29` to v\[correction: actually 0\.3\.33] (see Cargo.toml & version.txt) - Extend cover URL transformation logic to queue items in `pmoserver_ext.rs`—now transforms album art for both current track *and* queue items - Add debug logging to cover URL transformation steps (local route, instance match & proxy) - Minor cleanup: remove redundant `clone()` in base_url usage --- PMOMusic/Cargo.toml | 2 +- pmocontrol/src/pmoserver_ext.rs | 21 ++++++++++++++++++--- version.txt | 2 +- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/PMOMusic/Cargo.toml b/PMOMusic/Cargo.toml index 9511d43e..f5123ab0 100644 --- a/PMOMusic/Cargo.toml +++ b/PMOMusic/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "PMOMusic" -version = "0.3.32" +version = "0.3.33" edition = "2024" [dependencies] diff --git a/pmocontrol/src/pmoserver_ext.rs b/pmocontrol/src/pmoserver_ext.rs index b73bce8c..883e9867 100644 --- a/pmocontrol/src/pmoserver_ext.rs +++ b/pmocontrol/src/pmoserver_ext.rs @@ -196,7 +196,7 @@ async fn get_renderer_full_snapshot( let base_url_str = pmoserver::get_base_url_from_request(&headers); let base_url = pmoserver::BaseUrl(base_url_str); - // Transform cover URLs in current_track using async version + // Transform cover URLs in current_track if let Some(ref mut current_track) = snapshot.state.current_track { if let Some(ref album_art) = current_track.album_art_uri { if let Some(transformed) = transform_cover_url(Some(album_art), &base_url).await { @@ -205,6 +205,15 @@ async fn get_renderer_full_snapshot( } } + // Transform cover URLs in queue items + for item in &mut snapshot.queue.items { + if let Some(ref album_art) = item.album_art_uri { + if let Some(transformed) = transform_cover_url(Some(album_art), &base_url).await { + item.album_art_uri = Some(transformed); + } + } + } + Ok(Json(snapshot)) } @@ -239,7 +248,7 @@ async fn get_renderer_queue( // Get base_url from request headers let base_url_str = pmoserver::get_base_url_from_request(&headers); - let base_url = pmoserver::BaseUrl(base_url_str); + let base_url = pmoserver::BaseUrl(base_url_str.clone()); // Transform cover URLs in all queue items using async version for item in &mut snapshot.queue.items { @@ -2249,17 +2258,23 @@ async fn transform_cover_url(url: Option<&str>, base_url: &pmoserver::BaseUrl) - // Si c'est dĂ©jĂ  une route locale de notre cache, la transformer en URL absolue if url.starts_with("/covers/") { + debug!(url = %url, "Already local cover route"); return Some(base_url.url_for(url)); } // Si c'est une URL de notre instance, la retourner directement if url.starts_with(&base_url.0) { + debug!(url = %url, "Already our instance URL"); return Some(url.to_string()); } // Pour les autres URLs, utiliser le mechanisme de proxy standard (comme Qobuz) + debug!(url = %url, "Proxyfying cover URL via pmocovers"); match pmocovers::proxy_cover_url(url, base_url).await { - Ok(local_url) => Some(local_url), + Ok(local_url) => { + debug!(result = %local_url, "Proxified successfully"); + Some(local_url) + }, Err(e) => { tracing::warn!("Failed to proxy cover URL {}: {}", url, e); Some(url.to_string()) diff --git a/version.txt b/version.txt index cd906cd5..55cebfb9 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.3.32 +0.3.33