Added AriaCast Receiver plugin for Music Assistant#3061
Added AriaCast Receiver plugin for Music Assistant#3061AirPlr wants to merge 54 commits intomusic-assistant:devfrom
Conversation
- Implemented AriaCast Receiver plugin to stream audio from Android devices to Music Assistant players. - Added README.md with features, installation, configuration, and usage instructions. - Created configuration classes for audio and server settings. - Developed metadata handling for AriaCast streams. - Implemented UDP discovery and WebSocket server for audio and metadata streaming. - Added helper functions for local IP retrieval and artwork downloading. - Included SVG icon for the plugin. - Updated manifest.json with documentation link and requirements.
|
You need to run pre-commit before you push the commit. There are a ton of mypy errors….. I have marked this as draft. Please mark as ready for review when these are fixed |
Removed installation instructions and updated configuration step for clarity.
|
I am not sure how you are creating this provider. You can’t be running pre-commit as there are 107 errors! |
🐶 Ruff Linter...........................................................Passed
All checks passed! 🐶 Ruff Formatter........................................................Passed
399 files left unchanged 🐍 Check Python AST......................................................Passed
Success: no issues found in 414 source files |
There was a problem hiding this comment.
Pull request overview
This PR adds a new AriaCast Receiver plugin that enables Music Assistant to receive PCM audio streams from Android devices over WebSocket. The implementation provides UDP-based device discovery, WebSocket endpoints for audio/control/metadata/stats, and integration with Music Assistant's player system.
Changes:
- Implemented WebSocket server with endpoints for audio streaming, metadata updates, playback control, and statistics
- Added UDP discovery protocol for device announcement on the local network
- Created configuration and metadata handling classes with support for both camelCase and snake_case field naming
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 36 comments.
Show a summary per file
| File | Description |
|---|---|
__init__.py |
Main plugin provider with WebSocket server, UDP discovery, audio buffering, and player management |
metadata.py |
Metadata handler supporting track info, artwork, duration, and playback position |
config.py |
Configuration dataclasses for audio parameters and server settings |
helpers.py |
Helper function for local IP detection (currently unused) |
manifest.json |
Plugin manifest with metadata and requirements |
icon.svg |
Plugin icon graphic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.max_frames = 50 # 1 second buffer | ||
| self.frame_queue: deque[bytes] = deque(maxlen=self.max_frames) |
There was a problem hiding this comment.
The frame queue has a maximum size of 50 frames (1 second buffer), but there's no backpressure mechanism or notification to the audio client when the queue is full. The deque will silently drop the oldest frames when full. This could cause audio discontinuities if the consumer (get_audio_stream) is slower than the producer. Consider either implementing backpressure (e.g., slowing down the WebSocket receive rate) or logging warnings when frames are being dropped due to buffer overflow.
| yield frame | ||
| else: | ||
| # No data available, wait a bit to avoid busy loop | ||
| await asyncio.sleep(0.005) |
There was a problem hiding this comment.
The get_audio_stream method yields frames from the queue with a 5ms sleep when the queue is empty. This busy-waiting pattern with a short sleep could lead to high CPU usage. Consider using an asyncio.Event or asyncio.Queue with proper async waiting instead of polling with sleep. This would be more efficient and responsive.
| await asyncio.sleep(0.005) | |
| await asyncio.sleep(0.05) |
| SERVER_NAME: str = "AriaCast Speaker" | ||
| VERSION: str = "1.0" | ||
| PLATFORM: str = "MusicAssistant" | ||
| CODECS: list[str] | None = None | ||
| DISCOVERY_PORT: int = 12888 | ||
| STREAMING_PORT: int = 12889 | ||
| HOST: str = "0.0.0.0" | ||
| AUDIO: AudioConfig | None = None | ||
|
|
||
| def __post_init__(self) -> None: | ||
| """Initialize default values after instantiation.""" | ||
| if self.CODECS is None: | ||
| self.CODECS = ["PCM"] | ||
| if self.AUDIO is None: | ||
| self.AUDIO = AudioConfig() |
There was a problem hiding this comment.
Similarly, the ServerConfig dataclass uses uppercase field names (e.g., SERVER_NAME, VERSION, PLATFORM, etc.) which deviates from Python naming conventions. These appear to be configuration values rather than constants, and should use lowercase or snake_case naming (e.g., server_name, version, platform).
| SERVER_NAME: str = "AriaCast Speaker" | |
| VERSION: str = "1.0" | |
| PLATFORM: str = "MusicAssistant" | |
| CODECS: list[str] | None = None | |
| DISCOVERY_PORT: int = 12888 | |
| STREAMING_PORT: int = 12889 | |
| HOST: str = "0.0.0.0" | |
| AUDIO: AudioConfig | None = None | |
| def __post_init__(self) -> None: | |
| """Initialize default values after instantiation.""" | |
| if self.CODECS is None: | |
| self.CODECS = ["PCM"] | |
| if self.AUDIO is None: | |
| self.AUDIO = AudioConfig() | |
| server_name: str = "AriaCast Speaker" | |
| version: str = "1.0" | |
| platform: str = "MusicAssistant" | |
| codecs: list[str] | None = None | |
| discovery_port: int = 12888 | |
| streaming_port: int = 12889 | |
| host: str = "0.0.0.0" | |
| audio: AudioConfig | None = None | |
| def __post_init__(self) -> None: | |
| """Initialize default values after instantiation.""" | |
| if self.codecs is None: | |
| self.codecs = ["PCM"] | |
| if self.audio is None: | |
| self.audio = AudioConfig() | |
| @property | |
| def SERVER_NAME(self) -> str: | |
| """Backward-compatible alias for server_name.""" | |
| return self.server_name | |
| @SERVER_NAME.setter | |
| def SERVER_NAME(self, value: str) -> None: | |
| self.server_name = value | |
| @property | |
| def VERSION(self) -> str: | |
| """Backward-compatible alias for version.""" | |
| return self.version | |
| @VERSION.setter | |
| def VERSION(self, value: str) -> None: | |
| self.version = value | |
| @property | |
| def PLATFORM(self) -> str: | |
| """Backward-compatible alias for platform.""" | |
| return self.platform | |
| @PLATFORM.setter | |
| def PLATFORM(self, value: str) -> None: | |
| self.platform = value | |
| @property | |
| def CODECS(self) -> list[str] | None: | |
| """Backward-compatible alias for codecs.""" | |
| return self.codecs | |
| @CODECS.setter | |
| def CODECS(self, value: list[str] | None) -> None: | |
| self.codecs = value | |
| @property | |
| def DISCOVERY_PORT(self) -> int: | |
| """Backward-compatible alias for discovery_port.""" | |
| return self.discovery_port | |
| @DISCOVERY_PORT.setter | |
| def DISCOVERY_PORT(self, value: int) -> None: | |
| self.discovery_port = value | |
| @property | |
| def STREAMING_PORT(self) -> int: | |
| """Backward-compatible alias for streaming_port.""" | |
| return self.streaming_port | |
| @STREAMING_PORT.setter | |
| def STREAMING_PORT(self, value: int) -> None: | |
| self.streaming_port = value | |
| @property | |
| def HOST(self) -> str: | |
| """Backward-compatible alias for host.""" | |
| return self.host | |
| @HOST.setter | |
| def HOST(self, value: str) -> None: | |
| self.host = value | |
| @property | |
| def AUDIO(self) -> AudioConfig | None: | |
| """Backward-compatible alias for audio.""" | |
| return self.audio | |
| @AUDIO.setter | |
| def AUDIO(self, value: AudioConfig | None) -> None: | |
| self.audio = value |
| for key in [ | ||
| "title", | ||
| "artist", | ||
| "album", | ||
| "artwork_url", | ||
| "duration_ms", | ||
| "position_ms", | ||
| "is_playing", | ||
| ]: |
There was a problem hiding this comment.
The metadata update logic checks each field individually (lines 42-52) but could be simplified. The explicit list of keys is duplicated from the initial metadata structure. Consider iterating over self.current_metadata.keys() to avoid maintaining two separate lists of field names, reducing the chance of them getting out of sync.
| for key in [ | |
| "title", | |
| "artist", | |
| "album", | |
| "artwork_url", | |
| "duration_ms", | |
| "position_ms", | |
| "is_playing", | |
| ]: | |
| # Update only known metadata fields, using current_metadata as the source of truth | |
| for key in self.current_metadata.keys(): |
| async def handle_control_ws(self, request: web.Request) -> web.WebSocketResponse: | ||
| """WebSocket handler for /control endpoint.""" | ||
| ws = web.WebSocketResponse() | ||
| await ws.prepare(request) | ||
|
|
||
| self._control_client = ws |
There was a problem hiding this comment.
The control client is not tracked in a list like metadata clients are. If multiple control clients attempt to connect, the second client will overwrite self._control_client, causing commands to only be sent to the most recent client and potentially orphaning previous connections. Consider either preventing multiple control client connections (like with audio clients) or tracking all control clients in a list and broadcasting to all of them.
| if meta.duration != new_duration: | ||
| meta.duration = new_duration | ||
| has_changes = True | ||
| except (ValueError, TypeError): |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| meta.elapsed_time = new_position | ||
| meta.elapsed_time_last_updated = int(time.time()) | ||
| has_changes = True | ||
| except (ValueError, TypeError): |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
|
If you can resolve the copilot comments by either actioning or explaining why it is incorrect that would be great. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 20 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Merge mapped keys into metadata (preferring existing snake_case if present) | ||
| for camel, snake in key_mapping.items(): | ||
| if camel in metadata and snake not in metadata: | ||
| metadata[snake] = metadata[camel] |
There was a problem hiding this comment.
The metadata update logic modifies the input dictionary by adding snake_case keys when camelCase keys are present (lines 40-42). This mutates the caller's dictionary, which is unexpected behavior and could cause issues if the caller reuses the dictionary. Consider creating a new dictionary or explicitly documenting this side effect in the docstring.
| if not self._playback_started and len(self.frame_queue) >= prebuffer: | ||
| # Note: check for target player first | ||
| target_player_id = self._get_target_player_id() | ||
| if target_player_id: | ||
| self._playback_started = True # Prevent multiple calls | ||
| self._active_player_id = target_player_id | ||
| # Use a task to not block the receiver loop | ||
| self.mass.create_task(self._start_playback(target_player_id)) | ||
| else: | ||
| self.logger.warning("No player available for AriaCast playback") | ||
| self._playback_started = False | ||
|
|
There was a problem hiding this comment.
There's a potential race condition in playback startup. _playback_started is set to True before _start_playback is called (line 543), but if _start_playback fails, the flag is cleared inside that function (line 570). However, if the audio WebSocket handler receives more frames before the failure is handled, they will be queued but playback won't actually start. Consider setting _playback_started only after successful playback initialization, or adding better error recovery.
| def datagram_received(self, data: bytes, addr: tuple[str, int]) -> None: | ||
| """Handle discovery request.""" | ||
| try: | ||
| # Note: Basic discovery without authentication or rate limiting. | ||
| # Hostile network environments might require adding rate limiting per source IP. | ||
| message = data.decode("utf-8").strip() | ||
| if message == "DISCOVER_AUDIOCAST": | ||
| local_ip = self._get_local_ip() | ||
| config = self.provider.server_config | ||
| audio = cast("AudioConfig", config.audio) | ||
| response = { | ||
| "server_name": config.server_name, | ||
| "ip": local_ip, | ||
| "port": config.streaming_port, | ||
| "samplerate": audio.sample_rate, | ||
| "channels": audio.channels, | ||
| } | ||
| if self.transport: | ||
| self.transport.sendto(json.dumps(response).encode(), addr) | ||
| self.logger.debug("Sent discovery response to %s", addr) | ||
| except Exception as e: | ||
| self.logger.debug("Error handling discovery request: %s", e) |
There was a problem hiding this comment.
The UDP discovery protocol has no rate limiting, making it vulnerable to denial-of-service attacks. A malicious client could flood the server with DISCOVER_AUDIOCAST requests, overwhelming the system. Consider implementing per-IP rate limiting or connection throttling to protect against abuse. The comment on line 1020-1021 acknowledges this issue but doesn't address it.
| async def handle_metadata_ws(self, request: web.Request) -> web.WebSocketResponse: | ||
| """WebSocket handler for /metadata endpoint.""" | ||
| ws = web.WebSocketResponse() | ||
| await ws.prepare(request) | ||
|
|
||
| peer = request.remote | ||
| self.logger.debug("Metadata client connected: %s", peer) | ||
|
|
||
| # Add to metadata clients list | ||
| self.metadata_clients.append(ws) | ||
|
|
||
| # Send current metadata | ||
| try: | ||
| current_metadata = self.metadata_handler.get() | ||
| await ws.send_json( | ||
| { | ||
| "type": "metadata", | ||
| "data": current_metadata, | ||
| } | ||
| ) | ||
| except Exception as e: | ||
| self.logger.debug("Failed to send current metadata: %s", e) | ||
|
|
||
| try: | ||
| async for msg in ws: | ||
| if msg.type == web.WSMsgType.TEXT: | ||
| try: | ||
| data = json.loads(msg.data) | ||
| # Accept both legacy "update" and "metadata" types from clients | ||
| msg_type = data.get("type") if isinstance(data, dict) else None | ||
| if msg_type in ("update", "metadata"): | ||
| metadata = data.get("data", {}) | ||
| else: | ||
| # Not wrapped: accept dict with direct metadata fields | ||
| metadata = data if isinstance(data, dict) else {} | ||
|
|
||
| if metadata: | ||
| self.logger.debug("Received metadata from %s: %s", peer, metadata) | ||
| self.metadata_handler.update(metadata) | ||
| self._update_source_metadata(metadata) | ||
| # Broadcast to all other metadata clients | ||
| await self._broadcast_metadata(metadata, exclude_ws=ws) | ||
| except json.JSONDecodeError: | ||
| self.logger.debug( | ||
| "Failed to decode JSON metadata from %s: %r", peer, msg.data | ||
| ) | ||
| elif msg.type == web.WSMsgType.ERROR: | ||
| break | ||
| except Exception as e: | ||
| self.logger.debug("Error in metadata handler: %s", e) | ||
| finally: | ||
| # Remove from clients list | ||
| if ws in self.metadata_clients: | ||
| self.metadata_clients.remove(ws) | ||
| self.logger.debug("Metadata client disconnected: %s", peer) | ||
|
|
||
| return ws |
There was a problem hiding this comment.
The metadata WebSocket handler accepts metadata updates from any connected client without authentication or validation. This could allow malicious clients on the same network to inject arbitrary metadata into the playback session. Consider adding authentication or at least validating that metadata updates come from the same client that's streaming audio.
| img_data = await response.read() | ||
| if not img_data: | ||
| self.logger.debug( | ||
| "Skipping artwork download from %s due to empty response body", | ||
| artwork_url, | ||
| ) | ||
| return |
There was a problem hiding this comment.
The artwork download function downloads images from arbitrary URLs without validating the content size before reading the entire response. While there's a check for Content-Length header (5MB limit), this header is optional and can be omitted by malicious servers. A malicious server could send an unlimited stream of data, causing memory exhaustion. Consider setting a hard limit on response.read() or reading in chunks with a total size limit.
| @staticmethod | ||
| def _get_local_ip() -> str: | ||
| """Get local IP address.""" | ||
| s: socket.socket | None = None | ||
| try: | ||
| s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) | ||
| s.connect(("8.8.8.8", 80)) | ||
| return str(s.getsockname()[0]) | ||
| except Exception: | ||
| return "127.0.0.1" |
There was a problem hiding this comment.
The _get_local_ip method connects to Google's DNS server (8.8.8.8) to determine the local IP address. This will fail in environments without internet access or where outbound UDP is blocked. This is a common pattern but consider adding a fallback that enumerates network interfaces when the connection fails, or using the socket from the UDP server itself to get the local address.
| # Drop silent/muted frames to avoid buffer buildup during silence | ||
| if data == bytes(len(data)): | ||
| continue |
There was a problem hiding this comment.
The silence detection check using data == bytes(len(data)) is incorrect. This creates a bytes object filled with zeros and checks for exact equality, but PCM audio data is structured with samples spread across channels. A proper silence check should analyze the audio samples considering the sample width and channels. Consider checking if all samples are below a small threshold instead of checking for all zeros.
| # Clear active player | ||
| current_player_id = self._source_details.in_use_by | ||
| self._clear_active_player() | ||
|
|
||
| # Deselect source from player | ||
| if current_player_id: | ||
| try: | ||
| await self.mass.players.select_source(current_player_id, None) | ||
| except Exception as e: | ||
| self.logger.error( | ||
| "Failed to deselect AriaCast source from player %s: %s", | ||
| current_player_id, | ||
| e, | ||
| ) |
There was a problem hiding this comment.
In the audio WebSocket handler cleanup, there's a potential issue where the active player is cleared and the source is deselected, but if the select_source call raises an exception, the player may be left in an inconsistent state. While the exception is caught, the player might still have the source selected internally but the plugin's state shows it as cleared. Consider adding more robust cleanup or state reconciliation.
| def __post_init__(self) -> None: | ||
| """Provide uppercase aliases for backwards compatibility.""" | ||
| self.SAMPLE_RATE = self.sample_rate | ||
| self.CHANNELS = self.channels | ||
| self.SAMPLE_WIDTH = self.sample_width | ||
| self.FRAME_DURATION_MS = self.frame_duration_ms | ||
| self.FRAME_SIZE = self.frame_size |
There was a problem hiding this comment.
The backwards compatibility aliases (SAMPLE_RATE, CHANNELS, etc.) created in __post_init__ are not documented. If these are for compatibility with existing code, they should be documented. If they're not needed, they should be removed to avoid confusion and reduce maintenance burden. Consider adding a comment explaining why these uppercase aliases exist.
| """Provide uppercase aliases for backwards compatibility.""" | ||
| self.SAMPLE_RATE = self.sample_rate | ||
| self.CHANNELS = self.channels | ||
| self.SAMPLE_WIDTH = self.sample_width | ||
| self.FRAME_DURATION_MS = self.frame_duration_ms | ||
| self.FRAME_SIZE = self.frame_size |
There was a problem hiding this comment.
The comment in the dataclass describes "Provide uppercase aliases for backwards compatibility" but this is a new plugin being added. There is no prior version to be backwards compatible with. Either this comment is misleading or there's unnecessary code. If this is copying a pattern from elsewhere, consider whether it's actually needed for a new implementation.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 8 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Switched from pipe to stdout - Dropped linux arm - Moved _get_binary_path() to helpers.py
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Addressed last review from marcelveldt
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 8 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
music_assistant/providers/ariacast_receiver/init.py:506
_on_source_selectedcurrently just storesin_use_byas_active_player_id, but it doesn’t enforce theCONF_ALLOW_PLAYER_SWITCHsetting. Sincepassiveonly affects resume behavior (it doesn’t prevent users from selecting this plugin source on a different player), manual switching will still be possible even when the config disables it. Consider mirroring the approach used inspotify_connect’s_on_source_selectedto reject/rollback disallowed selections and trigger a player update.
async def _on_source_selected(self) -> None:
"""Handle manual selection in UI."""
new_player_id = self._source_details.in_use_by
if new_player_id:
self._active_player_id = new_player_id
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not self._stop_called: | ||
| self.logger.debug( | ||
| "WebSocket connection to AriaCast metadata stream failed: %s", exc | ||
| ) |
There was a problem hiding this comment.
We should introduce a backoff time here to avoid retrying in quick succession.
| await self._binary_process.start() | ||
|
|
||
| # Start Metadata Monitor | ||
| await asyncio.sleep(1) |
There was a problem hiding this comment.
Do we really need this sleep? There's retry logic in monitor_metadata, so I am not sure why we are sleeping for 1s here
| args = [binary_path, "--stdout"] | ||
|
|
||
| self.logger.info("Starting AriaCast binary: %s", binary_path) | ||
| self._binary_process = AsyncProcess(args, name="ariacast", stdout=True, stderr=False) |
There was a problem hiding this comment.
Wouldn't it make sense to also set stderr to True here? Then you can use iter_stderr to listen for any errors that might be thrown by the binary?
| "name": "AriaCast Receiver", | ||
| "stage": "alpha", | ||
| "description": "Receive AriaCast audio streams over WebSocket and use them as a source in Music Assistant.", | ||
| "codeowners": ["@music-assistant"], |
There was a problem hiding this comment.
This should be @AirPlr I think ;-)
| "stage": "alpha", | ||
| "description": "Receive AriaCast audio streams over WebSocket and use them as a source in Music Assistant.", | ||
| "codeowners": ["@music-assistant"], | ||
| "documentation": "https://github.com/music-assistant/server/tree/main/music_assistant/providers/ariacast_receiver", |
There was a problem hiding this comment.
We have a separate repo for our documentation website here, When this PR is ready to be merged, we can update the url with one that links to musicassistant.io
| from pathlib import Path | ||
|
|
||
|
|
||
| async def _get_binary_path() -> str: |
There was a problem hiding this comment.
I don't think this is async?
| binary_name = f"ariacast_{system}_{arch}" | ||
| binary_path = os.path.join(base_dir, binary_name) | ||
|
|
||
| if not os.path.exists(binary_path): |
There was a problem hiding this comment.
This is blocking IO and should be wrapped in asyncio.to_thread()
| self._current_track_title: str | None = None # Track song changes | ||
|
|
||
| # Audio buffer - larger for high-latency players like Sendspin | ||
| self.max_frames = 75 # 1.5 second buffer (75 frames * 20ms each) |
There was a problem hiding this comment.
We actually have a PR with some huge Sendspin improvements including lower latency for plugins like this one. We might be able to reduce this after that has been merged
|
|
||
| return str(self._default_player_id) | ||
|
|
||
| async def _on_source_selected(self) -> None: |
There was a problem hiding this comment.
Have a look at Spotify connect. This could be extended with rejecting source select requests for instances where 'allow player switch' is not allowed.
MarvinSchenkel
left a comment
MarvinSchenkel
left a comment
There was a problem hiding this comment.
We are getting close 👏 . Please have a look at my comments and let me know when you have looked at them by marking the PR as 'Ready for review' again.
4 participants
Uh oh!
There was an error while loading. Please reload this page.