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

Modularize Normalisation a bit #1163

Closed
wants to merge 2 commits into from
Closed

Conversation

JasonLG1979
Copy link
Contributor

No description provided.

@roderickvd
Copy link
Member

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

@JasonLG1979
Copy link
Contributor Author

JasonLG1979 commented May 14, 2023

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
Copy link
Member

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
Copy link
Contributor Author

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

@JasonLG1979
Copy link
Contributor Author

#1180

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

Successfully merging this pull request may close these issues.

2 participants