Skip to content

Conversation

vltkv
Copy link
Contributor

@vltkv vltkv commented Oct 7, 2025

Closes RNAA-205

⚠️ Breaking changes ⚠️

Introduced changes

  • added tests for BiquadFilterNode

Checklist

  • Linked relevant issue
  • Updated relevant documentation
  • Added/Conducted relevant tests
  • Performed self-review of the code
  • Updated Web Audio API coverage
  • Added support for web

@vltkv vltkv changed the title Test/biquad tests Test: biquad tests Oct 8, 2025
@vltkv vltkv changed the title Test: biquad tests test: biquad tests Oct 8, 2025
@vltkv vltkv added the tests label Oct 10, 2025
@vltkv vltkv marked this pull request as ready for review October 10, 2025 21:50
@vltkv vltkv force-pushed the test/biquad-tests branch from c9a7504 to ebb69ca Compare October 11, 2025 18:51
Copy link
Contributor

@poneciak57 poneciak57 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job 👍

double a2 = coeffs.a2;

for (size_t k = 0; k < frequency.size(); ++k) {
if (frequency[k] < 0 || frequency[k] > 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (frequency[k] < 0 || frequency[k] > 1) {
if (frequency[k] < 0.0 || frequency[k] > 1.0) {

}

auto omega = -PI * frequencyArray[i] / context_->getNyquistFrequency();
auto omega = -PI * frequencyArray[i];

This comment was marked as resolved.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see: static constexpr float PI = std::numbers::pi_v<float>; nvm then


auto omega = -PI * frequencyArray[i] / context_->getNyquistFrequency();
auto omega = -PI * frequencyArray[i];
auto z = std::complex<float>(cos(omega), sin(omega));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::cos and std::sin

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants