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

Minor issues with BiquadFilter AudioParams #2087

Closed
rtoy opened this issue Oct 23, 2019 · 0 comments
Closed

Minor issues with BiquadFilter AudioParams #2087

rtoy opened this issue Oct 23, 2019 · 0 comments
Milestone

Comments

@rtoy
Copy link
Member

rtoy commented Oct 23, 2019

Describe the issue
A few minor issues I noticed in the spec when I was looking at how some filters behave (at https://googlechromelabs.github.io/web-audio-samples/archive/demos/mag-phase.html).

  • The frequency attribute is allowed to be any finite float. I think this should probably be limited to the range from 0 Hz to the Nyquist frequency.
  • The detune attribute is likewise allowed to be any finite float. This should probably be limited a smaller range like for OscillatorNode.detune.
  • The gain attribute is any finite float. But since it's in dB, the
    range is actually much smaller since it would cause an overflow once it gets larger than about 770.

Finally, Q is a bit messy. First, for lowpass and highpass filters, Q is in dB, but is a linear value for other filter types.. Right now you can only tell if you look at the filter characterics to see how Q is used to determine the filter coefficients. We should make a note of that in the description of Q. It can be negative or positive in that case.

However, for the other filters such as bandpass, Q is a linear value. But negative values don't seem to be allowed. From https://www.w3.org/2011/audio/audio-eq-cookbook.html we can see that in this case, the Q value is related to the bandwidth for bandpass, notch, or peaking filters. Bandwidth is generally a non-negative number, so Q should be too.

Thus, we should make a note that Q is in dB for lowpass and highpass filters and can be positive or negative. For other filters, Q is linear and should be positive.

@rtoy rtoy added this to the Web Audio V1 milestone Oct 24, 2019
rtoy added a commit to rtoy/web-audio-api that referenced this issue Oct 25, 2019
As mentioned in the bug, we are

- limiting the frequency from 0 to Nyquist,
- limiting the gain parameter is limited so that it won't cause overflow in the
  formulas
- limiting the detune values as done for the Oscillator.detune
- add some additiona text to mention Q is in dB for lowpass and
  highpass filters and that for the other filters Q, negative values
  of Q are clamped to 0.
pull bot pushed a commit to FreddyZeng/chromium that referenced this issue Jan 17, 2020
The limits for the detune AudioParam have been updated to use the same
limits as OscillatorNode.detune.  The gain AudioParam limits have been
reduced so as not to cause overflow since the gain is in dB.

Tests in audioparam-nominal-range.html updated with new results.  We
also took this opportunity to fix a few minor style issues. (Use
mostPositiveFloat instead of literal value, and add space between
prefix and message for clipped values.

See WebAudio spec issues:
WebAudio/web-audio-api#2113
WebAudio/web-audio-api#2087

Chrome Status: https://www.chromestatus.com/feature/6567195645575168

This CL makes the implementation conform to the spec.

Bug: 1018303
Change-Id: I42c7ec2883fc20dbd59bc6c011ce865159648359
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1968293
Commit-Queue: Raymond Toy <[email protected]>
Reviewed-by: Hongchan Choi <[email protected]>
Cr-Commit-Position: refs/heads/master@{#732643}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants