[webaudio] Migrate setValueCurveAtTime test to testharness.js [chromium/src : main]
0 views
Skip to first unread message
Michael Wilson (Gerrit)
Jun 17, 2025, 7:39:10 PMJun 17
to Punith Nayak, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org
Attention needed from Punith Nayak
Michael Wilson added 7 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 3 (Latest):
Michael Wilson . resolved
Basically looks good; please see other comments.
File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/audioparam-setValueCurveAtTime.html
Line 4, Patchset 3 (Latest):
<title>AudioParam.setValueCurveAtTime (migrated)</title>Michael Wilson . unresolved
I don't think we need to write "(migrated")
Line 5, Patchset 3 (Latest):
<meta charset="utf-8">Michael Wilson . unresolved
I don't think we need this.
Line 8, Patchset 3 (Latest):
<!-- Web-Audio helper utilities (same as original) -->Michael Wilson . unresolved
I don't think we need this comment.
Line 17, Patchset 3 (Parent):
// Play a long DC signal out through an AudioGainNode and for each time
// interval call setValueCurveAtTime() to set the values for the duration
// of the interval. Each curve is a sine wave, and we assume that the
// time interval is not an exact multiple of the period. This causes a
// discontinuity between time intervals which is used to test timing.Michael Wilson . unresolved
Let's keep this comment.
Line 26, Patchset 3 (Parent):
// Max allowed difference between the rendered data and the expected
// result. Because of the linear interpolation, the rendered curve isn't
// exactly the same as the reference. This value is experimentally
// determined.
let maxAllowedError = 3.7194e-6;Michael Wilson . unresolved
Let's keep this comment because it says how the value was determined.
Line 29, Patchset 3 (Latest):
* very small subset of the Audit-style “should” API used by the helpers
* so that we don’t have to ship audit.js.Michael Wilson . unresolved
We shouldn't refer to audit.js in comments since we will be removing this file. Let's just say this is the minimum required functionality for createAudioGraphAndTest.
Related details
Attention is currently required from:
- Punith Nayak
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Punith Nayak (Gerrit)
Jun 19, 2025, 4:21:28 PMJun 19
to Alvin Ji, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org
Attention needed from Michael Wilson
Punith Nayak added 6 comments![]( "Open in Gerrit")
File third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/audioparam-setValueCurveAtTime.html
Line 4, Patchset 3:
<title>AudioParam.setValueCurveAtTime (migrated)</title>Michael Wilson . resolved
I don't think we need to write "(migrated")
Punith Nayak
Done
I don't think we need this.
Punith Nayak
Done
<!-- Web-Audio helper utilities (same as original) -->Michael Wilson . resolved
I don't think we need this comment.
Punith Nayak
Done
Line 17, Patchset 3 (Parent):
// Play a long DC signal out through an AudioGainNode and for each time
// interval call setValueCurveAtTime() to set the values for the duration
// of the interval. Each curve is a sine wave, and we assume that the
// time interval is not an exact multiple of the period. This causes a
// discontinuity between time intervals which is used to test timing.Michael Wilson . resolved
Let's keep this comment.
Punith Nayak
Done
Line 26, Patchset 3 (Parent):
// Max allowed difference between the rendered data and the expected
// result. Because of the linear interpolation, the rendered curve isn't
// exactly the same as the reference. This value is experimentally
// determined.
let maxAllowedError = 3.7194e-6;Michael Wilson . resolved
Let's keep this comment because it says how the value was determined.
Punith Nayak
Done
Line 29, Patchset 3:
* very small subset of the Audit-style “should” API used by the helpers
* so that we don’t have to ship audit.js.Michael Wilson . resolved
We shouldn't refer to audit.js in comments since we will be removing this file. Let's just say this is the minimum required functionality for createAudioGraphAndTest.
Attention is currently required from:
- Michael Wilson
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Wilson (Gerrit)
Jun 20, 2025, 1:10:09 PMJun 20
to Punith Nayak, Alvin Ji, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org
Attention needed from Punith Nayak
Michael Wilson voted and added 1 comment![]( "Open in Gerrit")
Votes added by Michael Wilson
| Code-Review | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Punith Nayak
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alvin Ji (Gerrit)
Jun 20, 2025, 11:50:43 PMJun 20
to Punith Nayak, Adam Raine, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org
Attention needed from Punith Nayak
Alvin Ji voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alvin Ji
| Code-Review | +1 |
1 comment
Patchset-level comments
Alvin Ji . resolved
LGTM
Related details
Attention is currently required from:
- Punith Nayak
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Punith Nayak (Gerrit)
Jun 24, 2025, 3:46:29 PM (14 days ago) Jun 24
to Alvin Ji, Adam Raine, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org
Punith Nayak voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Jun 24, 2025, 4:15:12 PM (14 days ago) Jun 24
to Punith Nayak, Alvin Ji, Adam Raine, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[webaudio] Migrate setValueCurveAtTime test to testharness.js
Replaces audit.js-based test with testharness.js for improved
maintainability and consistency with WPT style.
Change-Id: Id75a755921cd390a9f262ea954b6aa704524d01f
Reviewed-by: Alvin Ji <alv...@chromium.org>
Reviewed-by: Michael Wilson <mjwi...@chromium.org>
Commit-Queue: Punith Nayak <punith...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1478154}
Files:
- M third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audioparam-interface/audioparam-setValueCurveAtTime.html
Change size: M
Delta: 1 file changed, 54 insertions(+), 37 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Alvin Ji, +1 by Michael Wilson
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Jun 24, 2025, 5:39:04 PM (14 days ago) Jun 24
to Chromium LUCI CQ, Punith Nayak, Alvin Ji, Adam Raine, AyeAye, chromium...@chromium.org, Hongchan Choi, blink-revie...@chromium.org, blink-...@chromium.org
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/53365
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages