Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements dynamic filter range selection in the GUI based on the filter type, replacing a fixed range (20.0..=20,000.0 Hz) with type-specific ranges. The change aims to provide more appropriate slider ranges for highpass and lowpass filters.
Key changes:
- Added a match expression to determine slider range based on
FilterType(Highpass vs Lowpass) - Highpass filters now use a range of 0.0..=1000.0 Hz
- Lowpass filters now use a range of 5000.0..=15000.0 Hz
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| FilterType::Highpass => 0.0..=1000.0, | ||
| FilterType::Lowpass => 5000.0..=15000.0, |
There was a problem hiding this comment.
The filter ranges create a problematic gap in the 1000-5000 Hz range. A highpass filter with a maximum cutoff of 1000 Hz and a lowpass filter with a minimum cutoff of 5000 Hz means users cannot set cutoff frequencies between 1000-5000 Hz. This is a critical frequency range for audio filtering.
Additionally, the ranges seem semantically incorrect:
- Highpass filters typically cut frequencies below the cutoff, so they should allow higher cutoff values (to filter out more low frequencies)
- Lowpass filters typically cut frequencies above the cutoff, so they should allow lower cutoff values (to filter out more high frequencies)
Consider ranges like:
- Highpass: 20.0..=5000.0 (or higher)
- Lowpass: 1000.0..=20000.0 (or use the full spectrum)
This would provide full coverage of the audio spectrum and allow users to configure filters appropriately for their needs.
| FilterType::Highpass => 0.0..=1000.0, | |
| FilterType::Lowpass => 5000.0..=15000.0, | |
| FilterType::Highpass => 20.0..=5000.0, | |
| FilterType::Lowpass => 1000.0..=20000.0, |
Closes #111