search

napframework / nap

NAP Framework source code
https://nap-framework.tech
Mozilla Public License 2.0
409 stars 23 forks source link

Fix RMS calculation #61

Closed CasimirGeelhoed closed 3 weeks ago

CasimirGeelhoed commented 4 weeks ago

I accidentally omitted a square root when rewriting the RMS implementation in LevelMeterNode. With this PR it is re-added.

I decided to add the sqrt() call in the getter to not impact audio thread performance.

cklosters commented 4 weeks ago

Thanks for the fix, the validation server is currently down because of a module (hardware) failure in my router. Will review this when it's back online.

lshoek commented 4 weeks ago

also prefix sqrt with std:: to avoid ambiguity :)

cklosters commented 3 weeks ago

also prefix sqrt with std:: to avoid ambiguity :)

Agreed, namespaces shouldn't be dropped, especially with common function declaration names such as sqrt. From the styleguide:

With few exceptions, place code in the nap namespace. Every namespace should have unique name based on the project. Using namespaces in .cpp files is encouraged. Do not use using directives (e.g. using namespace foo). If so, only in .cpp files.

Do not use inline namespaces
Do not use namespace aliases: namespace foo_bar = ::foo
CasimirGeelhoed commented 3 weeks ago

Thanks for the review! I have updated the class to align with the styleguide by adding the std:: prefix, using an enum class and renaming Type to EType.