Closed cswank closed 2 years ago
This looks cool! @dusk125 will do a proper review, here's just a quick note from me: I see you're returning an *Equalizer
struct who's only methods are the ones of the Streamer
interface. In such cases, I think it's better to make the Equalizer
struct unexported and simply return an opaque Streamer
value.
@faiface sounds good, I made those changes.
One more note: I see you use fs float64
to denote the sample frequency. The rest of Beep uses beep.SampleRate
, so I think it would be better to use it here as well.
@faiface and @dusk125 I just pushed that change. Also, I have one more commit ready that implements an (optional) stereo equalizer. Would you rather take a look at the code as it is now before I push the stereo change? Stereo makes the code more complicated so maybe it would be good to verify the functionality in mono first.
Never mind, I'm going to push the stereo change because I'm about to be driving for 2 days.
Sounds good! That'll give me some time to review and wrap my head around this! Safe travels!
Thanks!
based on https://octovoid.com/2017/11/04/coding-a-parametric-equalizer-for-audio-applications/