-
-
Notifications
You must be signed in to change notification settings - Fork 83
Add parametric equalizer with interactive frequency response curve #606
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
base: master
Are you sure you want to change the base?
Conversation
- Replace fixed 10-band graphic EQ with fully parametric EQ - Support Bell, Low Shelf, and High Shelf filter types - Add interactive frequency response curve with draggable nodes - Implement undo/redo stack and A-B comparison - Add automatic migration from legacy graphic EQ presets - Include unit tests for band parameters and migration logic
|
|
||
| nonisolated private func addLowPrio(objects: [(DownloadElementInfo, Downloadable)], library: LibraryStorage) -> [(DownloadElementInfo, Downloadable, Download)] { | ||
|
|
||
| nonisolated private func addLowPrio( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a merge error.
| @@ -229,6 +229,7 @@ public protocol PlayerFacade { | |||
|
|
|||
| func updateEqualizerEnabled(isEnabled: Bool) | |||
| func updateEqualizerSetting(eqSetting: EqualizerSetting) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could be removed since we only use the ParametricEQ
| @@ -509,6 +510,10 @@ class PlayerFacadeImpl: PlayerFacade { | |||
| backendAudioPlayer.updateEqualizerSetting(eqSetting: eqSetting) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one too.
| return downloadMOs?.compactMap { Download(managedObject: $0) } ?? [Download]() | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
| set { _activeParametricEqualizerSetting = newValue } | ||
| } | ||
|
|
||
| private var _equalizerMigrationCompleted: Bool = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed. Check that the old EQ Settings are not empty. Do the conversion once and let the old EQ settings.
|
|
||
| private func migrateEqualizerSettingsIfNeeded() { | ||
| // Check if migration has already been completed | ||
| guard !storage.settings.user.equalizerMigrationCompleted else { return } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check for old EQ settings are empty
BLeeEZ
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job. The settings view looks great. To be honest the calculation is way over my head. I have no idea if that what is displayed is correctly applied to the audio engine. I left a couple of comments.
|
I have two other questions:
|
|
Thanks BLeeEZ, I'll go through and apply the changes and update 😊 I used the CoreAudio engine calculations as the base, so hopefully they should be accurate. You never know with Apple whether that will change something in the future which will break the calculations. It's a very good thought, I will try to add a 'detail' slider to hide/show the detail such as the gain/cut calculations as some users may find it too technical. For your other questions:
|
I noticed a gap in the market for music players that have really decent, but easy to use, Parametric EQ capabilities. Amperfy is my go-to music player. So I wrote one!
This PR swaps the existing graphic EQ system for a comprehensive Parametric EQ system.
I've run it through the testing and formatting tools in the Amperfy repo, all of which pass. I've built it locally on my Mac and tested using simulators, but would welcome on-device testing.
It would be great to be able to allow the user to switch between the existing graphic and Parametric EQ's. Happy to build this functionality out if useful.
Features:
Changes
New Files
Amperfy/SwiftUI/Settings/Player/ParametricEQ/(5 files)AmperfyKitTests/Cases/Player/ParametricEQTest.swiftModified Files
AmperfyKit/Storage/PersistentStorage.swift- New types and settingsAmperfyKit/AmperfyKit.swift- Migration logicAmperfyKit/Player/BackendAudioPlayer.swift- Parametric EQ engineAmperfyKit/Player/PlayerFacade.swift- Protocol updateAmperfy/SwiftUI/Settings/ObservableSettings.swift- Observable propertiesAmperfy/SwiftUI/Settings/Player/EqualizerSettingsView.swift- UI rewriteAmperfy/Screens/ViewController/SettingsHostVC.swift- Settings bindingsTesting
./BuildTools/applyFormat.shScreenshots