Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Add multi device DSP support #1839

Merged
merged 10 commits into from
Jan 9, 2025

Conversation

maximmaxim345
Copy link
Contributor

@maximmaxim345 maximmaxim345 commented Jan 8, 2025

Requires music-assistant/models#29 to be merged first.

Overview

This PR primarily adds grouped playback support to the DSP of Music Assistant.
DSP is automatically disabled when grouped playback is enabled on player providers that do not support the new MULTI_DEVICE_DSP PlayerFeature flag.
If a player provider has this flag, it is responsible for retrieving get_player_filter_params for each sub-device separately.
Universal Groups allow all contained devices to apply their own DSP.
So if a user wants to use DSP on two players that do not support MULTI_DEVICE_DSP (like Snapcast), they can still create a Universal Group with them that will apply the DSP separately (at the cost of synchronization).

Universal Group Adjustments

Universal Groups previously were forced to use MP3 for all devices. This PR decouples the audio processing into two stages:

  • The previous speed limiter
  • An optional per-client stream with applied DSP and correct output format

This has two main advantages:

  • A filter can be applied when necessary, allowing us to use individual DSP on every device in the group
  • We are no longer forced to use MP3 for every device, since a device not supporting lossless playback can request its own MP3 stream

Currently, all players except Slimproto, Snapcast, and AirPlay still use MP3, but this can be changed in the future.

Supported Providers

With this PR, DSP is supported on:

  • All player providers with single device playback active
  • Grouped AirPlay players
  • Grouped Slimproto players
  • All players inside a Universal Group

With automatic disablement in unsupported cases.

Testing

I have verified this PR as follows:

  • Snapcast with 2 Snapweb browsers: Since Snapcast does not support multi-device DSP, this PR properly disables DSP when grouped
  • Permanent Snapcast group: Works as above, disables the DSP
  • Edge cases on startup: Works since reload detection is handled in update()
  • Snapcast with manually added MULTI_DEVICE_DSP FeatureFlag: Keeps DSP enabled
  • AirPlay: Each device gets its own DSP during synced playback
  • Universal group with 2 Snapcast players: Individual players' DSPs are applied
  • Universal group with 2 DLNA devices: Individual players' DSPs are applied
  • Universal group with 2 DLNA devices, 1 Snapcast player, and 1 Chromecast: All 4 players receive their own DSP

Future Work

  • Add UI for multi-device DSP (including a warning that it's disabled!)
  • Fix links to DSP Settings in grouped playback UIs
  • Disable DSP config UI for Universal Group players

Similar to slimprotos MultiClientStream, we can now generate a stream on
a per device basis.

This has two main advantages:
- a filter can be applied when necessary, allowing us to use an
individual DSP on every device in the group
- we are no longer forced to use MP3 for every device
This is the final piece that allows us to apply a DSP on a per player
basis when using universal groups.
@maximmaxim345 maximmaxim345 marked this pull request as ready for review January 9, 2025 19:18
Copy link
Member

@marcelveldt marcelveldt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice work!

@marcelveldt
Copy link
Member

models have been bumped btw

@marcelveldt marcelveldt merged commit 2a5f200 into music-assistant:dev Jan 9, 2025
4 checks passed
@maximmaxim345 maximmaxim345 deleted the feat/dsp-multiroom branch January 17, 2025 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants