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!

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.

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?

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.

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.

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.

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.