diff --git a/music_assistant/controllers/media/base.py b/music_assistant/controllers/media/base.py index 69e5613d35..4e4b810c99 100644 --- a/music_assistant/controllers/media/base.py +++ b/music_assistant/controllers/media/base.py @@ -27,6 +27,7 @@ from music_assistant.constants import ( DB_TABLE_GENRE_MEDIA_ITEM_EXCLUSION, DB_TABLE_GENRE_MEDIA_ITEM_MAPPING, + DB_TABLE_LOUDNESS_MEASUREMENTS, DB_TABLE_PLAYLOG, DB_TABLE_PROVIDER_MAPPINGS, MASS_LOGGER_NAME, @@ -238,6 +239,16 @@ async def remove_item_from_library(self, item_id: str | int, recursive: bool = T "provider": prov_mapping.provider_instance, }, ) + # cleanup loudness measurements for this provider mapping + for prov_key in (prov_mapping.provider_domain, prov_mapping.provider_instance): + await self.mass.music.database.delete( + DB_TABLE_LOUDNESS_MEASUREMENTS, + { + "media_type": self.media_type.value, + "item_id": prov_mapping.item_id, + "provider": prov_key, + }, + ) # delete genre exclusions for this media item await self.mass.music.database.delete( DB_TABLE_GENRE_MEDIA_ITEM_EXCLUSION, diff --git a/music_assistant/controllers/music.py b/music_assistant/controllers/music.py index 2ea2f011a6..f5c0f7b9c7 100644 --- a/music_assistant/controllers/music.py +++ b/music_assistant/controllers/music.py @@ -683,12 +683,14 @@ async def in_progress_items( # An audiobook can be part of the library, in contrast to podcast episodes. # We then need to check the provider mappings table. + one_week_ago = int(utc_timestamp()) - (7 * 86400) query = ( "SELECT p.item_id, p.media_type, p.name, p.image, p.provider " f"FROM {DB_TABLE_PLAYLOG} p " "WHERE p.media_type IN ('audiobook', 'podcast_episode') " "AND p.fully_played = 0 " "AND p.seconds_played > 0 " + f"AND (p.media_type != 'podcast_episode' OR p.timestamp >= {one_week_ago}) " ) query += ( "AND ( " diff --git a/music_assistant/controllers/player_queues.py b/music_assistant/controllers/player_queues.py index 38418db5c0..4112348625 100644 --- a/music_assistant/controllers/player_queues.py +++ b/music_assistant/controllers/player_queues.py @@ -1596,6 +1596,10 @@ async def _load_item( fade_in=fade_in, prefer_album_loudness=bool(playing_album_tracks), ) + # update queue_item.duration from streamdetails if we got a better value + if queue_item.streamdetails.duration and not queue_item.duration: + queue_item.duration = queue_item.streamdetails.duration + self.signal_update(queue_id, items_changed=True) # pre-initialize the AudioBuffer so audio is ready # when the player requests it. For the current/first track this ensures diff --git a/music_assistant/controllers/players/protocol_linking.py b/music_assistant/controllers/players/protocol_linking.py index c60f60d6f1..e9abbb3ad9 100644 --- a/music_assistant/controllers/players/protocol_linking.py +++ b/music_assistant/controllers/players/protocol_linking.py @@ -606,6 +606,29 @@ def _check_merge_universal_players(self, universal_player: UniversalPlayer) -> N if not self._identifiers_match(universal_player, player, ""): continue + # Do not merge if both UPs have protocols from the same domain. + # Multiple instances of the same protocol on one host (e.g., several + # squeezelite players on the same VM) are separate devices that happen + # to share an IP. Merging them would orphan one instance's protocol. + domains_a = { + link.protocol_domain + for link in universal_player.linked_output_protocols + if link.protocol_domain + } + domains_b = { + link.protocol_domain + for link in player.linked_output_protocols + if link.protocol_domain + } + if domains_a & domains_b: + self.logger.debug( + "Skipping merge of %s and %s: shared protocol domain(s) %s", + universal_player.player_id, + player.player_id, + domains_a & domains_b, + ) + continue + # Determine which player absorbs the other (more protocols wins) keep, remove = ( (universal_player, player) diff --git a/music_assistant/controllers/streams/audio.py b/music_assistant/controllers/streams/audio.py index 87e06983ae..6cbf4170fa 100644 --- a/music_assistant/controllers/streams/audio.py +++ b/music_assistant/controllers/streams/audio.py @@ -262,10 +262,10 @@ async def get_stream_details( streamdetails.loudness = result[0] streamdetails.loudness_album = result[1] - if not streamdetails.duration: + if streamdetails.duration is None: if queue_item.media_item and queue_item.media_item.duration: streamdetails.duration = queue_item.media_item.duration - else: + elif queue_item.duration: streamdetails.duration = queue_item.duration if seek_position and (not streamdetails.allow_seek or not streamdetails.duration): self.logger.warning("seeking is not possible on duration-less streams!") @@ -1688,6 +1688,8 @@ async def _limited_fade_in() -> AsyncGenerator[bytes, None]: seconds_streamed = bytes_written / pcm_format.pcm_sample_size streamdetails.seconds_streamed = seconds_streamed streamdetails.duration = int(streamdetails.seek_position + seconds_streamed) + # propagate accurate duration to queue_item so UI displays it + queue_item.duration = streamdetails.duration self.logger.debug( "Finished Streaming queue track: %s (%s) on queue %s " "- crossfade data prepared for next track: %s", @@ -1969,6 +1971,8 @@ async def get_queue_flow_stream( queue_track.streamdetails.duration = int( queue_track.streamdetails.seek_position + seconds_streamed ) + # propagate accurate duration to queue_item so UI displays it + queue_track.duration = queue_track.streamdetails.duration play_log_entry.seconds_streamed = seconds_streamed play_log_entry.duration = queue_track.streamdetails.duration if last_play_log_entry is play_log_entry and last_fadeout_part: diff --git a/music_assistant/helpers/compare.py b/music_assistant/helpers/compare.py index a4a51ca04b..ea9eef55d1 100644 --- a/music_assistant/helpers/compare.py +++ b/music_assistant/helpers/compare.py @@ -146,6 +146,17 @@ def compare_track( # return early on exact item_id match if compare_item_ids(base_item, compare_item): return True + # tracks on the same album but different discs are always distinct, + # even if they share external IDs (e.g. same recording on multiple discs) + if ( + base_item.album + and compare_item.album + and base_item.disc_number + and compare_item.disc_number + and base_item.disc_number != compare_item.disc_number + and compare_album(base_item.album, compare_item.album, False) + ): + return False # return early on (un)matched primary/unique external id for ext_id in ( ExternalID.MB_RECORDING, diff --git a/music_assistant/helpers/images.py b/music_assistant/helpers/images.py index ae41f5e7c7..bb136d9c28 100644 --- a/music_assistant/helpers/images.py +++ b/music_assistant/helpers/images.py @@ -34,6 +34,7 @@ # Thumbnail cache: on-disk (persistent) + small in-memory FIFO (hot path) _THUMB_CACHE_DIR = "thumbnails" _THUMB_MEMORY_CACHE_MAX = 50 +_ALLOWED_THUMB_FORMATS: frozenset[str] = frozenset({"PNG", "JPEG"}) _thumb_memory_cache: OrderedDict[str, bytes] = OrderedDict() @@ -188,6 +189,9 @@ async def get_image_thumb( image_format = image_format.upper() if image_format == "JPG": image_format = "JPEG" + if image_format not in _ALLOWED_THUMB_FORMATS: + msg = f"Unsupported thumbnail format: {image_format}" + raise ValueError(msg) thumb_hash = _create_thumb_hash(provider, path_or_url) cache_filename = _thumb_cache_filename(thumb_hash, size, image_format) @@ -199,6 +203,10 @@ async def get_image_thumb( # 2. Check on-disk cache thumb_dir = os.path.join(mass.cache_path, _THUMB_CACHE_DIR) cache_filepath = os.path.join(thumb_dir, cache_filename) + resolved = os.path.realpath(cache_filepath) + if not resolved.startswith(os.path.realpath(thumb_dir) + os.sep): + msg = f"Cache path escapes thumbnail directory: {cache_filepath}" + raise OSError(msg) if await asyncio.to_thread(os.path.isfile, cache_filepath): async with aiofiles.open(cache_filepath, "rb") as f: thumb_data = cast("bytes", await f.read()) @@ -266,6 +274,10 @@ def _create_image() -> bytes: # Persist to disk cache (best-effort, don't fail on I/O errors) try: + resolved = os.path.realpath(cache_filepath) + thumb_dir = os.path.realpath(os.path.join(mass.cache_path, _THUMB_CACHE_DIR)) + if not resolved.startswith(thumb_dir + os.sep): + raise OSError("Cache path escapes thumbnail directory") await asyncio.to_thread(os.makedirs, os.path.dirname(cache_filepath), exist_ok=True) async with aiofiles.open(cache_filepath, "wb") as f: await f.write(thumb_data) diff --git a/music_assistant/helpers/playlists.py b/music_assistant/helpers/playlists.py index fe0ff4bc28..edf9753408 100644 --- a/music_assistant/helpers/playlists.py +++ b/music_assistant/helpers/playlists.py @@ -27,7 +27,7 @@ UniqueList, ) -from music_assistant.helpers.util import detect_charset +from music_assistant.helpers.util import detect_charset, try_parse_int if TYPE_CHECKING: from music_assistant.mass import MusicAssistant @@ -389,10 +389,7 @@ def construct_media_item_from_playlist_item( except ValueError: media_type = MediaType.TRACK name = metadata.get("name") or item.title or item.path - try: - duration = int(item.length) if item.length else 0 - except ValueError: - duration = 0 + duration = try_parse_int(item.length, default=None) if item.length else None provider_mappings = _resolve_provider_mappings(item, mass) external_ids = _collect_external_ids(metadata) @@ -428,18 +425,18 @@ def construct_media_item_from_playlist_item( item_id=item_id, provider=item_provider, name=name, - duration=duration, position=0, provider_mappings=provider_mappings, podcast=podcast_mapping, external_ids=external_ids, ) + if duration is not None: + media_item.duration = duration elif media_type == MediaType.AUDIOBOOK: media_item = Audiobook( item_id=item_id, provider=item_provider, name=name, - duration=duration, provider_mappings=provider_mappings, authors=UniqueList( metadata.get("authors", "").split("; ") if metadata.get("authors") else [] @@ -449,6 +446,8 @@ def construct_media_item_from_playlist_item( ), external_ids=external_ids, ) + if duration is not None: + media_item.duration = duration else: media_item = _construct_track( item, @@ -520,7 +519,7 @@ def _construct_track( item_id: str, item_provider: str, name: str, - duration: int, + duration: int | None, provider_mappings: set[ProviderMapping], external_ids: set[tuple[ExternalID, str]], ) -> Track: @@ -548,17 +547,19 @@ def _construct_track( version=item.album.version, media_type=MediaType.ALBUM, ) - return Track( + track = Track( item_id=item_id, provider=item_provider, name=name, version=metadata.get("version", ""), - duration=duration, artists=artists, album=album_mapping, provider_mappings=provider_mappings, external_ids=external_ids, ) + if duration is not None: + track.duration = duration + return track # --------------------------------------------------------------------------- # diff --git a/music_assistant/providers/airplay/helpers.py b/music_assistant/providers/airplay/helpers.py index 0d03b2cd2f..d6f76c7fb6 100644 --- a/music_assistant/providers/airplay/helpers.py +++ b/music_assistant/providers/airplay/helpers.py @@ -86,13 +86,12 @@ def _routing_lookup() -> str: def convert_airplay_volume(value: float) -> int: - """Remap AirPlay Volume to 0..100 scale.""" - airplay_min = -30 - airplay_max = 0 - normal_min = 0 - normal_max = 100 - portion = (value - airplay_min) * (normal_max - normal_min) / (airplay_max - airplay_min) - return int(portion + normal_min) + """Remap AirPlay dB volume (-30..0) to 0..100 scale.""" + airplay_min = -30.0 + airplay_max = 0.0 + value = max(airplay_min, min(airplay_max, value)) + portion = (value - airplay_min) * 100.0 / (airplay_max - airplay_min) + return max(0, min(100, round(portion))) def get_model_info(info: AsyncServiceInfo) -> tuple[str, str]: # noqa: PLR0911 diff --git a/music_assistant/providers/airplay/player.py b/music_assistant/providers/airplay/player.py index 4785333286..b8ffeef128 100644 --- a/music_assistant/providers/airplay/player.py +++ b/music_assistant/providers/airplay/player.py @@ -867,7 +867,7 @@ def update_volume_from_device(self, volume: int) -> None: return cur_volume = self.volume_level or 0 - if abs(cur_volume - volume) > 3 or (time.time() - self.last_command_sent) > 3: + if abs(cur_volume - volume) > 1 or (time.time() - self.last_command_sent) > 3: self.mass.create_task(self.volume_set(volume)) else: self._attr_volume_level = volume diff --git a/music_assistant/providers/apple_music/__init__.py b/music_assistant/providers/apple_music/__init__.py index 9dea05f2f1..62ed370204 100644 --- a/music_assistant/providers/apple_music/__init__.py +++ b/music_assistant/providers/apple_music/__init__.py @@ -935,6 +935,8 @@ def _parse_track( track.metadata.genres = set(genres) if composers := attributes.get("composerName"): track.metadata.performers = set(composers.split(", ")) + if content_rating := attributes.get("contentRating"): + track.metadata.explicit = content_rating == "explicit" if isrc := attributes.get("isrc"): track.external_ids.add((ExternalID.ISRC, isrc)) track.favorite = is_favourite or False diff --git a/music_assistant/providers/bluesound/const.py b/music_assistant/providers/bluesound/const.py index 123df3722f..a79617cd95 100644 --- a/music_assistant/providers/bluesound/const.py +++ b/music_assistant/providers/bluesound/const.py @@ -12,6 +12,7 @@ PLAYER_FEATURES_BASE = { PlayerFeature.PLAY_MEDIA, PlayerFeature.SET_MEMBERS, + PlayerFeature.VOLUME_SET, PlayerFeature.VOLUME_MUTE, PlayerFeature.PAUSE, PlayerFeature.SELECT_SOURCE, diff --git a/music_assistant/providers/bluesound/player.py b/music_assistant/providers/bluesound/player.py index 1cf66a7b79..4029463aa4 100644 --- a/music_assistant/providers/bluesound/player.py +++ b/music_assistant/providers/bluesound/player.py @@ -7,7 +7,7 @@ from typing import TYPE_CHECKING from music_assistant_models.config_entries import ConfigEntry, ConfigValueType -from music_assistant_models.enums import IdentifierType, PlaybackState, PlayerFeature +from music_assistant_models.enums import IdentifierType, PlaybackState from music_assistant_models.errors import PlayerCommandFailed from pyblu import Player as BluosPlayer from pyblu import Status, SyncStatus @@ -86,10 +86,7 @@ def requires_flow_mode(self) -> bool: async def setup(self) -> None: """Set up the player.""" - # Add volume support if available await self.update_attributes() - if self.discovery_info.get("zs"): - self._attr_supported_features.add(PlayerFeature.VOLUME_SET) await self.mass.players.register_or_update(self) async def get_config_entries( diff --git a/music_assistant/providers/bluesound/provider.py b/music_assistant/providers/bluesound/provider.py index c821e2bc9f..d6bc35fc38 100644 --- a/music_assistant/providers/bluesound/provider.py +++ b/music_assistant/providers/bluesound/provider.py @@ -28,7 +28,6 @@ class BluesoundDiscoveryInfo(TypedDict): port: str mac: str model: str - zs: bool class BluesoundPlayerProvider(PlayerProvider): @@ -96,7 +95,6 @@ async def on_mdns_service_state_change( port=str(port), mac=mac_address, model=info.decoded_properties.get("model", ""), - zs=info.decoded_properties.get("zs", False), ) # Create BluOS player diff --git a/music_assistant/providers/sendspin/player.py b/music_assistant/providers/sendspin/player.py index 0d812e4135..bac5e9f2da 100644 --- a/music_assistant/providers/sendspin/player.py +++ b/music_assistant/providers/sendspin/player.py @@ -681,7 +681,7 @@ async def send_current_media_metadata(self) -> None: elapsed_time = current_media.corrected_elapsed_time if elapsed_time is None: elapsed_time = self.corrected_elapsed_time if is_playing else self.elapsed_time - track_progress = int(elapsed_time * 1000) if elapsed_time is not None else 0 + track_progress = max(0, int(elapsed_time * 1000)) if elapsed_time is not None else 0 metadata = Metadata( title=current_media.title, diff --git a/tests/core/test_compare.py b/tests/core/test_compare.py index 578efbe815..b292d57dfe 100644 --- a/tests/core/test_compare.py +++ b/tests/core/test_compare.py @@ -369,6 +369,23 @@ def test_compare_track() -> None: # noqa: PLR0915 (ExternalID.MB_RECORDING, "abcd"), } assert compare.compare_track(track_a, track_b) is False + # test multi-disc: same album, same external IDs, different disc numbers should NOT match + track_a.external_ids = {(ExternalID.MB_RECORDING, "same-recording-id")} + track_b.external_ids = {(ExternalID.MB_RECORDING, "same-recording-id")} + track_a.album = media_items.ItemMapping(item_id="1", provider="test1", name="Album A") + track_b.album = media_items.ItemMapping(item_id="1", provider="test1", name="Album A") + track_a.disc_number = 1 + track_b.disc_number = 2 + track_a.track_number = 3 + track_b.track_number = 3 + assert compare.compare_track(track_a, track_b) is False + # same disc number should still match via external ID + track_b.disc_number = 1 + assert compare.compare_track(track_a, track_b) is True + # different disc but different albums should still match via external ID + track_b.disc_number = 2 + track_b.album = media_items.ItemMapping(item_id="2", provider="test1", name="Album B") + assert compare.compare_track(track_a, track_b) is True def test_compare_strings_case_insensitive_fuzzy() -> None: diff --git a/tests/core/test_protocol_linking.py b/tests/core/test_protocol_linking.py index 301aefacdf..ea1fcd615c 100644 --- a/tests/core/test_protocol_linking.py +++ b/tests/core/test_protocol_linking.py @@ -4727,6 +4727,82 @@ def capture_task(task: Awaitable[object]) -> None: assert "dlna_cached" in keep._protocol_player_ids assert config_store["players/dlna_cached/values/protocol_parent_id"] == "up_keep" + def test_no_merge_same_domain_protocols_on_same_ip(self, mock_mass: MagicMock) -> None: + """Test that UPs with same-domain protocols on the same IP are NOT merged. + + Scenario: 3 squeezelite instances on the same VM (same IP, different real MACs). + Each gets its own universal player. The merge check must NOT merge them, + because merging would combine protocols from the same domain. + + Reproduces: https://github.com/music-assistant/support/issues/5266 + """ + controller = PlayerController(mock_mass) + up_provider = create_mock_universal_provider(mock_mass) + + # Create 3 universal players, each with one squeezelite protocol + squeeze_provider = MockProvider("squeezelite", mass=mock_mass) + ups = [] + protocols = [] + for i in range(1, 4): + mac = f"00:00:00:00:00:{i:02X}" + up = UniversalPlayer( + provider=up_provider, + player_id=f"up00000000000{i}", + name=f"Zone {i}", + device_info=DeviceInfo(model="Squeezelite", manufacturer="Test"), + protocol_player_ids=[mac], + ) + up._attr_device_info.add_identifier(IdentifierType.MAC_ADDRESS, mac) + up._attr_device_info.add_identifier(IdentifierType.IP_ADDRESS, "192.168.1.50") + up._cache.clear() + up.update_state(signal_event=False) + up.set_initialized() + ups.append(up) + + proto = MockPlayer( + squeeze_provider, + mac, + f"squeezeplay: {mac}", + player_type=PlayerType.PROTOCOL, + identifiers={ + IdentifierType.MAC_ADDRESS: mac, + IdentifierType.IP_ADDRESS: "192.168.1.50", + }, + ) + proto.set_initialized() + protocols.append(proto) + + controller._players = {} + for up in ups: + controller._players[up.player_id] = up + for proto in protocols: + controller._players[proto.player_id] = proto + controller._player_throttlers = {k: Throttler(1, 0.05) for k in controller._players} + + # Link each protocol to its UP + for up, proto in zip(ups, protocols, strict=True): + controller._add_protocol_link(up, proto, "squeezelite") + + # Verify initial state: each protocol linked to its own UP + for up, proto in zip(ups, protocols, strict=True): + assert proto.protocol_parent_id == up.player_id + + # Now run merge check on each UP - none should merge + with patch.object(controller, "_save_universal_player_data"): + for up in ups: + controller._check_merge_universal_players(up) + + # All 3 UPs must still exist with their original protocol links + for up, proto in zip(ups, protocols, strict=True): + assert proto.protocol_parent_id == up.player_id, ( + f"Protocol {proto.player_id} should still be linked to {up.player_id}, " + f"but is linked to {proto.protocol_parent_id}" + ) + assert len(up.linked_output_protocols) == 1, ( + f"UP {up.player_id} should have exactly 1 protocol, " + f"but has {len(up.linked_output_protocols)}" + ) + class TestUniversalPlayerReplacement: """Tests for replacing a universal player with a native player."""