librespot-org / librespot

Open Source Spotify client library
MIT License
4.52k stars 544 forks source link

Modularize Normalisation a bit #1163

Closed JasonLG1979 closed 1 year ago

roderickvd commented 1 year ago

I guess making this modular is a technical enabler for other limiters / compressors?

JasonLG1979 commented 1 year ago

I guess making this modular is a technical enabler for other limiters / compressors?

Sure all you'd have to do is put whatever different kind of Normaliser in the Normaliser enum, with a few caveats ofc. I tried on purpose not to have to Box dny it. If you did anything that would make you have to box it, you might as well just use Traits. But I'm working under the assumption that any kind of normaliser is more or less just going to be some sort of fancy calculator and it shouldn't need to be complex from a moving parts aspect.

I did design it using Traits 1st. In that version the Normaliser kind was set at init. I then designed it as an enum to see what difference it made performance wise. It made absolutely none and the enum version is much simpler it allows for only 1 extra struct (currently ofc) because no state is needed for no and basic normalisation.

roderickvd commented 1 year ago

I think it's a nice add and allows some flexibility for those would want to do their own DSP, without making it too complex. Also taking it out of the monolithic player is a good thing. Would you add a changelog?

JasonLG1979 commented 1 year ago

@roderickvd The change log is updated. Ready for you to merge.

JasonLG1979 commented 1 year ago

https://github.com/librespot-org/librespot/pull/1180