Add Configurable DSP with Parametric Equalizer

[maximmaxim345]

Overview

This Pull Request introduces a sophisticated multi-stage Digital Signal Processing (DSP) system to Music Assistant, featuring a highly-flexible Parametric Equalizer as its core component.

To replace the old existing EQ, this PR also adds a Filter "Tone Controls" with automatic migration.

Screenshot

For me, the main feature missing from Music Assistant was the lack of a Parametric Equalizer, therefore I added one.

Related Changes

This PR is to be viewed in conjunction with music-assistant/frontend#756 and music-assistant/models#18.

This PR depends on music-assistant/models#18.

Future Filters

This modular system allows the addition of other filters in the future, that I will add in future PRs including:

• Crossfeed: Creates a more natural stereo image by mixing left and right channels
• Compressor: Useful for listening in loud environments
• Limiter: Prevents audio clipping by limiting peak levels
• Convolution: Applies Room Correction filters using impulse responses
• AutoEq: Predefined Equalizers for a large number of Headphones

Implementation Details

DSP Configuration is saved in a separate key compared to all other player configuration.
This has the following advantages:

• Applying changes can be sped up, since main player configuration
  stays the same
• Changing DSP does not require resending Player options as dictated by
  the provider
• Later filters like convolution require special handling for the IR WAV
  file
• This allows saving/loading presets without changing the player's
  configuration

DSP should function with all player providers as I understand, since get_raw_player_config_value is used regardless of the underlying provider.

Tested with DLNA, Chromecast and Sonos.

[marcelveldt]

Wow, just wow. I was kind of hoping someone would pick up the glove for this (as I did prepare a few hooks for this in the code). You just did. Amazing!

I will give this a proper review tomorrow. First impression: Super nice work!

[maximmaxim345]

I'm thinking, maybe this PR (or another PR in the same release) should also add a Tone Controls Filter?

Then we can easily migrate the CONF_EQ_BASS, CONF_EQ_MID and CONF_EQ_TREBLE settings by:

• Automatically adding a "Tone Controls" filter to the default DSP config in case the user previously set CONF_EQ_BASS, CONF_EQ_MID or CONF_EQ_TREBLE
• hiding CONF_EQ_BASS, CONF_EQ_MID and CONF_EQ_TREBLE from the player config UI
• resetting CONF_EQ_BASS, CONF_EQ_MID and CONF_EQ_TREBLE to 0 if a DSP config is saved

Otherwise it would be more difficult to handle this case:

1. The user already uses CONF_EQ_BASS, CONF_EQ_MID or CONF_EQ_TREBLE
2. The user changed the DSP config (by adding a Parametric EQ)
3. The user updated Music Assistant with the "Tone Controls" moved to DSP Config

Let me know, if I should put this in this set of PRs or open another one.

[MarvinSchenkel]

Let me first start off by saying 'Amazing work'. Really love the amount of detail you put into this PR.

I only really have one suggestion. Personally I think it makes sense to put the filter calculations inside the model classes themselves. This avoids quite some logic in the audio.py helper class.

So, for example, instead of using an Enum for the ParametricEQBandType, we could change this to an actual class with a get_filter(sample_rate) function that calculates the filter for us:

class PeakParametricEQBand:

  ....

  def get_filter(sample_rate: int) -> str:
    b0 = 1 + alpha * a
    b1 = -2 * math.cos(w_0)
    b2 = 1 - alpha * a
    a0 = 1 + alpha / a
    a1 = -2 * math.cos(w_0)
    a2 = 1 - alpha / a

    return f"biquad=b0={b0}:b1={b1}:b2={b2}:a0={a0}:a1={a1}:a2={a2}"

Then, in audio.py we can reduce all the logic to simply:

...
# Process each DSP filter sequentially
  for f in dsp.filters:
      if not f.enabled:
          continue
  
      if isinstance(f, ParametricEQFilter):
          for b in f.bands:
              if not b.enabled:
                  continue

              filter = b.get_filter(sample_rate)
              filter_params.append(filter)

What do you and @marcelveldt think?

[marcelveldt]

What do you and @marcelveldt think?

I was kind of wondering the same - the models that we have defined in our separate repo/package are basically models that are shared between client and server (we handle serialization and some validation there as well) but these seem to be server-side models only, its pretty much self-contained data and logic. So yeah it could make sense to make these server side models (actual classes and not dataclasses) but you can even have a mix of both worlds, it perfectly fine to add some helper methods to the dataclasses, for instance to create this filter config with a simple helper method within the model class.

I'm fine either way but if this logic is server-side only I'd vote for moving them to the servercode instead of the shared models package.

[marcelveldt]

I'm thinking, maybe this PR (or another PR in the same release) should also add a Tone Controls Filter?

Yes, it sounds good to consolidate the old simple tone controls to EQ by either just drop them or write a small migration function.

[maximmaxim345]

I'm fine either way but if this logic is server-side only I'd vote for moving them to the servercode instead of the shared models package.

That was my thought too.
Right now, the code to generate FFmpeg options from the underlying models could be put in both the server and model packages, but potential future filters like "convolution" or "AutoEQ" would require additional data only found on the server.

What do you and @marcelveldt think?

Another solution is to move this logic to a new dsp.py module on the server. Then the get_player_filter_params function would call that to convert the model types to FFmpeg arguments.

[maximmaxim345]

I made the following changes:

• Moved FFmpeg filter conversion to dsp.filter_to_ffmpeg_params
• Added a 'Tone Control' filter
• Added automatic migration from the old EQ settings to the new 'Tone Control' filter

The migration will be performed once someone plays music.
A future version could then remove this migration logic.

[marcelveldt]

Sorry for the late review. I have been busy last 2 weeks with other (HA) stuff so there was no more time left for MA.
I'll do my best to get this merged today. Thanks again for your efforts!

[maximmaxim345]

No problem. Let me know if some changes are necessary.

[marcelveldt]

Nice work!

[marcelveldt]

I will also do a release of the frontend and then release a beta with this feature included later today or tomorrow!

[OzGav]

@maximmaxim345 I haven't had a chance to play with this awesomeness much but one small thing. Can the toggles change to blue when on to match the rest of the UI?

[maximmaxim345]

@maximmaxim345 I haven't had a chance to play with this awesomeness much but one small thing. Can the toggles change to blue when on to match the rest of the UI?

Just tried it. Unfortunately, when using dark mode, using the primary color clashes with the background, resulting in a contrast that's too low for my liking:

Compared to the current default color:

[OzGav]

Maybe adjust the blue then as I use dark mode and this is the existing situation

I think maybe because you have a grey banner there? Either way I think not having consistency in the UI is undesirable.

[maximmaxim345]

I think maybe because you have a grey banner there? Either way I think not having consistency in the UI is undesirable.

Good point! I've experimented with tweaking the background colors while keeping the same blue. Here's what I came up with:

Dark Mode:

Light Mode:

What do you think about these adjustments?
Or should I also add an separator below the toolbar?

[OzGav]

Yeah i think that looks great and a separator would look good.