Skip to content

Commit

Permalink
Some stability and bigfixes (#1043)
Browse files Browse the repository at this point in the history
* fix reloading of sonos provider

* fix thread safety issue

* fix reload of sonos provider

* handle changed IP from sonos player

* fix some small typos

* some more typos

* Improve polling to detect players go offline

* fix for unresumable radio streams

* fix pause on slimproto

* fix for some radio stations not playing due to parse error

handover to ffmpeg

* fix for replace next
  • Loading branch information
marcelveldt authored Jan 29, 2024
1 parent 189520f commit b9822a3
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 50 deletions.
11 changes: 10 additions & 1 deletion music_assistant/server/controllers/player_queues.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,12 @@ async def play_media(
self.domain, f"default_enqueue_action_{media_item.media_type.value}"
)
)
if option == QueueOption.REPLACE_NEXT and queue.state not in (
PlayerState.PLAYING,
PlayerState.PAUSED,
):
# replace next requested but nothing is playing
option = QueueOption.REPLACE
# clear queue if needed
if option == QueueOption.REPLACE:
self.clear(queue_id)
Expand Down Expand Up @@ -617,6 +623,9 @@ async def resume(self, queue_id: str, fade_in: bool | None = None) -> None:
if resume_item is not None:
resume_pos = resume_pos if resume_pos > 10 else 0
fade_in = fade_in if fade_in is not None else resume_pos > 0
if resume_item.media_type == MediaType.RADIO:
# we're not able to skip in online radio so this is pointless
resume_pos = 0
await self.play_index(queue_id, resume_item.queue_item_id, resume_pos, fade_in)
else:
raise QueueEmpty(f"Resume queue requested but queue {queue_id} is empty")
Expand Down Expand Up @@ -979,7 +988,7 @@ async def _enqueue_next(index: int, supports_enqueue: bool = False):
if PlayerFeature.ENQUEUE_NEXT in player.supported_features:
# player supports enqueue next feature.
# we enqueue the next track after a new track
# has started playing and before the current track ends
# has started playing and (repeat) before the current track ends
new_track_started = new_state.get("state") == PlayerState.PLAYING and prev_state.get(
"current_index"
) != new_state.get("current_index")
Expand Down
6 changes: 3 additions & 3 deletions music_assistant/server/controllers/players.py
Original file line number Diff line number Diff line change
Expand Up @@ -831,13 +831,13 @@ async def _poll_players(self) -> None:
if player_playing:
self.mass.loop.call_soon(self.update, player_id)
# Poll player;
# - every 360 seconds if the player if not powered
# - every 120 seconds if the player if not powered
# - every 30 seconds if the player is powered
# - every 10 seconds if the player is playing
if (
(player.powered and count % 30 == 0)
or (player_playing and count % 10 == 0)
or count == 360
or count % 120 == 0
) and (player_prov := self.get_player_provider(player_id)):
try:
await player_prov.poll_player(player_id)
Expand All @@ -855,7 +855,7 @@ async def _poll_players(self) -> None:
finally:
# always update player state
self.mass.loop.call_soon(self.update, player_id)
if count >= 360:
if count >= 120:
count = 0
await asyncio.sleep(1)

Expand Down
15 changes: 9 additions & 6 deletions music_assistant/server/helpers/audio.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from typing import TYPE_CHECKING

import aiofiles
from aiohttp import ClientTimeout
from aiohttp import ClientResponseError, ClientTimeout

from music_assistant.common.models.errors import (
AudioError,
Expand Down Expand Up @@ -539,11 +539,14 @@ async def resolve_radio_stream(mass: MusicAssistant, url: str) -> tuple[str, boo
# determine ICY metadata support by looking at the http headers
headers = {"Icy-MetaData": "1", "User-Agent": "VLC/3.0.2.LibVLC/3.0.2"}
timeout = ClientTimeout(total=0, connect=10, sock_read=5)
async with mass.http_session.head(
url, headers=headers, allow_redirects=True, timeout=timeout
) as resp:
headers = resp.headers
supports_icy = int(headers.get("icy-metaint", "0")) > 0
try:
async with mass.http_session.head(
url, headers=headers, allow_redirects=True, timeout=timeout
) as resp:
headers = resp.headers
supports_icy = int(headers.get("icy-metaint", "0")) > 0
except ClientResponseError as err:
LOGGER.debug("Error while parsing radio URL %s: %s", url, err)

result = (url, supports_icy)
await mass.cache.set(cache_key, result)
Expand Down
1 change: 1 addition & 0 deletions music_assistant/server/providers/slimproto/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,7 @@ async def _handle_player_update(self, client: SlimClient) -> None:
PlayerFeature.POWER,
PlayerFeature.SYNC,
PlayerFeature.VOLUME_SET,
PlayerFeature.PAUSE,
),
max_sample_rate=int(client.max_sample_rate),
supports_24bit=int(client.max_sample_rate) > 44100,
Expand Down
44 changes: 26 additions & 18 deletions music_assistant/server/providers/sonos/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
from typing import TYPE_CHECKING

import soco.config as soco_config
from requests.exceptions import Timeout
from soco import SoCoException, events_asyncio, zonegroupstate
from requests.exceptions import RequestException
from soco import events_asyncio, zonegroupstate
from soco.core import SoCo
from soco.discovery import discover

Expand All @@ -36,7 +36,7 @@
from music_assistant.common.models.errors import PlayerCommandFailed, PlayerUnavailableError
from music_assistant.common.models.player import DeviceInfo, Player
from music_assistant.common.models.queue_item import QueueItem
from music_assistant.constants import CONF_CROSSFADE, CONF_PLAYERS
from music_assistant.constants import CONF_CROSSFADE
from music_assistant.server.helpers.didl_lite import create_didl_metadata
from music_assistant.server.models.player_provider import PlayerProvider

Expand All @@ -55,6 +55,7 @@
PlayerFeature.VOLUME_MUTE,
PlayerFeature.VOLUME_SET,
PlayerFeature.ENQUEUE_NEXT,
PlayerFeature.PAUSE,
)

CONF_NETWORK_SCAN = "network_scan"
Expand Down Expand Up @@ -454,9 +455,12 @@ def do_discover():
for soco in discovered_devices:
try:
self._add_player(soco)
except (OSError, SoCoException, Timeout) as err:
except RequestException as err:
# player is offline
self.logger.debug("Failed to add SonosPlayer %s: %s", soco, err)
except Exception as err:
self.logger.warning(
"Failed to add SonosPlayer using %s: %s", soco, err, exc_info=err
"Failed to add SonosPlayer %s: %s", soco, err, exc_info=err
)
finally:
self._discovery_running = False
Expand All @@ -468,32 +472,30 @@ def reschedule():
self.mass.create_task(self._run_discovery())

# reschedule self once finished
self._discovery_reschedule_timer = self.mass.loop.call_later(300, reschedule)
self._discovery_reschedule_timer = self.mass.loop.call_later(1800, reschedule)

def _add_player(self, soco: SoCo) -> None:
"""Add discovered Sonos player."""
player_id = soco.uid
if player_id in self.sonosplayers:
return # already added
# check if existing player changed IP
if existing := self.sonosplayers.get(player_id):
if existing.soco.ip_address != soco.ip_address:
existing.update_ip(soco.ip_address)
return
if not soco.is_visible:
return
enabled = self.mass.config.get(f"{CONF_PLAYERS}/{player_id}/enabled", True)
enabled = self.mass.config.get_raw_player_config_value(player_id, "enabled", True)
if not enabled:
self.logger.debug("Ignoring disabled player: %s", player_id)
return

if soco not in soco.visible_zones:
return

speaker_info = soco.get_speaker_info(True, timeout=7)
if soco.uid not in self.boot_counts:
self.boot_counts[soco.uid] = soco.boot_seqnum
self.logger.debug("Adding new player: %s", speaker_info)
support_hires = speaker_info["model_name"] in HIRES_MODELS
self.sonosplayers[player_id] = sonos_player = SonosPlayer(
self,
soco=soco,
mass_player=Player(
if not (mass_player := self.mass.players.get(soco.uid)):
mass_player = Player(
player_id=soco.uid,
provider=self.domain,
type=PlayerType.PLAYER,
Expand All @@ -508,10 +510,16 @@ def _add_player(self, soco: SoCo) -> None:
),
max_sample_rate=48000 if support_hires else 44100,
supports_24bit=support_hires,
),
)
self.sonosplayers[player_id] = sonos_player = SonosPlayer(
self,
soco=soco,
mass_player=mass_player,
)
sonos_player.setup()
self.mass.loop.call_soon_threadsafe(self.mass.players.register, sonos_player.mass_player)
self.mass.loop.call_soon_threadsafe(
self.mass.players.register_or_update, sonos_player.mass_player
)

async def _enqueue_item(
self,
Expand Down
22 changes: 9 additions & 13 deletions music_assistant/server/providers/sonos/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,32 @@
from collections.abc import Callable
from typing import TYPE_CHECKING, Any, Concatenate, ParamSpec, TypeVar, overload

from requests.exceptions import Timeout
from soco import SoCo
from soco.exceptions import SoCoException, SoCoUPnPException

from music_assistant.common.models.errors import PlayerCommandFailed

if TYPE_CHECKING:
from . import SonosPlayer
from .media import SonosMedia


UID_PREFIX = "RINCON_"
UID_POSTFIX = "01400"
SONOS_SPEAKER_ACTIVITY = "sonos_speaker_activity"

_LOGGER = logging.getLogger(__name__)

_T = TypeVar("_T", bound="SonosPlayer | SonosMedia")
_T = TypeVar("_T", bound="SonosPlayer")
_R = TypeVar("_R")
_P = ParamSpec("_P")

_FuncType = Callable[Concatenate[_T, _P], _R]
_ReturnFuncType = Callable[Concatenate[_T, _P], _R | None]


class SonosUpdateError(PlayerCommandFailed):
"""Update failed."""


@overload
def soco_error(
errorcodes: None = ...,
Expand All @@ -55,7 +57,7 @@ def wrapper(self: _T, *args: _P.args, **kwargs: _P.kwargs) -> _R | None:
args_soco = next((arg for arg in args if isinstance(arg, SoCo)), None)
try:
result = funct(self, *args, **kwargs)
except (OSError, SoCoException, SoCoUPnPException, Timeout) as err:
except (OSError, SoCoException, SoCoUPnPException, TimeoutError) as err:
error_code = getattr(err, "error_code", None)
function = funct.__qualname__
if errorcodes and error_code in errorcodes:
Expand All @@ -66,7 +68,7 @@ def wrapper(self: _T, *args: _P.args, **kwargs: _P.kwargs) -> _R | None:
raise RuntimeError("Unexpected use of soco_error") from err

message = f"Error calling {function} on {target}: {err}"
raise PlayerCommandFailed(message) from err
raise SonosUpdateError(message) from err

return result

Expand All @@ -77,15 +79,9 @@ def wrapper(self: _T, *args: _P.args, **kwargs: _P.kwargs) -> _R | None:

def _find_target_identifier(instance: Any, fallback_soco: SoCo | None) -> str | None:
"""Extract the best available target identifier from the provided instance object."""
if entity_id := getattr(instance, "entity_id", None):
# SonosEntity instance
return entity_id
if zone_name := getattr(instance, "zone_name", None):
# SonosSpeaker instance
# SonosPlayer instance
return zone_name
if speaker := getattr(instance, "speaker", None):
# Holds a SonosSpeaker instance attribute
return speaker.zone_name
if soco := getattr(instance, "soco", fallback_soco):
# Holds a SoCo instance attribute
# Only use attributes with no I/O
Expand Down
52 changes: 43 additions & 9 deletions music_assistant/server/providers/sonos/player.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@
from music_assistant.common.helpers.datetime import utc
from music_assistant.common.models.enums import PlayerFeature, PlayerState
from music_assistant.common.models.errors import PlayerCommandFailed
from music_assistant.common.models.player import Player
from music_assistant.server.providers.sonos.helpers import soco_error
from music_assistant.common.models.player import DeviceInfo, Player

from .helpers import SonosUpdateError, soco_error

if TYPE_CHECKING:
from . import SonosPlayerProvider
Expand Down Expand Up @@ -100,10 +101,6 @@ class SonosSubscriptionsFailed(PlayerCommandFailed):
"""Subscription creation failed."""


class SonosUpdateError(PlayerCommandFailed):
"""Update failed."""


class SonosPlayer:
"""Wrapper around Sonos/SoCo with some additional attributes."""

Expand Down Expand Up @@ -171,6 +168,13 @@ def missing_subscriptions(self) -> set[str]:
subscribed_services = {sub.service.service_type for sub in self._subscriptions}
return SUBSCRIPTION_SERVICES - subscribed_services

@property
def should_poll(self) -> bool:
"""Return if this player should be polled/pinged."""
if not self.available:
return True
return (time.monotonic() - self._last_activity) > 120

def setup(self) -> None:
"""Run initial setup of the speaker (NOT async friendly)."""
if self.soco.is_coordinator:
Expand Down Expand Up @@ -289,18 +293,41 @@ async def unsubscribe(self) -> None:

async def check_poll(self) -> None:
"""Validate availability of the speaker based on recent activity."""
if not (not self.available or (time.monotonic() - self._last_activity) > 600):
if not self.should_poll:
return
self.logger.debug("Polling player for availability...")
try:
await self.mass.create_task(self.ping)
await asyncio.to_thread(self.ping)
self._speaker_activity("ping")
except SonosUpdateError:
if not self.available:
return # already offline
self.logger.warning(
"No recent activity and cannot reach %s, marking unavailable",
self.zone_name,
)
await self.offline()

def update_ip(self, ip_address: str) -> None:
"""Handle updated IP of a Sonos player (NOT async friendly)."""
if self.available:
return
self.logger.info(
"Player IP-address changed from %s to %s", self.soco.ip_address, ip_address
)
try:
self.ping()
except SonosUpdateError:
return
self.soco.ip_address = ip_address
self.setup()
self.mass_player.device_info = DeviceInfo(
model=self.mass_player.device_info.model,
address=ip_address,
manufacturer=self.mass_player.device_info.manufacturer,
)
self.update_player()

@soco_error()
def ping(self) -> None:
"""Test device availability. Failure will raise SonosUpdateError."""
Expand Down Expand Up @@ -328,7 +355,7 @@ def update_player(self, signal_update: bool = True) -> None:
# send update to the player manager right away only if we are triggered from an event
# when we're just updating from a manual poll, the player manager
# will detect changes to the player object itself
self.sonos_prov.mass.players.update(self.player_id)
self.mass.loop.call_soon_threadsafe(self.sonos_prov.mass.players.update, self.player_id)

@soco_error()
def poll_track_info(self) -> dict[str, Any]:
Expand Down Expand Up @@ -668,6 +695,13 @@ def _update_attributes(self):
# generic attributes (player_info)
self.mass_player.available = self.available

if not self.available:
self.mass_player.powered = False
self.mass_player.state = PlayerState.IDLE
self.mass_player.synced_to = None
self.mass_player.group_childs = set()
return

# transport info (playback state)
self.mass_player.state = current_state = _convert_state(self.playback_status)

Expand Down

0 comments on commit b9822a3

Please sign in to comment.