diff --git a/pmocontrol/src/control_point.rs b/pmocontrol/src/control_point.rs index beafa772..53dcd1a9 100644 --- a/pmocontrol/src/control_point.rs +++ b/pmocontrol/src/control_point.rs @@ -31,7 +31,9 @@ use crate::control_point::music_queue::MusicQueue; use crate::control_point::openhome_queue::OpenHomeQueue; use crate::discovery::DiscoveryManager; use crate::events::{MediaServerEventBus, RendererEventBus}; -use crate::media_server::{MediaBrowser, MediaEntry, MediaServerInfo, MusicServer, ServerId}; +use crate::media_server::{ + playback_item_from_entry, MediaBrowser, MediaEntry, MediaServerInfo, MusicServer, ServerId, +}; use crate::media_server_events::spawn_media_server_event_runtime; use crate::model::TrackMetadata; use crate::model::{MediaServerEvent, RendererEvent, RendererId, RendererInfo}; @@ -1820,23 +1822,29 @@ impl ControlPoint { "Attaching new playlist: clearing renderer queue" ); - // Clear the renderer's queue for OpenHome renderers - // We also sync the local cache to reflect the empty state, which will trigger - // refresh_attached_queue_for() to use replace_entire_playlist() instead of gentle sync + // Prepare the renderer for the new playlist (backend-agnostic) + let renderer = self.music_renderer_by_id(renderer_id).ok_or_else(|| { + anyhow!("Renderer {} not found", renderer_id.0) + })?; + renderer.clear_for_playlist_attach()?; + + // For OpenHome, also sync the local cache to reflect the empty state if self.runtime.uses_openhome_playlist(renderer_id) { - let renderer = self.openhome_renderer(renderer_id)?; - renderer.openhome_playlist_clear()?; - // Sync local cache to reflect the empty renderer state self.sync_openhome_playlist_for(renderer_id)?; - debug!( - renderer = renderer_id.0.as_str(), - "Cleared OpenHome renderer playlist and synced local cache" - ); - } else { - // For non-OpenHome renderers, use the standard clear_queue - self.clear_queue(renderer_id)?; } + // Clear the local queue (detach binding + clear runtime queue structure) + self.detach_playlist_binding(renderer_id, "attach_new_playlist"); + self.runtime.with_music_queue_mut(renderer_id, |queue| { + queue.clear_queue()?; + Ok(()) + })?; + + debug!( + renderer = renderer_id.0.as_str(), + "Cleared renderer and local queue for new playlist" + ); + let binding = PlaylistBinding { server_id: server_id.clone(), container_id: container_id.to_string(), @@ -2712,6 +2720,16 @@ fn refresh_attached_queue_for( } }; + debug!( + renderer = renderer_id.0.as_str(), + server = server_id.0.as_str(), + container = container_id.as_str(), + total_entries = entries.len(), + containers = entries.iter().filter(|e| e.is_container).count(), + items_count = entries.iter().filter(|e| !e.is_container).count(), + "Browse returned entries for playlist refresh" + ); + // Step 4: Convert MediaEntry to PlaybackItem let new_items: Vec = entries .iter() @@ -2719,11 +2737,12 @@ fn refresh_attached_queue_for( .collect(); if new_items.is_empty() { - debug!( + warn!( renderer = renderer_id.0.as_str(), server = server_id.0.as_str(), container = container_id.as_str(), - "Refreshed playlist is empty, clearing queue" + total_entries = entries.len(), + "Refreshed playlist is empty, clearing queue - all entries were filtered out" ); runtime.with_music_queue_mut(renderer_id, |queue| queue.clear_queue())?; runtime.invalidate_openhome_cache(renderer_id); @@ -2886,41 +2905,6 @@ fn refresh_attached_queue_for( Ok(()) } -/// Helper to convert a MediaEntry to a PlaybackItem. -fn playback_item_from_entry(server: &MusicServer, entry: &MediaEntry) -> Option { - // Ignore containers - if entry.is_container { - return None; - } - - // Skip "live stream" entries (heuristic from example) - if entry.title.to_ascii_lowercase().contains("live stream") { - return None; - } - - // Find an audio resource - let resource = entry.resources.iter().find(|res| res.is_audio())?; - - let metadata = TrackMetadata { - title: Some(entry.title.clone()), - artist: entry.artist.clone(), - album: entry.album.clone(), - genre: entry.genre.clone(), - album_art_uri: entry.album_art_uri.clone(), - date: entry.date.clone(), - track_number: entry.track_number.clone(), - creator: entry.creator.clone(), - }; - - Some(PlaybackItem { - media_server_id: server.id().clone(), - didl_id: entry.id.clone(), - uri: resource.uri.clone(), - protocol_info: resource.protocol_info.clone(), - metadata: Some(metadata), - }) -} - const OPENHOME_TRACK_PREFIX: &str = "openhome:"; fn playback_item_from_openhome_track( diff --git a/pmocontrol/src/media_server.rs b/pmocontrol/src/media_server.rs index 6410e57e..9c2f7f45 100644 --- a/pmocontrol/src/media_server.rs +++ b/pmocontrol/src/media_server.rs @@ -4,8 +4,11 @@ use anyhow::{Result, anyhow}; use pmodidl::{self, DIDLLite}; use pmoupnp::soap::SoapEnvelope; use pmoupnp::soap::error_codes; +use tracing::{debug, warn}; use xmltree::{Element, XMLNode}; +use crate::model::TrackMetadata; +use crate::queue_backend::PlaybackItem; use crate::soap_client::{SoapCallResult, invoke_upnp_action_with_timeout}; /// Unique identifier for a media server registered by the control point. @@ -42,15 +45,49 @@ impl MediaResource { /// Returns true if this resource represents audio content. pub fn is_audio(&self) -> bool { let lower = self.protocol_info.to_ascii_lowercase(); + + // Standard case: audio/* MIME types if lower.contains("audio/") { return true; } + + // List of known audio format subtypes (the part after the /) + // These are recognized regardless of the MIME type prefix + const AUDIO_FORMATS: &[&str] = &[ + "flac", "ogg", "opus", "vorbis", + "mp3", "mpeg", "mp4", "m4a", "aac", + "wav", "wave", "pcm", + "wma", "webm", + "ape", "alac", "aiff", + "dsd", "dsf", "dff", + ]; + + // Check if any known audio format appears in the protocol_info + for format in AUDIO_FORMATS { + if lower.contains(format) { + return true; + } + } + // protocolInfo format: protocol:network:contentFormat:additionalInfo - lower - .split(':') - .nth(2) - .map(|mime| mime.starts_with("audio/")) - .unwrap_or(false) + // Extract the MIME type (3rd field) for more precise checking + if let Some(mime) = lower.split(':').nth(2) { + // Check if it's audio/* or contains a known audio format + if mime.starts_with("audio/") { + return true; + } + + // Check the subtype (part after /) for known audio formats + if let Some(subtype) = mime.split('/').nth(1) { + for format in AUDIO_FORMATS { + if subtype.contains(format) { + return true; + } + } + } + } + + false } } @@ -145,6 +182,80 @@ impl MediaBrowser for MusicServer { } } +/// Helper to convert a MediaEntry to a PlaybackItem. +/// +/// This function filters out containers and entries without audio resources, +/// returning None for items that cannot be played. +pub fn playback_item_from_entry(server: &MusicServer, entry: &MediaEntry) -> Option { + // Ignore containers + if entry.is_container { + debug!( + server_id = server.id().0.as_str(), + entry_id = entry.id.as_str(), + title = entry.title.as_str(), + class = entry.class.as_str(), + "Skipping container entry" + ); + return None; + } + + // Skip "live stream" entries (heuristic from example) + if entry.title.to_ascii_lowercase().contains("live stream") { + debug!( + server_id = server.id().0.as_str(), + entry_id = entry.id.as_str(), + title = entry.title.as_str(), + "Skipping 'live stream' entry" + ); + return None; + } + + // Find an audio resource + let resource = entry.resources.iter().find(|res| res.is_audio()); + + if resource.is_none() { + warn!( + server_id = server.id().0.as_str(), + entry_id = entry.id.as_str(), + title = entry.title.as_str(), + class = entry.class.as_str(), + resource_count = entry.resources.len(), + resources = ?entry.resources.iter().map(|r| &r.protocol_info).collect::>(), + "No audio resource found for entry" + ); + return None; + } + + let resource = resource.unwrap(); + + let metadata = TrackMetadata { + title: Some(entry.title.clone()), + artist: entry.artist.clone(), + album: entry.album.clone(), + genre: entry.genre.clone(), + album_art_uri: entry.album_art_uri.clone(), + date: entry.date.clone(), + track_number: entry.track_number.clone(), + creator: entry.creator.clone(), + }; + + debug!( + server_id = server.id().0.as_str(), + entry_id = entry.id.as_str(), + title = entry.title.as_str(), + uri = resource.uri.as_str(), + "Created playback item" + ); + + Some(PlaybackItem { + media_server_id: server.id().clone(), + didl_id: entry.id.clone(), + uri: resource.uri.clone(), + protocol_info: resource.protocol_info.clone(), + metadata: Some(metadata), + }) +} + /// Single UPnP ContentDirectory backend implementation. #[derive(Clone, Debug)] pub struct UpnpMediaServer { diff --git a/pmocontrol/src/music_renderer.rs b/pmocontrol/src/music_renderer.rs index 7040e54e..4080a89d 100644 --- a/pmocontrol/src/music_renderer.rs +++ b/pmocontrol/src/music_renderer.rs @@ -269,6 +269,41 @@ impl MusicRenderer { } } + /// High-level method to prepare the renderer for attaching a new playlist. + /// + /// This method handles backend-specific clearing logic: + /// - For OpenHome: clears the OpenHome playlist + /// - For AVTransport/Chromecast/etc.: stops the renderer (since they don't have a persistent queue) + /// + /// This should be called by ControlPoint when attaching a new playlist, ensuring that: + /// - Any currently playing content is stopped + /// - The renderer is in a clean state ready to receive new content + pub fn clear_for_playlist_attach(&self) -> Result<()> { + match self { + MusicRenderer::OpenHome(_) => { + // For OpenHome: clear the playlist on the renderer itself + self.openhome_playlist_clear() + } + MusicRenderer::Upnp(_) + | MusicRenderer::Chromecast(_) + | MusicRenderer::LinkPlay(_) + | MusicRenderer::ArylicTcp(_) + | MusicRenderer::HybridUpnpArylic { .. } => { + // For AVTransport and other single-track renderers: stop playback + // This ensures we're not in the middle of playing when we start the new playlist + self.stop().or_else(|err| { + // If stop fails (e.g., already stopped), that's fine - we just want to ensure it's not playing + warn!( + renderer = self.id().0.as_str(), + error = %err, + "Stop failed when preparing for playlist attach (continuing anyway)" + ); + Ok(()) + }) + } + } + } + pub fn openhome_playlist_add_track( &self, uri: &str, diff --git a/pmocontrol/src/pmoserver_ext.rs b/pmocontrol/src/pmoserver_ext.rs index 4db91f16..bf77432a 100644 --- a/pmocontrol/src/pmoserver_ext.rs +++ b/pmocontrol/src/pmoserver_ext.rs @@ -8,7 +8,9 @@ use crate::control_point::{ ControlPoint, OpenHomeAccessError, OPENHOME_SNAPSHOT_CACHE_TTL, }; #[cfg(feature = "pmoserver")] -use crate::media_server::{MediaBrowser, MediaEntry, MusicServer, ServerId}; +use crate::media_server::{ + playback_item_from_entry, MediaBrowser, MediaEntry, MusicServer, ServerId, +}; #[cfg(feature = "pmoserver")] use crate::model::{RendererCapabilities, RendererId, RendererProtocol, TrackMetadata}; #[cfg(feature = "pmoserver")] @@ -1924,51 +1926,33 @@ fn fetch_playback_items( // Browse the object to get entries let entries = music_server.browse_children(object_id, 0, BROWSE_PAGE_SIZE)?; + debug!( + server_id = server_id.0.as_str(), + object_id = object_id, + total_entries = entries.len(), + containers = entries.iter().filter(|e| e.is_container).count(), + items_count = entries.iter().filter(|e| !e.is_container).count(), + "Browse returned entries" + ); + // Convert to PlaybackItem let items: Vec = entries .iter() .filter_map(|entry| playback_item_from_entry(&music_server, entry)) .collect(); + if items.is_empty() && !entries.is_empty() { + warn!( + server_id = server_id.0.as_str(), + object_id = object_id, + total_entries = entries.len(), + "No playable items found - all entries were filtered out" + ); + } + Ok(items) } -/// Helper to convert a MediaEntry to a PlaybackItem. -#[cfg(feature = "pmoserver")] -fn playback_item_from_entry(server: &MusicServer, entry: &MediaEntry) -> Option { - // Ignore containers - if entry.is_container { - return None; - } - - // Skip "live stream" entries - if entry.title.to_ascii_lowercase().contains("live stream") { - return None; - } - - // Find an audio resource - let resource = entry.resources.iter().find(|res| res.is_audio())?; - - let metadata = TrackMetadata { - title: Some(entry.title.clone()), - artist: entry.artist.clone(), - album: entry.album.clone(), - genre: entry.genre.clone(), - album_art_uri: entry.album_art_uri.clone(), - date: entry.date.clone(), - track_number: entry.track_number.clone(), - creator: entry.creator.clone(), - }; - - Some(PlaybackItem { - media_server_id: server.id().clone(), - didl_id: entry.id.clone(), - uri: resource.uri.clone(), - protocol_info: resource.protocol_info.clone(), - metadata: Some(metadata), - }) -} - #[cfg(feature = "pmoserver")] fn protocol_summary(protocol: &RendererProtocol) -> RendererProtocolSummary { match protocol {