From cc69942d02d6deed6ae4b71898c82e1f54a7f0cf Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Fri, 5 Jul 2024 09:13:43 +0200 Subject: [PATCH 1/5] Fix check_output --- music_assistant/server/helpers/audio.py | 2 +- music_assistant/server/helpers/process.py | 19 ++++++------------- music_assistant/server/helpers/util.py | 2 +- .../server/providers/airplay/__init__.py | 3 ++- .../providers/filesystem_smb/__init__.py | 4 ++-- .../server/providers/snapcast/__init__.py | 2 +- .../server/providers/spotify/__init__.py | 6 +++--- 7 files changed, 16 insertions(+), 22 deletions(-) diff --git a/music_assistant/server/helpers/audio.py b/music_assistant/server/helpers/audio.py index 721c54470..61821d992 100644 --- a/music_assistant/server/helpers/audio.py +++ b/music_assistant/server/helpers/audio.py @@ -819,7 +819,7 @@ async def get_ffmpeg_stream( async def check_audio_support() -> tuple[bool, bool, str]: """Check if ffmpeg is present (with/without libsoxr support).""" # check for FFmpeg presence - returncode, output = await check_output(["ffmpeg", "-version"]) + returncode, output = await check_output("ffmpeg", "-version") ffmpeg_present = returncode == 0 and "FFmpeg" in output.decode() # use globals as in-memory cache diff --git a/music_assistant/server/helpers/process.py b/music_assistant/server/helpers/process.py index 73a010400..71c3a61d6 100644 --- a/music_assistant/server/helpers/process.py +++ b/music_assistant/server/helpers/process.py @@ -257,20 +257,13 @@ async def wait(self) -> int: return self._returncode -async def check_output(args: str | list[str]) -> tuple[int, bytes]: +async def check_output(*args: list[str]) -> tuple[int, bytes]: """Run subprocess and return returncode and output.""" - if isinstance(args, str): - proc = await asyncio.create_subprocess_shell( - args, - stderr=asyncio.subprocess.STDOUT, - stdout=asyncio.subprocess.PIPE, - ) - else: - proc = await asyncio.create_subprocess_exec( - *args, - stderr=asyncio.subprocess.STDOUT, - stdout=asyncio.subprocess.PIPE, - ) + proc = await asyncio.create_subprocess_exec( + *args, + stderr=asyncio.subprocess.STDOUT, + stdout=asyncio.subprocess.PIPE, + ) stdout, _ = await proc.communicate() return (proc.returncode, stdout) diff --git a/music_assistant/server/helpers/util.py b/music_assistant/server/helpers/util.py index fa1c0163a..352af3783 100644 --- a/music_assistant/server/helpers/util.py +++ b/music_assistant/server/helpers/util.py @@ -34,7 +34,7 @@ async def install_package(package: str) -> None: """Install package with pip, raise when install failed.""" LOGGER.debug("Installing python package %s", package) args = ["pip", "install", "--find-links", HA_WHEELS, package] - return_code, output = await check_output(args) + return_code, output = await check_output(*args) if return_code != 0: msg = f"Failed to install package {package}\n{output.decode()}" diff --git a/music_assistant/server/providers/airplay/__init__.py b/music_assistant/server/providers/airplay/__init__.py index aadab9aa1..6899e015f 100644 --- a/music_assistant/server/providers/airplay/__init__.py +++ b/music_assistant/server/providers/airplay/__init__.py @@ -818,7 +818,8 @@ async def _getcliraop_binary(self): async def check_binary(cliraop_path: str) -> str | None: try: returncode, output = await check_output( - [cliraop_path, "-check"], + cliraop_path, + "-check", ) if returncode == 0 and output.strip().decode() == "cliraop check": self.cliraop_bin = cliraop_path diff --git a/music_assistant/server/providers/filesystem_smb/__init__.py b/music_assistant/server/providers/filesystem_smb/__init__.py index 806898bc7..6a0c4759c 100644 --- a/music_assistant/server/providers/filesystem_smb/__init__.py +++ b/music_assistant/server/providers/filesystem_smb/__init__.py @@ -225,13 +225,13 @@ async def mount(self) -> None: [m.replace(password, "########") if password else m for m in mount_cmd], ) - returncode, output = await check_output(mount_cmd) + returncode, output = await check_output(*mount_cmd) if returncode != 0: msg = f"SMB mount failed with error: {output.decode()}" raise LoginFailed(msg) async def unmount(self, ignore_error: bool = False) -> None: """Unmount the remote share.""" - returncode, output = await check_output(["umount", self.base_path]) + returncode, output = await check_output("umount", self.base_path) if returncode != 0 and not ignore_error: self.logger.warning("SMB unmount failed with error: %s", output.decode()) diff --git a/music_assistant/server/providers/snapcast/__init__.py b/music_assistant/server/providers/snapcast/__init__.py index 73d829e91..73ec3d3e5 100644 --- a/music_assistant/server/providers/snapcast/__init__.py +++ b/music_assistant/server/providers/snapcast/__init__.py @@ -104,7 +104,7 @@ async def get_config_entries( action: [optional] action key called from config entries UI. values: the (intermediate) raw values for config entries sent with the action. """ - returncode, output = await check_output(["snapserver", "-v"]) + returncode, output = await check_output("snapserver", "-v") snapserver_version = int(output.decode().split(".")[1]) if returncode == 0 else -1 local_snapserver_present = snapserver_version >= 27 if returncode == 0 and not local_snapserver_present: diff --git a/music_assistant/server/providers/spotify/__init__.py b/music_assistant/server/providers/spotify/__init__.py index 87035247b..15b57a01d 100644 --- a/music_assistant/server/providers/spotify/__init__.py +++ b/music_assistant/server/providers/spotify/__init__.py @@ -696,7 +696,7 @@ async def _get_token(self): ] if self._ap_workaround: args += ["--ap-port", "12345"] - _returncode, output = await check_output(args) + _returncode, output = await check_output(*args) if _returncode == 0 and output.decode().strip() != "authorized": raise LoginFailed(f"Login failed for username {self.config.get_value(CONF_USERNAME)}") # get token with (authorized) librespot @@ -731,7 +731,7 @@ async def _get_token(self): ] if self._ap_workaround: args += ["--ap-port", "12345"] - _returncode, output = await check_output(args) + _returncode, output = await check_output(*args) duration = round(time.time() - time_start, 2) try: result = json.loads(output) @@ -873,7 +873,7 @@ async def get_librespot_binary(self): async def check_librespot(librespot_path: str) -> str | None: try: - returncode, output = await check_output([librespot_path, "--check"]) + returncode, output = await check_output(librespot_path, "--check") if returncode == 0 and b"ok spotty" in output and b"using librespot" in output: self._librespot_bin = librespot_path return librespot_path From 3dbcdebf536f7f5d7821738bd4185a35b1a5716d Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Fri, 5 Jul 2024 09:23:21 +0200 Subject: [PATCH 2/5] do not try to be smart on resume --- music_assistant/server/controllers/player_queues.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/music_assistant/server/controllers/player_queues.py b/music_assistant/server/controllers/player_queues.py index 384a51ba9..c29eef642 100644 --- a/music_assistant/server/controllers/player_queues.py +++ b/music_assistant/server/controllers/player_queues.py @@ -660,18 +660,9 @@ async def resume(self, queue_id: str, fade_in: bool | None = None) -> None: queue = self._queues[queue_id] queue_items = self._queue_items[queue_id] resume_item = queue.current_item - next_item = queue.next_item resume_pos = queue.elapsed_time - if ( - resume_item - and next_item - and resume_item.duration - and resume_pos > (resume_item.duration * 0.9) - ): - # track is already played for > 90% - skip to next - resume_item = next_item - resume_pos = 0 - elif not resume_item and queue.current_index is not None and len(queue_items) > 0: + + if not resume_item and queue.current_index is not None and len(queue_items) > 0: resume_item = self.get_item(queue_id, queue.current_index) resume_pos = 0 elif not resume_item and queue.current_index is None and len(queue_items) > 0: From 316fabfeb10362693ac3d3a794501cef9a568dbc Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Fri, 5 Jul 2024 09:34:45 +0200 Subject: [PATCH 3/5] Fix RadioBrowser browse --- .../server/providers/radiobrowser/__init__.py | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/music_assistant/server/providers/radiobrowser/__init__.py b/music_assistant/server/providers/radiobrowser/__init__.py index 8b349296a..1ee1f461e 100644 --- a/music_assistant/server/providers/radiobrowser/__init__.py +++ b/music_assistant/server/providers/radiobrowser/__init__.py @@ -102,12 +102,15 @@ async def search( return result - @use_cache(86400 * 7) + @use_cache(3600) async def browse(self, path: str, offset: int, limit: int) -> list[MediaItemType]: """Browse this provider's items. :param path: The path to browse, (e.g. provid://artists). """ + if offset != 0: + # paging is broken on RadioBrowser, we just return some big lists + return [] subpath = path.split("://", 1)[1] subsubpath = "" if "/" not in subpath else subpath.split("/")[-1] @@ -138,13 +141,11 @@ async def browse(self, path: str, offset: int, limit: int) -> list[MediaItemType ] if subpath == "popular": - return await self.get_by_popularity(limit=limit, offset=offset) + return await self.get_by_popularity() if subpath == "tag": tags = await self.radios.tags( hide_broken=True, - limit=limit, - offset=offset, order=Order.STATION_COUNT, reverse=True, ) @@ -161,9 +162,7 @@ async def browse(self, path: str, offset: int, limit: int) -> list[MediaItemType if subpath == "country": items: list[BrowseFolder | Radio] = [] - for country in await self.radios.countries( - order=Order.NAME, hide_broken=True, limit=limit, offset=offset - ): + for country in await self.radios.countries(order=Order.NAME, hide_broken=True): folder = BrowseFolder( item_id=country.code.lower(), provider=self.domain, @@ -181,19 +180,18 @@ async def browse(self, path: str, offset: int, limit: int) -> list[MediaItemType items.append(folder) return items - if subsubpath in await self.get_tag_names(limit=limit, offset=offset): + if subsubpath in await self.get_tag_names(): return await self.get_by_tag(subsubpath) - if subsubpath in await self.get_country_codes(limit=limit, offset=offset): + if subsubpath in await self.get_country_codes(): return await self.get_by_country(subsubpath) return [] - async def get_tag_names(self, limit: int, offset: int): + async def get_tag_names(self): """Get a list of tag names.""" tags = await self.radios.tags( hide_broken=True, - limit=limit, - offset=offset, + limit=10000, order=Order.STATION_COUNT, reverse=True, ) @@ -203,22 +201,19 @@ async def get_tag_names(self, limit: int, offset: int): tag_names.append(tag.name.lower()) return tag_names - async def get_country_codes(self, limit: int, offset: int): + async def get_country_codes(self): """Get a list of country names.""" - countries = await self.radios.countries( - order=Order.NAME, hide_broken=True, limit=limit, offset=offset - ) + countries = await self.radios.countries(order=Order.NAME, hide_broken=True) country_codes = [] for country in countries: country_codes.append(country.code.lower()) return country_codes - async def get_by_popularity(self, limit: int, offset: int): + async def get_by_popularity(self): """Get radio stations by popularity.""" stations = await self.radios.stations( hide_broken=True, - limit=limit, - offset=offset, + limit=10000, order=Order.CLICK_COUNT, reverse=True, ) From c68edcdd552abd467b67b3a124578e7547b3b26d Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Fri, 5 Jul 2024 09:43:45 +0200 Subject: [PATCH 4/5] tweak --- music_assistant/server/helpers/process.py | 24 ++++++++--------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/music_assistant/server/helpers/process.py b/music_assistant/server/helpers/process.py index 71c3a61d6..492bca619 100644 --- a/music_assistant/server/helpers/process.py +++ b/music_assistant/server/helpers/process.py @@ -257,7 +257,7 @@ async def wait(self) -> int: return self._returncode -async def check_output(*args: list[str]) -> tuple[int, bytes]: +async def check_output(*args: str) -> tuple[int, bytes]: """Run subprocess and return returncode and output.""" proc = await asyncio.create_subprocess_exec( *args, @@ -269,23 +269,15 @@ async def check_output(*args: list[str]) -> tuple[int, bytes]: async def communicate( - args: str | list[str], + args: list[str], input: bytes | None = None, # noqa: A002 ) -> tuple[int, bytes, bytes]: """Communicate with subprocess and return returncode, stdout and stderr output.""" - if isinstance(args, str): - proc = await asyncio.create_subprocess_shell( - args, - stderr=asyncio.subprocess.PIPE, - stdout=asyncio.subprocess.PIPE, - stdin=asyncio.subprocess.PIPE if input is not None else None, - ) - else: - proc = await asyncio.create_subprocess_exec( - *args, - stderr=asyncio.subprocess.PIPE, - stdout=asyncio.subprocess.PIPE, - stdin=asyncio.subprocess.PIPE if input is not None else None, - ) + proc = await asyncio.create_subprocess_exec( + *args, + stderr=asyncio.subprocess.PIPE, + stdout=asyncio.subprocess.PIPE, + stdin=asyncio.subprocess.PIPE if input is not None else None, + ) stdout, stderr = await proc.communicate(input) return (proc.returncode, stdout, stderr) From bb4920d800e50b216c8700395f70bda02c54a551 Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Fri, 5 Jul 2024 09:51:00 +0200 Subject: [PATCH 5/5] change caching --- music_assistant/server/providers/radiobrowser/__init__.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/music_assistant/server/providers/radiobrowser/__init__.py b/music_assistant/server/providers/radiobrowser/__init__.py index 1ee1f461e..23c550aff 100644 --- a/music_assistant/server/providers/radiobrowser/__init__.py +++ b/music_assistant/server/providers/radiobrowser/__init__.py @@ -102,7 +102,6 @@ async def search( return result - @use_cache(3600) async def browse(self, path: str, offset: int, limit: int) -> list[MediaItemType]: """Browse this provider's items. @@ -187,6 +186,7 @@ async def browse(self, path: str, offset: int, limit: int) -> list[MediaItemType return await self.get_by_country(subsubpath) return [] + @use_cache(3600 * 24) async def get_tag_names(self): """Get a list of tag names.""" tags = await self.radios.tags( @@ -201,6 +201,7 @@ async def get_tag_names(self): tag_names.append(tag.name.lower()) return tag_names + @use_cache(3600 * 24) async def get_country_codes(self): """Get a list of country names.""" countries = await self.radios.countries(order=Order.NAME, hide_broken=True) @@ -209,11 +210,12 @@ async def get_country_codes(self): country_codes.append(country.code.lower()) return country_codes + @use_cache(3600) async def get_by_popularity(self): """Get radio stations by popularity.""" stations = await self.radios.stations( hide_broken=True, - limit=10000, + limit=5000, order=Order.CLICK_COUNT, reverse=True, ) @@ -222,6 +224,7 @@ async def get_by_popularity(self): items.append(await self._parse_radio(station)) return items + @use_cache(3600) async def get_by_tag(self, tag: str): """Get radio stations by tag.""" items = [] @@ -236,6 +239,7 @@ async def get_by_tag(self, tag: str): items.append(await self._parse_radio(station)) return items + @use_cache(3600) async def get_by_country(self, country_code: str): """Get radio stations by country.""" items = []