[WebAudio] Fix DCHECK crash in Biquad::TailFrame [chromium/src : main]
0 views
Skip to first unread message
Hongchan Choi (Gerrit)
Jul 24, 2026, 12:00:13 PM (16 hours ago) Jul 24
to Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Michael Wilson
Hongchan Choi added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Michael Wilson
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Wilson (Gerrit)
Jul 24, 2026, 6:04:21 PM (10 hours ago) Jul 24
to Hongchan Choi, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Hongchan Choi
Michael Wilson added 4 comments![]( "Open in Gerrit")
File third_party/blink/renderer/modules/webaudio/biquad_filter_handler_test.cc
Line 8, Patchset 6 (Latest):
#include "third_party/blink/renderer/platform/audio/biquad.h"Michael Wilson . unresolved
The new tests in this file don't seem to use the BiquadFilterHandler at all. Would it make sense to move them to a new platform/audio/biquad_test.cc file instead?
File third_party/blink/renderer/platform/audio/biquad.cc
Line 606, Patchset 6 (Latest):
// (n-2)*log(r) + log(|c1*(n+1)*r^2+c2|) - log(eps)Michael Wilson . unresolved
Let's update this comment
```suggestion
// (n-2)*log(|r|) + log(|c1*(n+1)*r^2+c2|) - log(eps)
```
Line 612, Patchset 6 (Latest):
fdlibm::log(fabs(c1 * (n + 1) * r * r + c2)) - log_eps;Michael Wilson . unresolved
Perhaps this should be `std::abs` too, since we're working with `double`s?
Line 799, Patchset 6 (Latest):
return 0;Michael Wilson . unresolved
Let's add a comment here saying something like we don't want to keep calculating tail time for filters with non-finite coefficients.
Related details
Attention is currently required from:
- Hongchan Choi
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hongchan Choi (Gerrit)
Jul 24, 2026, 6:39:03 PM (10 hours ago) Jul 24
to Nate Chapin, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Michael Wilson and Nate Chapin
Hongchan Choi added 5 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 9 (Latest):
Hongchan Choi . resolved
japhet@ PTAL at t/b/r/p/BUILD.gn
Thanks in advance!
File third_party/blink/renderer/modules/webaudio/biquad_filter_handler_test.cc
Line 8, Patchset 6:
#include "third_party/blink/renderer/platform/audio/biquad.h"Michael Wilson . resolved
The new tests in this file don't seem to use the BiquadFilterHandler at all. Would it make sense to move them to a new platform/audio/biquad_test.cc file instead?
Hongchan Choi
Done
File third_party/blink/renderer/platform/audio/biquad.cc
Let's update this comment
```suggestion
// (n-2)*log(|r|) + log(|c1*(n+1)*r^2+c2|) - log(eps)
```
Hongchan Choi
Done
Line 612, Patchset 6:
fdlibm::log(fabs(c1 * (n + 1) * r * r + c2)) - log_eps;Michael Wilson . resolved
Perhaps this should be `std::abs` too, since we're working with `double`s?
Hongchan Choi
Done
Let's add a comment here saying something like we don't want to keep calculating tail time for filters with non-finite coefficients.
Attention is currently required from:
- Michael Wilson
- Nate Chapin
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nate Chapin (Gerrit)
Jul 24, 2026, 6:44:38 PM (10 hours ago) Jul 24
to Hongchan Choi, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Michael Wilson
Nate Chapin voted and added 1 comment![]( "Open in Gerrit")
Votes added by Nate Chapin
| Code-Review | +1 |
1 comment
Patchset-level comments
Nate Chapin . resolved
Build file change LGTM - didn't review the logic change
Related details
Attention is currently required from:
- Michael Wilson
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hongchan Choi (Gerrit)
Jul 24, 2026, 8:25:48 PM (8 hours ago) Jul 24
to Nate Chapin, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Michael Wilson
Hongchan Choi voted Auto-Submit+1![]( "Open in Gerrit")
| Auto-Submit | +1 |
Related details
Attention is currently required from:
- Michael Wilson
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Wilson (Gerrit)
1:18 AM (3 hours ago) 1:18 AM
to Hongchan Choi, Nate Chapin, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org
Attention needed from Hongchan Choi
Michael Wilson voted and added 1 comment![]( "Open in Gerrit")
Votes added by Michael Wilson
| Code-Review | +1 |
| Commit-Queue | +2 |
1 comment
Patchset-level comments
Attention is currently required from:
- Hongchan Choi
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages