From: Marcel van der Veldt Date: Tue, 14 Mar 2023 17:19:51 +0000 (+0100) Subject: Fix playback issues of http(s) based streams (#529) X-Git-Url: https://git.kitaultman.com/?a=commitdiff_plain;h=456c74cbe531750b11278338602ebeca8ba49f03;p=music-assistant-server.git Fix playback issues of http(s) based streams (#529) * change regex for latest tag * fix test workflow * allow config values to be none/null * add build to pip packages * add setuptools * Update test.yml * lint * lint * Update dependabot.yml * Update test.yml * change create provider config logic a bit --- diff --git a/.github/dependabot.yml b/.github/dependabot.yml index ec8bf989..f14ba452 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -3,7 +3,7 @@ updates: - package-ecosystem: "github-actions" directory: "/" schedule: - interval: weekly + interval: daily - package-ecosystem: "pip" directory: "/" schedule: diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml index 09d931ad..63663023 100644 --- a/.github/workflows/docker-build.yml +++ b/.github/workflows/docker-build.yml @@ -41,7 +41,7 @@ jobs: # If the VERSION looks like a version number, assume that # this is the most recent version of the image and also # tag it 'latest'. - if [[ $VERSION =~ ^\d+\.\d+\.\d+[[:space:]]?(b[[:space:]]?\d+)? ]]; then + if [[ $VERSION =~ [0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3} ]]; then TAGS="$TAGS,${DOCKER_IMAGE}:latest" fi diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 09bde45a..97f5364c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -23,22 +23,10 @@ jobs: python-version: "3.11" - name: Install dependencies run: | - sudo apt-get update - sudo apt-get install -y ffmpeg - python -m pip install --upgrade pip - pip install -e .[server] -r requirements-test.txt + python -m pip install --upgrade pip build setuptools + pip install .[test] - name: Lint/test with pre-commit run: pre-commit run --all-files - - name: Flake8 - run: flake8 scripts/ music_assistant/ - - name: Black - run: black --check scripts/ music_assistant/ - - name: isort - run: isort --check scripts/ music_assistant/ - - name: pylint - run: pylint music_assistant/ - # - name: mypy - # run: mypy music_assistant/ test: runs-on: ubuntu-latest @@ -58,9 +46,7 @@ jobs: python-version: ${{ matrix.python-version }} - name: Install dependencies run: | - sudo apt-get update - sudo apt-get install -y libgirepository1.0-dev - python -m pip install --upgrade pip - pip install -e .[server] -r requirements-test.txt + python -m pip install --upgrade pip build setuptools + pip install .[test] - name: Pytest - run: pytest --durations 10 --cov-report term-missing --cov=music_assistant --cov-report=xml tests/server/ + run: pytest --durations 10 --cov-report term-missing --cov=music_assistant --cov-report=xml tests/ diff --git a/.vscode/.ropeproject/config.py b/.vscode/.ropeproject/config.py index 0b2a3169..af5a0446 100644 --- a/.vscode/.ropeproject/config.py +++ b/.vscode/.ropeproject/config.py @@ -113,9 +113,7 @@ def set_prefs(prefs): # listed in module rope.base.oi.type_hinting.providers.interfaces # For example, you can add you own providers for Django Models, or disable # the search type-hinting in a class hierarchy, etc. - prefs[ - "type_hinting_factory" - ] = "rope.base.oi.type_hinting.factory.default_type_hinting_factory" + prefs["type_hinting_factory"] = "rope.base.oi.type_hinting.factory.default_type_hinting_factory" def project_opened(project): diff --git a/.vscode/launch.json b/.vscode/launch.json index 198035cf..30f98be9 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -15,4 +15,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/music_assistant/common/models/config_entries.py b/music_assistant/common/models/config_entries.py index 23afcd95..694de33c 100644 --- a/music_assistant/common/models/config_entries.py +++ b/music_assistant/common/models/config_entries.py @@ -94,7 +94,7 @@ class ConfigEntryValue(ConfigEntry): cls, entry: ConfigEntry, value: ConfigValueType, - allow_none: bool = False, + allow_none: bool = True, ) -> ConfigEntryValue: """Parse ConfigEntryValue from the config entry and plain value.""" result = ConfigEntryValue.from_dict(entry.to_dict()) @@ -108,7 +108,7 @@ class ConfigEntryValue(ConfigEntry): result.value = result.label if not isinstance(result.value, expected_type): if result.value is None and allow_none: - # In some cases we allow this (e.g. create default config), hence the allow_none + # In some cases we allow this (e.g. create default config) return result # handle common conversions/mistakes if expected_type == float and isinstance(result.value, int): @@ -147,11 +147,10 @@ class Config(DataClassDictMixin): cls, config_entries: Iterable[ConfigEntry], raw: dict[str, Any], - allow_none: bool = False, ) -> Config: """Parse Config from the raw values (as stored in persistent storage).""" values = { - x.key: ConfigEntryValue.parse(x, raw.get("values", {}).get(x.key), allow_none).to_dict() + x.key: ConfigEntryValue.parse(x, raw.get("values", {}).get(x.key)).to_dict() for x in config_entries } conf = cls.from_dict({**raw, "values": values}) diff --git a/music_assistant/server/controllers/config.py b/music_assistant/server/controllers/config.py index bb83a043..f74b6674 100644 --- a/music_assistant/server/controllers/config.py +++ b/music_assistant/server/controllers/config.py @@ -173,7 +173,7 @@ class ConfigController: for prov in self.mass.get_available_providers(): if prov.domain != raw_conf["domain"]: continue - return ProviderConfig.parse(prov.config_entries, raw_conf, allow_none=True) + return ProviderConfig.parse(prov.config_entries, raw_conf) raise KeyError(f"No config found for provider id {instance_id}") @api_command("config/providers/update") @@ -192,7 +192,9 @@ class ConfigController: self.mass.create_task(self.mass.load_provider(updated_config)) @api_command("config/providers/create") - def create_provider_config(self, provider_domain: str) -> ProviderConfig: + def create_provider_config( + self, provider_domain: str, default_enabled: bool = False + ) -> ProviderConfig: """Create default/empty ProviderConfig. This is intended to be used as helper method to add a new provider, @@ -230,9 +232,9 @@ class ConfigController: "domain": manifest.domain, "instance_id": instance_id, "name": name, + "enabled": default_enabled, "values": {}, }, - allow_none=True, ) # config provided and checks passed, storeconfig diff --git a/music_assistant/server/controllers/streams.py b/music_assistant/server/controllers/streams.py index e0980071..8f0c559f 100644 --- a/music_assistant/server/controllers/streams.py +++ b/music_assistant/server/controllers/streams.py @@ -191,7 +191,7 @@ class StreamsController: self._serve_queue_stream, ) - ffmpeg_present, libsoxr_support = await check_audio_support(True) + ffmpeg_present, libsoxr_support, version = await check_audio_support() if not ffmpeg_present: LOGGER.error("FFmpeg binary not found on your system, playback will NOT work!.") elif not libsoxr_support: @@ -200,7 +200,11 @@ class StreamsController: "highest quality audio not available. " ) await self._cleanup_stale() - LOGGER.info("Started stream controller") + LOGGER.info( + "Started stream controller (using ffmpeg version %s %s)", + version, + "with libsoxr support" if libsoxr_support else "", + ) async def close(self) -> None: """Cleanup on exit.""" diff --git a/music_assistant/server/helpers/audio.py b/music_assistant/server/helpers/audio.py index e15b83ac..814ec598 100644 --- a/music_assistant/server/helpers/audio.py +++ b/music_assistant/server/helpers/audio.py @@ -593,7 +593,7 @@ async def get_file_stream( yield data -async def check_audio_support(try_install: bool = False) -> tuple[bool, bool]: +async def check_audio_support() -> tuple[bool, bool, str]: """Check if ffmpeg is present (with/without libsoxr support).""" cache_key = "audio_support_cache" if cache := globals().get(cache_key): @@ -602,18 +602,11 @@ async def check_audio_support(try_install: bool = False) -> tuple[bool, bool]: # check for FFmpeg presence returncode, output = await check_output("ffmpeg -version") ffmpeg_present = returncode == 0 and "FFmpeg" in output.decode() - if not ffmpeg_present and try_install: - # try a few common ways to install ffmpeg - # this all assumes we have enough rights and running on a linux based platform (or docker) - await check_output("apt-get update && apt-get install ffmpeg") - await check_output("apk add ffmpeg") - # test again - returncode, output = await check_output("ffmpeg -version") - ffmpeg_present = returncode == 0 and "FFmpeg" in output.decode() # use globals as in-memory cache + version = output.decode().split("ffmpeg version ")[1].split(" ")[0].split("-")[0] libsoxr_support = "enable-libsoxr" in output.decode() - result = (ffmpeg_present, libsoxr_support) + result = (ffmpeg_present, libsoxr_support, version) globals()[cache_key] = result return result @@ -732,13 +725,16 @@ async def _get_ffmpeg_args( """Collect all args to send to the ffmpeg process.""" input_format = streamdetails.content_type - ffmpeg_present, libsoxr_support = await check_audio_support() + ffmpeg_present, libsoxr_support, version = await check_audio_support() if not ffmpeg_present: raise AudioError( "FFmpeg binary is missing from system." "Please install ffmpeg on your OS to enable playback.", ) + + major_version = int(version.split(".")[0]) + # generic args generic_args = [ "ffmpeg", @@ -758,13 +754,18 @@ async def _get_ffmpeg_args( "1", "-reconnect_streamed", "1", - "-reconnect_on_network_error", - "1", - "-reconnect_on_http_error", - "5xx", "-reconnect_delay_max", "10", ] + if major_version > 4: + # these options are only supported in ffmpeg > 5 + input_args += [ + "-reconnect_on_network_error", + "1", + "-reconnect_on_http_error", + "5xx", + ] + if seek_position: input_args += ["-ss", str(seek_position)] input_args += ["-i", streamdetails.direct] diff --git a/music_assistant/server/helpers/database.py b/music_assistant/server/helpers/database.py index ba8616e4..12a11ca2 100755 --- a/music_assistant/server/helpers/database.py +++ b/music_assistant/server/helpers/database.py @@ -103,9 +103,7 @@ class DatabaseConnection: sql_query += f' VALUES ({",".join((f":{x}" for x in keys))})' await self.execute(sql_query, values) # return inserted/replaced item - lookup_vals = { - key: value for key, value in values.items() if value is not None and value != "" - } + lookup_vals = {key: value for key, value in values.items() if value not in (None, "")} return await self.get_row(table, lookup_vals) async def insert_or_replace(self, table: str, values: dict[str, Any]) -> Mapping: diff --git a/music_assistant/server/providers/slimproto/__init__.py b/music_assistant/server/providers/slimproto/__init__.py index 9e9e8a44..67585d3a 100644 --- a/music_assistant/server/providers/slimproto/__init__.py +++ b/music_assistant/server/providers/slimproto/__init__.py @@ -609,7 +609,7 @@ def dict_to_strings(source: dict) -> list[str]: result: list[str] = [] for key, value in source.items(): - if value is None or value == "": + if value in (None, ""): continue if isinstance(value, list): for subval in value: diff --git a/music_assistant/server/server.py b/music_assistant/server/server.py index 1bb083a2..dad85ed9 100644 --- a/music_assistant/server/server.py +++ b/music_assistant/server/server.py @@ -417,7 +417,7 @@ class MusicAssistant: existing = any(x for x in provider_configs if x.domain == prov_manifest.domain) if existing: continue - self.config.create_provider_config(prov_manifest.domain) + self.config.create_provider_config(prov_manifest.domain, True) # load all configured (and enabled) providers for allow_depends_on in (False, True): diff --git a/requirements_all.txt b/requirements_all.txt index 0c3690ac..2ab1e2f9 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -10,7 +10,7 @@ asyncio-throttle==1.0.2 coloredlogs==15.0.1 cryptography==39.0.2 databases==0.7.0 -getmac==0.9.2 +getmac==0.8.2 mashumaro==3.5.0 memory-tempfile==2.2.3 music-assistant-frontend==20230313.0