Closed Bug 1499597 Opened 1 year ago Closed 4 months ago

Unable to set negative value for Q of BiquadFilterNode when type is "lowpass" or "highpass"

Categories

(Core :: Web Audio, defect, P2)

62 Branch
Desktop
All
defect

Tracking

()

VERIFIED FIXED
mozilla71
Tracking Status
firefox-esr68 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox71 --- verified
firefox72 --- verified
firefox73 --- verified

People

(Reporter: uuhhyyoouu, Assigned: padenot)

Details

Attachments

(4 files)

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:62.0) Gecko/20100101 Firefox/62.0

Steps to reproduce:

I wrote a code to make Karplus-Strong synthesizer with WebAudio in Firefox.

```javascript
function createKarplusStrongFilter(ctx, pitch, cutoff) {
  var delay = ctx.createDelay(4)
  delay.delayTime.value = 1 / pitch

  var filter = ctx.createBiquadFilter()
  filter.type = "lowpass"
  filter.frequency.value = cutoff
  filter.Q.value = -3 // This isn't working.
  filter.gain.value = 0

  var gain = ctx.createGain()
  gain.gain.value = 0.99

  var input = gain
  var output = gain

  delay.connect(filter)
  filter.connect(gain)
  gain.connect(delay)

  return { input, output, delay, filter, gain }
}

var ctx = new AudioContext()

var ksFilter = createKarplusStrongFilter(ctx, 220, 2000)

// Create impulse.
var buffer = ctx.createBuffer(2, 1, ctx.sampleRate)
buffer.copyToChannel(new Float32Array([1]), 0, 0)
buffer.copyToChannel(new Float32Array([1]), 1, 0)

var source = ctx.createBufferSource()
source.buffer = buffer

// Routing.
source.connect(ksFilter.input)
ksFilter.output.connect(ctx.destination)

source.start()
```


Actual results:

The above code blows up in Firefox while it doesn't in chromium.

Tested version:
- Firefox 62.0.3
- Firefox 64.0b1
- Chromium 69.0.3497.100 (Developer Build) Fedora Project (64-bit)


Expected results:

The above code shouldn't blows up.

Root cause of this issue is Q of BiquadFilterNode doesn't accept negative
value when type is "lowpass" or "highpass".

Removing

```cpp
resonance = std::max(0.0, resonance); // can't go negative
```

from setLowpassParams() and setHighpassParams() in
dom/media/webaudio/blink/Biquad.cpp may fix this issue.
Hi Takamitsu Endo, 

Please provide us with some steps to reproduce, actual and expected results, so that I can confirm and eventually verify it.

Thank you for your contribution!
Flags: needinfo?(uuhhyyoouu)
Flags: needinfo?(uuhhyyoouu)
Attached file biquad_demo.zip
Interactive demo of BiquadFilterNode for checking the different behavior of Q.
Hi Daniel.

To reproduce the issue I encountered, copy and paste the javascript code in description to Web Console in developer tools.

Be careful not to execute code on critical sound system. Also turn down the volume. It may hurt your ear and equippment.

I attached the recordings of actual (K-S_actual.wav) and expected (K-S_expected.wav) results.

When I execute the code on firefox, I got K-S_actual.wav. Be careful when playing this.

When I execute the code on chromium, I got K-S_expected.wav.

---

Direct way to check BiquadFilterNode behavior is to use getFrequencyResponse function.

https://webaudio.github.io/web-audio-api/#dom-biquadfilternode-getfrequencyresponse

We can compare a magResponse of firefox to chromium one to confirm this issue.

Example code to get magResponse.

```javascript
var ctx = new AudioContext()

var filter = ctx.createBiquadFilter()
filter.type = "lowpass"
filter.frequency.value = 1000
filter.Q.value = -3 // This isn't working.

var freq = new Float32Array([20, 200, 2000, 20000])
var mag = new Float32Array(freq.length)
var phase = new Float32Array(freq.length)

filter.getFrequencyResponse(freq, mag, phase)

console.log(mag)
```

Output on firefox. (Actual result)

```
[ 1.000199317932129, 1.0197079181671143, 0.27431735396385193, 0.00011016758071491495 ]
```

Output on chromium. (Expected result)

```
[1.0000008344650269, 0.9993003606796265, 0.24034753441810608, 0.00011016154167009518]
```

To check further, You could change values of freq.

---

I also attached biquad_demo.zip for checking the behavior of BiquadFilterNode.

Unzip and open index.html with firefox and chromium, then tweak the Q and check how the black line on the top left canvas (magnitude of frequency response in decibel) is changing.

---

Just in case, this is the current implementation of biquad filter on chromium. As I mentioned in description, there aren't lines that clamping Q (which is named resonance in the code below).

https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/audio/biquad.cc?sq=package:chromium&dr=CSs&g=0


Thanks for taking time to this issue.
I have confirmed the issue on Windows 10 on the 3 main versions of Firefox by pasting the code in comment 0 in the web console and comparing the sounds. Thanks.
Status: UNCONFIRMED → NEW
Component: Untriaged → Audio/Video
Ever confirmed: true
OS: Unspecified → All
Product: Firefox → Core
Hardware: Unspecified → Desktop
Rank: 15
Component: Audio/Video → Web Audio
Priority: -- → P2
Assignee: nobody → padenot
Pushed by padenot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6ecf31593df4
Don't clamp the Q of a BiquadFilterNode when in low-pass and high-pass mode. r=karlt

Thanks for the report and sorry for the delay!

Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

The sound after the fix does not sound identical to the one posted in comment 3, but it is very similar and free of distortion, so I deem it verified.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.