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

Make DSP Configuration UI More Consistent #798

Merged
merged 3 commits into from
Dec 31, 2024

Conversation

maximmaxim345
Copy link
Contributor

Overview

This PR primarily makes sliders and switches use the primary color, as found in the rest of Music Assistant (notably the existing Settings Menu).

The other visual adjustments, as seen in the screenshots, increase the contrast between the primary color and the background (and include some margin adjustments resulting from these changes).

Screenshots

new-dark

new-light

Reference Screenshots before this PR

old-dark

old-light

@OzGav
Copy link
Contributor

OzGav commented Dec 31, 2024

I appreciate your efforts. This looks good to me!

@marcelveldt marcelveldt merged commit ae50aa5 into music-assistant:main Dec 31, 2024
2 checks passed
@OzGav
Copy link
Contributor

OzGav commented Jan 1, 2025

I am just starting to play with this some more now. The following are just my thoughts/ opinions with no UI precedent so feel free to disagree!

I note that when adding a filter a blue/cyan dot appears on the left to indicate it is enabled (and selected or not) and no dot when it is disabled. I wonder if a green (enabled) and red (disabled) dot may be more intuitive? Or maybe leave the current enabled indication and just have a red dot (or an x) to show it is disabled?

I am no DSP whiz so this may be a dumb question but does it make sense to be able to add multiple of the same filter?

Lastly, and not a UI question, we need to add some info to the docs about this awesomeness. Probably some basic info here https://music-assistant.io/player-support/ and maybe advanced info here or amendments to this https://music-assistant.io/faq/tech-info/ If you have any raw text or links to info I can paraphrase then I can do the actual PRs

P.S. Are you on our Discord server?

@maximmaxim345
Copy link
Contributor Author

I am just starting to play with this some more now. The following are just my thoughts/ opinions with no UI precedent so feel free to disagree!

No problem! Not many music players offer such a DSP feature. The only one that comes to mind is ROON: https://help.roonlabs.com/portal/en/kb/articles/muse

I note that when adding a filter a blue/cyan dot appears on the left to indicate it is enabled (and selected or not) and no dot when it is disabled. I wonder if a green (enabled) and red (disabled) dot may be more intuitive? Or maybe leave the current enabled indication and just have a red dot (or an x) to show it is disabled?

Here are my thoughts on the current design:

  • The line represents the audio path from the audio file (top) to the speaker (bottom)
  • A dot represents a filter that changes the signal, so disabled filters don't have a dot since they don't touch the audio signal
  • It would be great to change the appearance of the dot to more clearly communicate this; however, this uses Vuetify's timelines, so a completely custom widget would need to be written

In case we show a red dot, this could also be interpreted as either still running the filter partly (I don't know, maybe a filter has a built-in preamp that will still be applied when disabled). An 'x' could also be understood as cutting the audio signal.

I am no DSP whiz so this may be a dumb question but does it make sense to be able to add multiple of the same filter?

I can think of a couple of scenarios:

  • Having one EQ measured (with a microphone in REW or from https://autoeq.app/) and a second EQ applying adjustments to sound preference (like adding more bass)
  • We already support multiple filter types (the complex PEQ and the simple Tone Control filter). So having multiple PEQs is not that far off
  • I am also planning to add Compressors/Limiters in the future. The order of having those and EQs will change the sound (this is the reason why you can already change the order of filters)

Lastly, and not a UI question, we need to add some info to the docs about this awesomeness. Probably some basic info here https://music-assistant.io/player-support/ and maybe advanced info here or amendments to this https://music-assistant.io/faq/tech-info/ If you have any raw text or links to info I can paraphrase then I can do the actual PRs

That would be very nice!
I will send you some resources/texts once I find some time.

P.S. Are you on our Discord server?

Yes I am, but I don't check any notifications.

On another note, I will soon open PRs for the following features:

  • Import and Export of EqualizerAPO Presets (also supports import of REW filters). The APO format is widely supported by most applications, so for example https://autoeq.app/ filters could then be simply imported.
  • Saving and loading of DSP presets. Useful for having DSP for multiple different listening positions, or just using headphones on different devices.

@OzGav
Copy link
Contributor

OzGav commented Jan 1, 2025

No problems on the Discord question I just thought it might be easier to converse vice this medium but this is working!

Your explanation of the UI sounds good. Looking at it again having read your response it makes perfect sense to me. I think once the docs are in place this will then be clear for all.

As for the docs, thanks for the Roon link, I think I will borrow some of their explanatory ideas!

I am away on holidays at the moment but I’ll see if I can make a start on the docs as this is so good, and as you say a very unique feature, that I would like to make sure everyone knows of its existence and how to use it.

@OzGav
Copy link
Contributor

OzGav commented Jan 1, 2025

One thing I think is missing from the UI is an indication of when the DSP settings are active. Correct me if I’m wrong but right now I think the idea is they are active by default but with no optional filters. You can then deactivate the DSP setting completely.

At the moment I can’t tell, without going into the player settings, if the DSP is active or if any optional filters are added.

Question is should the settings be enabled by default? (I understand they do nothing by default as the gains are set to zero)

Regardless of the answer to that I think an indication of the status should at least be done in the player settings and maybe via an icon in the Player List (if it’s active)?

@maximmaxim345
Copy link
Contributor Author

No problems on the Discord question I just thought it might be easier to converse vice this medium but this is working!

I just haven't had the time to clean up my Discord from all the spam. Maybe I'll DM you if I do so.

One thing I think is missing from the UI is an indication of when the DSP settings are active. Correct me if I’m wrong but right now I think the idea is they are active by default but with no optional filters. You can then deactivate the DSP setting completely.

Question is should the settings be enabled by default? (I understand they do nothing by default as the gains are set to zero)

You are mostly right. I initially set DSP to be active by default to accommodate users of the old EQ system (which was primarily available on Airplay). This would automatically add a Tone Control filter to their DSP settings. This approach was part of my original migration logic, though it was never merged into the main branch.

But I've since rewritten part of the migration logic (merged into server), so that we now could only enable the DSP if the user really used the old EQ.

At the moment I can’t tell, without going into the player settings, if the DSP is active or if any optional filters are added.

This should not be an issue anymore if the DSP is disabled by default.
I will open a PR for that!

Regardless of the answer to that I think an indication of the status should at least be done in the player settings and maybe via an icon in the Player List (if it’s active)?

This is definitely something we need to think about.
Maybe somewhere in/around/beneath the Streamdetails on the fullscreen player?
I mean replay gain is shown there, which is technically also a DSP??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants